search

Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.98k stars 558 forks source link

valgrind errors in pp_seq dealing with semaphores #13626

Open p5pRT opened 10 years ago

p5pRT commented 10 years ago

Migrated from rt.perl.org#121335 (status was 'open')

Searchable as RT121335$

p5pRT commented 10 years ago

From @khwilliamson

These errors showed up

t/io/sem ...................................................... ==13598== Conditional jump or move depends on uninitialised value(s) ==13598== at 0x5CE913​: S_uiv_2buf (sv.c​:2751) ==13598== by 0x5D0180​: Perl_sv_2pv_flags (sv.c​:2934) ==13598== by 0x5FACC3​: Perl_sv_eq_flags (sv.c​:7511) ==13598== by 0x638C0F​: Perl_pp_seq (pp.c​:2146) ==13598== by 0x552D01​: Perl_runops_debug (dump.c​:2372) ==13598== by 0x45BE7C​: S_run_body (perl.c​:2445) ==13598== by 0x45B29E​: perl_run (perl.c​:2361) ==13598== by 0x41AAC4​: main (perlmain.c​:112) ==13598== ==13598== Conditional jump or move depends on uninitialised value(s) ==13598== at 0x5CE9A3​: S_uiv_2buf (sv.c​:2758) ==13598== by 0x5D0180​: Perl_sv_2pv_flags (sv.c​:2934) ==13598== by 0x5FACC3​: Perl_sv_eq_flags (sv.c​:7511) ==13598== by 0x638C0F​: Perl_pp_seq (pp.c​:2146) ==13598== by 0x552D01​: Perl_runops_debug (dump.c​:2372) ==13598== by 0x45BE7C​: S_run_body (perl.c​:2445) ==13598== by 0x45B29E​: perl_run (perl.c​:2361) ==13598== by 0x41AAC4​: main (perlmain.c​:112) ==13598== ==13598== Conditional jump or move depends on uninitialised value(s) ==13598== at 0x4C2A647​: bcmp (mc_replace_strmem.c​:934) ==13598== by 0x5FB2BB​: Perl_sv_eq_flags (sv.c​:7556) ==13598== by 0x638C0F​: Perl_pp_seq (pp.c​:2146) ==13598== by 0x552D01​: Perl_runops_debug (dump.c​:2372) ==13598== by 0x45BE7C​: S_run_body (perl.c​:2445) ==13598== by 0x45B29E​: perl_run (perl.c​:2361) ==13598== by 0x41AAC4​: main (perlmain.c​:112) ==13598== ==13598== Conditional jump or move depends on uninitialised value(s) ==13598== at 0x4C2A669​: bcmp (mc_replace_strmem.c​:934) ==13598== by 0x5FB2BB​: Perl_sv_eq_flags (sv.c​:7556) ==13598== by 0x638C0F​: Perl_pp_seq (pp.c​:2146) ==13598== by 0x552D01​: Perl_runops_debug (dump.c​:2372) ==13598== by 0x45BE7C​: S_run_body (perl.c​:2445) ==13598== by 0x45B29E​: perl_run (perl.c​:2361) ==13598== by 0x41AAC4​: main (perlmain.c​:112) ==13598== ==13598== Conditional jump or move depends on uninitialised value(s) ==13598== at 0x5FB2BE​: Perl_sv_eq_flags (sv.c​:7556) ==13598== by 0x638C0F​: Perl_pp_seq (pp.c​:2146) ==13598== by 0x552D01​: Perl_runops_debug (dump.c​:2372) ==13598== by 0x45BE7C​: S_run_body (perl.c​:2445) ==13598== by 0x45B29E​: perl_run (perl.c​:2361) ==13598== by 0x41AAC4​: main (perlmain.c​:112) ==13598==

And in cpan/IPC-SysV/t/sem ........................................... ==17091== Conditional jump or move depends on uninitialised value(s) ==17091== at 0x5CE913​: S_uiv_2buf (sv.c​:2751) ==17091== by 0x5D0180​: Perl_sv_2pv_flags (sv.c​:2934) ==17091== by 0x6C6668​: Perl_do_join (doop.c​:693) ==17091== by 0x5AC3F7​: Perl_pp_join (pp_hot.c​:741) ==17091== by 0x552D01​: Perl_runops_debug (dump.c​:2372) ==17091== by 0x45BE7C​: S_run_body (perl.c​:2445) ==17091== by 0x45B29E​: perl_run (perl.c​:2361) ==17091== by 0x41AAC4​: main (perlmain.c​:112) ==17091== ==17091== Conditional jump or move depends on uninitialised value(s) ==17091== at 0x5CE9A3​: S_uiv_2buf (sv.c​:2758) ==17091== by 0x5D0180​: Perl_sv_2pv_flags (sv.c​:2934) ==17091== by 0x6C6668​: Perl_do_join (doop.c​:693) ==17091== by 0x5AC3F7​: Perl_pp_join (pp_hot.c​:741) ==17091== by 0x552D01​: Perl_runops_debug (dump.c​:2372) ==17091== by 0x45BE7C​: S_run_body (perl.c​:2445) ==17091== by 0x45B29E​: perl_run (perl.c​:2361) ==17091== by 0x41AAC4​: main (perlmain.c​:112) ==17091== ==17091== Conditional jump or move depends on uninitialised value(s) ==17091== at 0x4C2A647​: bcmp (mc_replace_strmem.c​:934) ==17091== by 0x5FB2BB​: Perl_sv_eq_flags (sv.c​:7556) ==17091== by 0x638C0F​: Perl_pp_seq (pp.c​:2146) ==17091== by 0x552D01​: Perl_runops_debug (dump.c​:2372) ==17091== by 0x45BE7C​: S_run_body (perl.c​:2445) ==17091== by 0x45B29E​: perl_run (perl.c​:2361) ==17091== by 0x41AAC4​: main (perlmain.c​:112) ==17091== ==17091== Conditional jump or move depends on uninitialised value(s) ==17091== at 0x4C2A669​: bcmp (mc_replace_strmem.c​:934) ==17091== by 0x5FB2BB​: Perl_sv_eq_flags (sv.c​:7556) ==17091== by 0x638C0F​: Perl_pp_seq (pp.c​:2146) ==17091== by 0x552D01​: Perl_runops_debug (dump.c​:2372) ==17091== by 0x45BE7C​: S_run_body (perl.c​:2445) ==17091== by 0x45B29E​: perl_run (perl.c​:2361) ==17091== by 0x41AAC4​: main (perlmain.c​:112) ==17091== ==17091== Conditional jump or move depends on uninitialised value(s) ==17091== at 0x5FB2BE​: Perl_sv_eq_flags (sv.c​:7556) ==17091== by 0x638C0F​: Perl_pp_seq (pp.c​:2146) ==17091== by 0x552D01​: Perl_runops_debug (dump.c​:2372) ==17091== by 0x45BE7C​: S_run_body (perl.c​:2445) ==17091== by 0x45B29E​: perl_run (perl.c​:2361) ==17091== by 0x41AAC4​: main (perlmain.c​:112) ==17091== -- Karl Williamson

p5pRT commented 10 years ago

From @iabyn

On Wed\, Feb 26\, 2014 at 08​:36​:36PM -0800\, Karl Williamson wrote​:

These errors showed up

t/io/sem ...................................................... ==13598== Conditional jump or move depends on uninitialised value(s) ==13598== at 0x5CE913​: S_uiv_2buf (sv.c​:2751)

I can't reproduce these. Can you supply perl -V output?

-- The warp engines start playing up a bit\, but seem to sort themselves out after a while without any intervention from boy genius Wesley Crusher.   -- Things That Never Happen in "Star Trek" #17

p5pRT commented 10 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 10 years ago

From @khwilliamson

On 02/28/2014 07​:44 AM\, Dave Mitchell wrote​:

On Wed\, Feb 26\, 2014 at 08​:36​:36PM -0800\, Karl Williamson wrote​:

These errors showed up

t/io/sem ...................................................... ==13598== Conditional jump or move depends on uninitialised value(s) ==13598== at 0x5CE913​: S_uiv_2buf (sv.c​:2751)

I can't reproduce these. Can you supply perl -V output?

They are still there in today's blead run on dromedary\, perl -V output below. In fact there are new failures. The output is attached. Some from mg.c and some from regcomp.c\, and some from cpan upstream. I will look into the regcomp ones.

We were talking on #irc about the advisability of doing regular smokes using valgrind. I think these results indicate that is a good idea. Matthew Horsfall's patch that he sent me\, and which I applied on dromedary allows valgrind to run in parallel\, cutting the test time to under 2 hours there.

