Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

valgrind errors in pp_seq dealing with semaphores #13626

Open
p5pRT opened this issue Feb 27, 2014 · 13 comments
Open

valgrind errors in pp_seq dealing with semaphores #13626

p5pRT opened this issue Feb 27, 2014 · 13 comments

Comments

@p5pRT
Copy link

p5pRT commented Feb 27, 2014

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

Searchable as RT121335$

@p5pRT
Copy link
Author

p5pRT commented Feb 27, 2014

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
Copy link
Author

p5pRT commented Feb 28, 2014

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
Copy link
Author

p5pRT commented Feb 28, 2014

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

@p5pRT
Copy link
Author

p5pRT commented Feb 28, 2014

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​: b838857
  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
Copy link
Author

p5pRT commented Feb 28, 2014

From @khwilliamson

valgrind.out

@p5pRT
Copy link
Author

p5pRT commented Mar 3, 2014

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
Copy link
Author

p5pRT commented Mar 5, 2014

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 --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
Copy link
Author

p5pRT commented Mar 6, 2014

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
Copy link
Author

p5pRT commented Mar 6, 2014

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
Copy link
Author

p5pRT commented Mar 6, 2014

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
Copy link
Author

p5pRT commented Mar 6, 2014

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
Copy link
Author

p5pRT commented Mar 11, 2014

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
Copy link
Author

p5pRT commented Mar 25, 2014

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants