Skip Menu |
Report information
Id: 121366
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: khw <khw [at] cpan.org>
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: low
Type:
  • core
  • unknown
Perl Version: 5.17.1
Fixed In: (no value)



Subject: valgrind errors in t/op/vec.t
Download (untitled) / with headers
text/plain 909b
A bisect yields this: 032061d233a4bb16c1677ef64615bdb15de5b8a1 is the first bad commit commit 032061d233a4bb16c1677ef64615bdb15de5b8a1 Author: Brian Fraser <fraserbn@gmail.com> Date: Sat May 26 17:35:26 2012 -0300 Fix for [perl #9423] vec assignments generate 2 warnings Before this commit, this: vec(my $v,0,1) = 1; would've produced four warnings about uninitialized values; however, the ticket argued that these were spurious. This commit removes the warning in the case of lvalue vec, since it is similar to |=, but leaves it in place for rvalue vec. :100644 100644 1bd16b5e668fdbd0d0be725e26625fd93cbf4051 b2b6546d8cd32bfffa1c65112697a04de9560090 M doop.c :040000 040000 febb4034f8c7bc380a93193e5df0039f1e16c4d2 88120f8bce08edef4a05927b045a3a465087a857 M t bisect run success That took 793 seconds -- Karl Williamson
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.6k
On Mon Mar 03 11:28:21 2014, khw wrote: Show quoted text
> A bisect yields this: > > 032061d233a4bb16c1677ef64615bdb15de5b8a1 is the first bad commit > commit 032061d233a4bb16c1677ef64615bdb15de5b8a1 > Author: Brian Fraser <fraserbn@gmail.com> > Date: Sat May 26 17:35:26 2012 -0300
This is happening when using vec() to modify an undef sv, eg: $ valgrind -q ./perl -e 'vec($Foo, 0, 1) = 1' ==814== Conditional jump or move depends on uninitialised value(s) ==814== at 0x5190C7: Perl_sv_pvn_force_flags (sv.c:9505) ==814== by 0x580C9B: Perl_do_vecset (doop.c:927) ==814== by 0x4D30AD: Perl_magic_setvec (mg.c:2305) ==814== by 0x4CE522: Perl_mg_set (mg.c:279) ==814== by 0x4EB417: Perl_pp_sassign (pp_hot.c:223) ==814== by 0x4C42C9: Perl_runops_debug (dump.c:2425) ==814== by 0x44BEC4: perl_run (perl.c:2449) ==814== by 0x422934: main (perlmain.c:112) ==814== The error is on this line: Show quoted text
>>> if (s != SvPVX_const(sv)) { /* Almost, but not quite, sv_setpvn() */
if (SvROK(sv)) sv_unref(sv); where we examine sv->sv_u.svu_pv when it hasn't been initialized yet. The following change prevents the error and passes all tests (DEBUGGING): diff --git a/sv.c b/sv.c index dcb1d5e..4742c7d 100644 --- a/sv.c +++ b/sv.c @@ -9502,7 +9502,8 @@ Perl_sv_pvn_force_flags(pTHX_ SV *const sv, STRLEN *const if (lp) *lp = len; - if (s != SvPVX_const(sv)) { /* Almost, but not quite, sv_setpvn() */ + if (SvTYPE(sv) < SVt_PV || + s != SvPVX_const(sv)) { /* Almost, but not quite, sv_setpvn() */ if (SvROK(sv)) sv_unref(sv); SvUPGRADE(sv, SVt_PV); /* Never FALSE */ Tony
RT-Send-CC: perl5-porters [...] perl.org
On Sun Mar 23 21:15:44 2014, tonyc wrote: Show quoted text
> On Mon Mar 03 11:28:21 2014, khw wrote:
> > A bisect yields this: > > > > 032061d233a4bb16c1677ef64615bdb15de5b8a1 is the first bad commit > > commit 032061d233a4bb16c1677ef64615bdb15de5b8a1 > > Author: Brian Fraser <fraserbn@gmail.com> > > Date: Sat May 26 17:35:26 2012 -0300
> > This is happening when using vec() to modify an undef sv, eg: > > $ valgrind -q ./perl -e 'vec($Foo, 0, 1) = 1' > ==814== Conditional jump or move depends on uninitialised value(s) > ==814== at 0x5190C7: Perl_sv_pvn_force_flags (sv.c:9505) > ==814== by 0x580C9B: Perl_do_vecset (doop.c:927) > ==814== by 0x4D30AD: Perl_magic_setvec (mg.c:2305) > ==814== by 0x4CE522: Perl_mg_set (mg.c:279) > ==814== by 0x4EB417: Perl_pp_sassign (pp_hot.c:223) > ==814== by 0x4C42C9: Perl_runops_debug (dump.c:2425) > ==814== by 0x44BEC4: perl_run (perl.c:2449) > ==814== by 0x422934: main (perlmain.c:112) > ==814== > > The error is on this line: >
> >>> if (s != SvPVX_const(sv)) { /* Almost, but not quite, > >>> sv_setpvn() */
> if (SvROK(sv)) > sv_unref(sv); > > where we examine sv->sv_u.svu_pv when it hasn't been initialized yet. > > The following change prevents the error and passes all tests > (DEBUGGING): > > diff --git a/sv.c b/sv.c > index dcb1d5e..4742c7d 100644 > --- a/sv.c > +++ b/sv.c > @@ -9502,7 +9502,8 @@ Perl_sv_pvn_force_flags(pTHX_ SV *const sv, > STRLEN *const > if (lp) > *lp = len; > > - if (s != SvPVX_const(sv)) { /* Almost, but not quite, > sv_setpvn() */ > + if (SvTYPE(sv) < SVt_PV || > + s != SvPVX_const(sv)) { /* Almost, but not quite, > sv_setpvn() */ > if (SvROK(sv)) > sv_unref(sv); > SvUPGRADE(sv, SVt_PV); /* Never FALSE */ >
I wanted to test your patch but could not because it didn't apply cleanly using "patch -p1 < …". The problem seems to be that it is all whitespace, while the sources use a mixture of tabs and spaces. I can apply it manually and provide a new patch, but shouldn't the patch be attached? Regards, -- Shlomi Fish
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 2.3k
On Tue Mar 25 01:24:32 2014, shlomif wrote: Show quoted text
> On Sun Mar 23 21:15:44 2014, tonyc wrote:
> > On Mon Mar 03 11:28:21 2014, khw wrote:
> > > A bisect yields this: > > > > > > 032061d233a4bb16c1677ef64615bdb15de5b8a1 is the first bad commit > > > commit 032061d233a4bb16c1677ef64615bdb15de5b8a1 > > > Author: Brian Fraser <fraserbn@gmail.com> > > > Date: Sat May 26 17:35:26 2012 -0300
> > > > This is happening when using vec() to modify an undef sv, eg: > > > > $ valgrind -q ./perl -e 'vec($Foo, 0, 1) = 1' > > ==814== Conditional jump or move depends on uninitialised value(s) > > ==814== at 0x5190C7: Perl_sv_pvn_force_flags (sv.c:9505) > > ==814== by 0x580C9B: Perl_do_vecset (doop.c:927) > > ==814== by 0x4D30AD: Perl_magic_setvec (mg.c:2305) > > ==814== by 0x4CE522: Perl_mg_set (mg.c:279) > > ==814== by 0x4EB417: Perl_pp_sassign (pp_hot.c:223) > > ==814== by 0x4C42C9: Perl_runops_debug (dump.c:2425) > > ==814== by 0x44BEC4: perl_run (perl.c:2449) > > ==814== by 0x422934: main (perlmain.c:112) > > ==814== > > > > The error is on this line: > >
> > >>> if (s != SvPVX_const(sv)) { /* Almost, but not quite, > > >>> sv_setpvn() */
> > if (SvROK(sv)) > > sv_unref(sv); > > > > where we examine sv->sv_u.svu_pv when it hasn't been initialized yet. > > > > The following change prevents the error and passes all tests > > (DEBUGGING): > > > > diff --git a/sv.c b/sv.c > > index dcb1d5e..4742c7d 100644 > > --- a/sv.c > > +++ b/sv.c > > @@ -9502,7 +9502,8 @@ Perl_sv_pvn_force_flags(pTHX_ SV *const sv, > > STRLEN *const > > if (lp) > > *lp = len; > > > > - if (s != SvPVX_const(sv)) { /* Almost, but not quite, > > sv_setpvn() */ > > + if (SvTYPE(sv) < SVt_PV || > > + s != SvPVX_const(sv)) { /* Almost, but not quite, > > sv_setpvn() */ > > if (SvROK(sv)) > > sv_unref(sv); > > SvUPGRADE(sv, SVt_PV); /* Never FALSE */ > >
> > I wanted to test your patch but could not because it didn't apply > cleanly using "patch -p1 < …". The problem seems to be that it is all > whitespace, while the sources use a mixture of tabs and spaces. I can > apply it manually and provide a new patch, but shouldn't the patch be > attached?
The reworked patch is attached to this comment. All tests are successful here (Mageia Linux x86-64 5/Cauldron). Regards, -- Shlomi Fish
Subject: rt121366.diff
Download rt121366.diff
text/x-patch 464b
diff --git a/sv.c b/sv.c index dcb1d5e..9afe983 100644 --- a/sv.c +++ b/sv.c @@ -9502,7 +9502,8 @@ Perl_sv_pvn_force_flags(pTHX_ SV *const sv, STRLEN *const lp, const I32 flags) if (lp) *lp = len; - if (s != SvPVX_const(sv)) { /* Almost, but not quite, sv_setpvn() */ + if (SvTYPE(sv) < SVt_PV || + s != SvPVX_const(sv)) { /* Almost, but not quite, sv_setpvn() */ if (SvROK(sv)) sv_unref(sv); SvUPGRADE(sv, SVt_PV); /* Never FALSE */
From: Brian Fraser <fraserbn [...] gmail.com>
CC: Perl5 Porters Mailing List <perl5-porters [...] perl.org>
Subject: Re: [perl #121366] valgrind errors in t/op/vec.t
To: Brian Fraser via RT <perlbug-followup [...] perl.org>
Date: Tue, 25 Mar 2014 10:04:23 +0100
On Mon, Mar 24, 2014 at 5:15 AM, Tony Cook via RT <perlbug-followup@perl.org> wrote: Show quoted text
> On Mon Mar 03 11:28:21 2014, khw wrote:
>> A bisect yields this: >> >> 032061d233a4bb16c1677ef64615bdb15de5b8a1 is the first bad commit >> commit 032061d233a4bb16c1677ef64615bdb15de5b8a1 >> Author: Brian Fraser <fraserbn@gmail.com> >> Date: Sat May 26 17:35:26 2012 -0300
> > This is happening when using vec() to modify an undef sv, eg: > > $ valgrind -q ./perl -e 'vec($Foo, 0, 1) = 1' > ==814== Conditional jump or move depends on uninitialised value(s) > ==814== at 0x5190C7: Perl_sv_pvn_force_flags (sv.c:9505) > ==814== by 0x580C9B: Perl_do_vecset (doop.c:927) > ==814== by 0x4D30AD: Perl_magic_setvec (mg.c:2305) > ==814== by 0x4CE522: Perl_mg_set (mg.c:279) > ==814== by 0x4EB417: Perl_pp_sassign (pp_hot.c:223) > ==814== by 0x4C42C9: Perl_runops_debug (dump.c:2425) > ==814== by 0x44BEC4: perl_run (perl.c:2449) > ==814== by 0x422934: main (perlmain.c:112) > ==814== > > The error is on this line: >
>>>> if (s != SvPVX_const(sv)) { /* Almost, but not quite, sv_setpvn() */
> if (SvROK(sv)) > sv_unref(sv); > > where we examine sv->sv_u.svu_pv when it hasn't been initialized yet. > > The following change prevents the error and passes all tests (DEBUGGING): > > diff --git a/sv.c b/sv.c > index dcb1d5e..4742c7d 100644 > --- a/sv.c > +++ b/sv.c > @@ -9502,7 +9502,8 @@ Perl_sv_pvn_force_flags(pTHX_ SV *const sv, STRLEN *const > if (lp) > *lp = len; > > - if (s != SvPVX_const(sv)) { /* Almost, but not quite, sv_setpvn() */ > + if (SvTYPE(sv) < SVt_PV || > + s != SvPVX_const(sv)) { /* Almost, but not quite, sv_setpvn() */ > if (SvROK(sv)) > sv_unref(sv); > SvUPGRADE(sv, SVt_PV); /* Never FALSE */ > > Tony > > --- > via perlbug: queue: perl5 status: new > https://rt.perl.org/Ticket/Display.html?id=121366
Thanks for picking up this, Tony! I haven't been able to get valgrind or gdb to work on my Mac, so I've been stuck being useless for a while.
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 968b
On Sun Mar 23 21:15:44 2014, tonyc wrote: Show quoted text
> This is happening when using vec() to modify an undef sv, eg: > > $ valgrind -q ./perl -e 'vec($Foo, 0, 1) = 1' > ==814== Conditional jump or move depends on uninitialised value(s) > ==814== at 0x5190C7: Perl_sv_pvn_force_flags (sv.c:9505) > ==814== by 0x580C9B: Perl_do_vecset (doop.c:927) > ==814== by 0x4D30AD: Perl_magic_setvec (mg.c:2305) > ==814== by 0x4CE522: Perl_mg_set (mg.c:279) > ==814== by 0x4EB417: Perl_pp_sassign (pp_hot.c:223) > ==814== by 0x4C42C9: Perl_runops_debug (dump.c:2425) > ==814== by 0x44BEC4: perl_run (perl.c:2449) > ==814== by 0x422934: main (perlmain.c:112) > ==814== > > The error is on this line: >
> >>> if (s != SvPVX_const(sv)) { /* Almost, but not quite, > >>> sv_setpvn() */
> if (SvROK(sv)) > sv_unref(sv); > > where we examine sv->sv_u.svu_pv when it hasn't been initialized yet.
Pushed the fix as e14119056960dbe28537c9870667b5b920d9d731. Tony


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org