dromedary> myperl -V Summary of my perl5 (revision 5 version 19 subversion 10) configuration​:   Derived from​: b838857b5256d738820be79973535c28fc4da01c   Platform​:   osname=linux\, osvers=2.6.32-358.el6.x86_64\, archname=x86_64-linux-thread-multi   uname='linux dromedary-001.ams6.corp.booking.com 2.6.32-358.el6.x86_64 #1 smp fri feb 22 00​:31​:26 utc 2013 x86_64 x86_64 x86_64 gnulinux '   config_args='-des -Dprefix=/home/khw/devel -Dusedevel -D'optimize=-ggdb3' -A'optimize=-ggdb3' -A'optimize=-O0' -Accflags=-Winline -Dman1dir=none -Dman3dir=none -DDEBUGGING -Dcc=g++ -Dusethreads'   hint=recommended\, useposix=true\, d_sigaction=define   useithreads=define\, usemultiplicity=define   use64bitint=define\, use64bitall=define\, uselongdouble=undef   usemymalloc=n\, bincompat5005=undef   Compiler​:   cc='g++'\, ccflags ='-D_REENTRANT -D_GNU_SOURCE -Winline -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'\,   optimize=' -ggdb3 -O0'\,   cppflags='-D_REENTRANT -D_GNU_SOURCE -Winline -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'   ccversion=''\, gccversion='4.4.7 20120313 (Red Hat 4.4.7-4)'\, gccosandvers=''   intsize=4\, longsize=8\, ptrsize=8\, doublesize=8\, byteorder=12345678   d_longlong=define\, longlongsize=8\, d_longdbl=define\, longdblsize=16   ivtype='long'\, ivsize=8\, nvtype='double'\, nvsize=8\, Off_t='off_t'\, lseeksize=8   alignbytes=8\, prototype=define   Linker and Libraries​:   ld='g++'\, ldflags =' -fstack-protector -L/usr/local/lib'

libpth=/usr/lib/gcc/x86_64-redhat-linux/4.4.7/../../../../include/c++/4.4.7 /usr/lib/gcc/x86_64-redhat-linux/4.4.7/../../../../include/c++/4.4.7/x86_64-redhat-linux /usr/lib/gcc/x86_64-redhat-linux/4.4.7/../../../../include/c++/4.4.7/backward /usr/local/lib /usr/lib /lib/../lib64 /usr/lib/../lib64 /lib /lib64 /usr/lib64 /usr/local/lib64   libs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc   perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc   libc=libc-2.12.so\, so=so\, useshrplib=false\, libperl=libperl.a   gnulibc_version='2.12'   Dynamic Linking​:   dlsrc=dl_dlopen.xs\, dlext=so\, d_dlsymun=undef\, ccdlflags='-Wl\,-E'   cccdlflags='-fPIC'\, lddlflags='-shared -ggdb3 -ggdb3 -O0 -L/usr/local/lib -fstack-protector'

Characteristics of this binary (from libperl)​:   Compile-time options​: DEBUGGING HAS_TIMES MULTIPLICITY PERLIO_LAYERS   PERL_DONT_CREATE_GVSV   PERL_HASH_FUNC_ONE_AT_A_TIME_HARD   PERL_IMPLICIT_CONTEXT PERL_MALLOC_WRAP   PERL_NEW_COPY_ON_WRITE PERL_PRESERVE_IVUV   PERL_TRACK_MEMPOOL PERL_USE_DEVEL USE_64_BIT_ALL   USE_64_BIT_INT USE_ITHREADS USE_LARGE_FILES   USE_LOCALE USE_LOCALE_COLLATE USE_LOCALE_CTYPE   USE_LOCALE_NUMERIC USE_PERLIO USE_PERL_ATOF   USE_REENTRANT_API   Locally applied patches​:   uncommitted-changes   Built under linux   Compiled at Feb 28 2014 18​:04​:25   %ENV​:   PERL5OPT="-w"   @​INC​:   /home/khw/perl/working/lib   /home/khw/devel/lib/perl5/site_perl/5.19.10/x86_64-linux-thread-multi   /home/khw/devel/lib/perl5/site_perl/5.19.10   /home/khw/devel/lib/perl5/5.19.10/x86_64-linux-thread-multi   /home/khw/devel/lib/perl5/5.19.10   .

p5pRT commented 10 years ago

From @khwilliamson

valgrind.out

p5pRT commented 10 years ago

From @khwilliamson

On 02/28/2014 12​:52 PM\, Karl Williamson wrote​:

They are still there in today's blead run on dromedary\, perl -V output below. In fact there are new failures. The output is attached. Some from mg.c and some from regcomp.c\, and some from cpan upstream. I will look into the regcomp ones.

I don't get a failure on my Ubuntu either\, but they are still there on Dromedary

We were talking on #irc about the advisability of doing regular smokes using valgrind. I think these results indicate that is a good idea. Matthew Horsfall's patch that he sent me\, and which I applied on dromedary allows valgrind to run in parallel\, cutting the test time to under 2 hours there.

p5pRT commented 10 years ago

From @tonycoz

On Mon Mar 03 12​:30​:11 2014\, public@​khwilliamson.com wrote​:

On 02/28/2014 12​:52 PM\, Karl Williamson wrote​:

They are still there in today's blead run on dromedary\, perl -V output below. In fact there are new failures. The output is attached. Some from mg.c and some from regcomp.c\, and some from cpan upstream. I will look into the regcomp ones.

I don't get a failure on my Ubuntu either\, but they are still there on Dromedary

We were talking on #irc about the advisability of doing regular smokes using valgrind. I think these results indicate that is a good idea. Matthew Horsfall's patch that he sent me\, and which I applied on dromedary allows valgrind to run in parallel\, cutting the test time to under 2 hours there.

This appears to be a limitation in valgrind in that it doesn't treat that memory as initialized when semctl() returns\, see​:

https://issues.apache.org/bugzilla/show_bug.cgi?id=53690

The following patch fixes the problem\, but I'm not sure if it's worth applying​:

Inline Patch ```diff diff --git a/doio.c b/doio.c index bdff84c..6275aae 100644 --- a/doio.c +++ b/doio.c @@ -2117,6 +2117,7 @@ Perl_do_ipcctl(pTHX_ I32 optype, SV **mark, SV **sp) { SvPV_force_nolen(astr); a = SvGROW(astr, infosize+1); + Zero(a, infosize, char); } else { Tony ```
p5pRT commented 10 years ago

From @tonycoz

On Tue Mar 04 21​:16​:17 2014\, tonyc wrote​:

This appears to be a limitation in valgrind in that it doesn't treat that memory as initialized when semctl() returns\, see​:

https://issues.apache.org/bugzilla/show_bug.cgi?id=53690

The system valgrind is 3.8.1 so I installed 3.9.0\, this displayed essentially the same errors​:

[tonyc@​dromedary-001 perl]$ ~/local/valgrind-3.9.0/bin/valgrind ./perl t/io/sem.t ==4728== Memcheck\, a memory error detector ==4728== Copyright (C) 2002-2013\, and GNU GPL'd\, by Julian Seward et al. ==4728== Using Valgrind-3.9.0 and LibVEX; rerun with -h for copyright info ==4728== Command​: ./perl t/io/sem.t ==4728== 1..7 ok 1 - acquired semaphore ok 2 - Initialize all 10 semaphores to zero ok 3 - Set semaphore 3 to 17 ok 4 - Get current semaphore values ok 5 - Make sure we get back statuses for all 10 semaphores ==4728== Conditional jump or move depends on uninitialised value(s) ==4728== at 0x4F9EF9​: S_uiv_2buf (sv.c​:2751) ==4728== by 0x506673​: Perl_sv_2pv_flags (sv.c​:2934) ==4728== by 0x518FDA​: Perl_sv_eq_flags (sv.c​:7574) ==4728== by 0x53D495​: Perl_pp_seq (pp.c​:2150) ==4728== by 0x4BF71A​: Perl_runops_debug (dump.c​:2425) ==4728== by 0x446702​: perl_run (perl.c​:2449) ==4728== by 0x41CEEB​: main (perlmain.c​:112) ==4728== ==4728== Conditional jump or move depends on uninitialised value(s) ==4728== at 0x4F9F35​: S_uiv_2buf (sv.c​:2760) ==4728== by 0x506673​: Perl_sv_2pv_flags (sv.c​:2934) ==4728== by 0x518FDA​: Perl_sv_eq_flags (sv.c​:7574) ==4728== by 0x53D495​: Perl_pp_seq (pp.c​:2150) ==4728== by 0x4BF71A​: Perl_runops_debug (dump.c​:2425) ==4728== by 0x446702​: perl_run (perl.c​:2449) ==4728== by 0x41CEEB​: main (perlmain.c​:112) ==4728== ==4728== Conditional jump or move depends on uninitialised value(s) ==4728== at 0x5191E0​: Perl_sv_eq_flags (sv.c​:7619) ==4728== by 0x53D495​: Perl_pp_seq (pp.c​:2150) ==4728== by 0x4BF71A​: Perl_runops_debug (dump.c​:2425) ==4728== by 0x446702​: perl_run (perl.c​:2449) ==4728== by 0x41CEEB​: main (perlmain.c​:112) ==4728== ==4728== Conditional jump or move depends on uninitialised value(s) ==4728== at 0x4F3B92​: Perl_pp_sassign (pp_hot.c​:223) ==4728== by 0x4BF71A​: Perl_runops_debug (dump.c​:2425) ==4728== by 0x446702​: perl_run (perl.c​:2449) ==4728== by 0x41CEEB​: main (perlmain.c​:112) ==4728== ==4728== Conditional jump or move depends on uninitialised value(s) ==4728== at 0x4FCF4E​: Perl_sv_setsv_flags (sv.c​:4085) ==4728== by 0x4F3BA3​: Perl_pp_sassign (pp_hot.c​:223) ==4728== by 0x4BF71A​: Perl_runops_debug (dump.c​:2425) ==4728== by 0x446702​: perl_run (perl.c​:2449) ==4728== by 0x41CEEB​: main (perlmain.c​:112) ==4728== ==4728== Use of uninitialised value of size 8 ==4728== at 0x4FCF79​: Perl_sv_setsv_flags (sv.c​:4095) ==4728== by 0x4F3BA3​: Perl_pp_sassign (pp_hot.c​:223) ==4728== by 0x4BF71A​: Perl_runops_debug (dump.c​:2425) ==4728== by 0x446702​: perl_run (perl.c​:2449) ==4728== by 0x41CEEB​: main (perlmain.c​:112) ...

The following patch fixes the problem\, but I'm not sure if it's worth applying​:

diff --git a/doio.c b/doio.c index bdff84c..6275aae 100644 --- a/doio.c +++ b/doio.c @​@​ -2117\,6 +2117\,7 @​@​ Perl_do_ipcctl(pTHX_ I32 optype\, SV **mark\, SV **sp) { SvPV_force_nolen(astr); a = SvGROW(astr\, infosize+1); + Zero(a\, infosize\, char); } else {

This continued to fix the error reports.

Tony

p5pRT commented 10 years ago

From @khwilliamson

On 03/05/2014 07​:46 PM\, Tony Cook via RT wrote​:

The following patch fixes the problem\, but I'm not sure if it's worth

applying​:

A reason some patch is worth applying is to keep this issue from coming up over and over and having to be reinvestigated.

p5pRT commented 10 years ago

From perl5-porters@perl.org

Karl Williamson wrote​:

A reason some patch is worth applying is to keep this issue from coming up over and over and having to be reinvestigated.

But this patch will cause a speed hit. Is it a good idea to have a valgrind-build mode? E.g.\, #ifdef VALGRIND?

p5pRT commented 10 years ago

From @tonycoz

On Thu Mar 06 11​:40​:51 2014\, perl5-porters@​perl.org wrote​:

Karl Williamson wrote​:

A reason some patch is worth applying is to keep this issue from coming up over and over and having to be reinvestigated.

Ideally this should be done with a suppression file\, preferably one specific to t/io/sem.t (two of the warnings are pretty generic)

But this patch will cause a speed hit. Is it a good idea to have a valgrind-build mode? E.g.\, #ifdef VALGRIND?

The problem is then we're valgrinding code that's different to what we send to users\, which could hide a real problem (or produce a real change in behaviour).

Reported to valgrind as https://bugs.kde.org/show_bug.cgi?id=331833

Tony

p5pRT commented 10 years ago

From @khwilliamson

On 03/06/2014 12​:40 PM\, Father Chrysostomos wrote​:

Karl Williamson wrote​:

A reason some patch is worth applying is to keep this issue from coming up over and over and having to be reinvestigated.

But this patch will cause a speed hit. Is it a good idea to have a valgrind-build mode? E.g.\, #ifdef VALGRIND?

Better than nothing is to add a comment at the place where a future investigator is likely to go first.

p5pRT commented 10 years ago

From @tonycoz

On Mon Mar 10 21​:27​:56 2014\, public@​khwilliamson.com wrote​:

Better than nothing is to add a comment at the place where a future investigator is likely to go first.

I thought about doing that\, but the problem is the error backtraces point outside of where the problem is actually occurring.

The only real solutions I see​:

1) zero the memory before calling semctl()

  This may lead to a different false-negative for valgrind - if the semctl() fails\, valgrind will treat the memory as initialized\, even though it's not initialized usefully.

2) use test script specific suppressions

  They need to be script specific since\, at least for this case the error is occurring in code that has nothing to do with semctl()\, suppressing this call stack globally could lead to suppressing other unrelated errors.

Tony