Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Perl: porters

[perl #68192] Tied scalar numeric FETCH corruption after STORE()ing a reference

 

 

Perl porters RSS feed   Index | Next | Previous | View Threaded


perlbug-followup at perl

Aug 4, 2009, 4:57 PM

Post #1 of 12 (462 views)
Permalink
[perl #68192] Tied scalar numeric FETCH corruption after STORE()ing a reference

# New Ticket Created by David Taylor
# Please include the string: [perl #68192]
# in the subject line of all future correspondence about this issue.
# <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=68192 >


With a tied scalar with a FETCH() method that returns a number, if a
reference is STORED()ed in the tied scalar and then a FETCH()ed value
is interpreted in a numeric context, the value that FETCH() returns is
ignored, and instead you get back the STORE()ed reference as an
integer.

5.8.9, 5.10.0 and blead all have the bug, 5.6.2 does not.

Demo:

# BEGIN SCRIPT

sub TIESCALAR { bless {}, __PACKAGE__ };
sub STORE {};
sub FETCH { 123456 };

my $foo;
tie $foo, __PACKAGE__;

my $a = [1234567];

my $x = 0 + $foo;
$foo = $a;
my $y = 0 + $foo;
my $yhex = sprintf '0x%x', $y;

print "x = $x, y = $y, a = $a, yhex = $yhex\n";

# END SCRIPT

Under 5.10.0 this prints:
x = 123456, y = 148755408, a = ARRAY(0x8ddd3d0), yhex = 0x8ddd3d0


$ perl -V
Summary of my perl5 (revision 5 version 10 subversion 0) configuration:
Platform:
osname=linux, osvers=2.6.24-23-server, archname=i486-linux-gnu-thread-multi
uname='linux rothera 2.6.24-23-server #1 smp wed apr 1 22:22:14
utc 2009 i686 gnulinux '
config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN
-Dcccdlflags=-fPIC -Darchname=i486-linux-gnu -Dprefix=/usr
-Dprivlib=/usr/share
/perl/5.10 -Darchlib=/usr/lib/perl/5.10 -Dvendorprefix=/usr
-Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5
-Dsiteprefix=/usr/local -Ds
itelib=/usr/local/share/perl/5.10.0
-Dsitearch=/usr/local/lib/perl/5.10.0 -Dman1dir=/usr/share/man/man1
-Dman3dir=/usr/share/man/man3 -Dsiteman1
dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Dman1ext=1
-Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh
-Ud_ualarm -Uusesfio -Uusenm -DDEBUGGING=-g -Doptimize=-O2
-Duseshrplib -Dlibperl=libperl.so.5.10.0 -Dd_dosuid -des'
hint=recommended, useposix=true, d_sigaction=define
useithreads=define, usemultiplicity=define
useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
use64bitint=undef, use64bitall=undef, uselongdouble=undef
usemymalloc=n, bincompat5005=undef
Compiler:
cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN
-fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE
-D_FILE_OFFSET_BITS=64',
optimize='-O2 -g',
cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing
-pipe -I/usr/local/include'
ccversion='', gccversion='4.3.2', gccosandvers=''
intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t',
lseeksize=8
alignbytes=4, prototype=define
Linker and Libraries:
ld='cc', ldflags =' -L/usr/local/lib'
libpth=/usr/local/lib /lib /usr/lib /usr/lib64
libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
perllibs=-ldl -lm -lpthread -lc -lcrypt
libc=/lib/libc-2.8.90.so, so=so, useshrplib=true, libperl=libperl.so.5.10.0
gnulibc_version='2.8.90'
Dynamic Linking:
dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
cccdlflags='-fPIC', lddlflags='-shared -O2 -g -L/usr/local/lib'


Characteristics of this binary (from libperl):
Compile-time options: MULTIPLICITY PERL_DONT_CREATE_GVSV
PERL_IMPLICIT_CONTEXT PERL_MALLOC_WRAP USE_ITHREADS
USE_LARGE_FILES USE_PERLIO USE_REENTRANT_API
Built under linux
Compiled at Jun 26 2009 19:08:22
@INC:
/etc/perl
/usr/local/lib/perl/5.10.0
/usr/local/share/perl/5.10.0
/usr/lib/perl5
/usr/share/perl5
/usr/lib/perl/5.10
/usr/share/perl/5.10
/usr/local/lib/site_perl
.


davidnicol at gmail

Aug 5, 2009, 7:14 AM

Post #2 of 12 (442 views)
Permalink
Re: [perl #68192] Tied scalar numeric FETCH corruption after STORE()ing a reference [In reply to]

the following has not been tested; just edited the demo in the report


{
# Tests for bug #68192
sub bug68192::TIESCALAR { bless {}, 'bug68192' };
sub bug68192::STORE {};
sub bug68192::FETCH { 123456 };
tie my $foo, 'bug68192';
is( 0+$foo, 123456, "#68192 unitialized scalar fetch works" ); # RECITAL
my $a = [1234567]; $foo = $a;
is( 0+$foo, 123456, "#68192 after storing reference" ); # FAILS
}


sprout at cpan

Nov 15, 2009, 1:05 PM

Post #3 of 12 (387 views)
Permalink
Re: [perl #68192] Tied scalar numeric FETCH corruption after STORE()ing a reference [In reply to]

This is a regression in perl 5.10.0, caused by http://perl5.git.perl.org/perl.git/commitdiff/800401ee2a8a5a67ef478227b68426cf701d0116
. The attached patch fixes this. I’m not sure whether what I’ve done
to sv_2nv is correct. Could someone please review it?

If anyone needs a workaround for this bug in the mean time, I’ve added
fix_tie to Tie::Util.
Attachments: open_mYOANO9w.txt (21.0 KB)


sprout at cpan

Dec 13, 2009, 12:58 PM

Post #4 of 12 (372 views)
Permalink
Re: [perl #68192] Tied scalar numeric FETCH corruption after STORE()ing a reference [In reply to]

On Nov 15, 2009, at 1:05 PM, Father Chrysostomos wrote:

> This is a regression in perl 5.10.0, caused by http://perl5.git.perl.org/perl.git/commitdiff/800401ee2a8a5a67ef478227b68426cf701d0116
> . The attached patch fixes this. I¢m not sure whether what I¢ve done
> to sv_2nv is correct. Could someone please review it?

Attached is the same patch, updated for 5.11.3.

BTW, here is another symptom:
$ perl -le' use bigint; select+(select(FOO), $|=1)[0]; print $|+0'
Can't call method "copy" without a package or object reference at /
System/Library/Perl/5.10.1/Math/BigInt.pm line 107.
Attachments: open_GpX66ecX.txt (20.9 KB)


sprout at cpan

Jan 17, 2010, 2:26 PM

Post #5 of 12 (338 views)
Permalink
Re: [perl #68192] Tied scalar numeric FETCH corruption after STORE()ing a reference [In reply to]

On Dec 13, 2009, at 12:58 PM, Father Chrysostomos wrote:

>
> On Nov 15, 2009, at 1:05 PM, Father Chrysostomos wrote:
>
>> This is a regression in perl 5.10.0, caused by http://perl5.git.perl.org/perl.git/commitdiff/800401ee2a8a5a67ef478227b68426cf701d0116
>> . The attached patch fixes this. I¢m not sure whether what I¢ve
>> done to sv_2nv is correct. Could someone please review it?
>
> Attached is the same patch, updated for 5.11.3.

Updated for 5.11.4.
Attachments: open_i6z0dl7D.txt (20.9 KB)


sprout at cpan

May 16, 2010, 12:00 PM

Post #6 of 12 (293 views)
Permalink
Re: [perl #68192] Tied scalar numeric FETCH corruption after STORE()ing a reference [In reply to]

On Jan 17, 2010, at 2:26 PM, Father Chrysostomos wrote:

>
> On Dec 13, 2009, at 12:58 PM, Father Chrysostomos wrote:
>
>>
>> On Nov 15, 2009, at 1:05 PM, Father Chrysostomos wrote:
>>
>>> This is a regression in perl 5.10.0, caused by http://perl5.git.perl.org/perl.git/commitdiff/800401ee2a8a5a67ef478227b68426cf701d0116
>>> . The attached patch fixes this. I’m not sure whether what I’ve
>>> done to sv_2nv is correct. Could someone please review it?
>>
>> Attached is the same patch, updated for 5.11.3.
>
> Updated for 5.11.4.

OK, let’s try this again:

sv_2num doesn’t take get-magic into account, so a tied scalar
containing a reference is not treated as tied, at least not by numeric
ops.

In trying to fix this, I found that most ops check for overloading
before get-magic, which causes the wrong overload methods to be used.
Fixing this problem fixes the original bug ‘for free’, so I followed
this approach.

Every pp_ function that does overloading needs to call get-magic up
front, before checking for overloading. This means they need to call
the _nomg variations of the Sv*V macros (to avoid double get-magic).
Every function that uses SvIV_please needs the same treatment, so I
modified SvIV_please to be non-magical.

Attached is a new patch, updated for blead (as of May, 2010). This one
no longer has sv_2nv_flags, since Dave Mitchell added that recently
when fixing a similar bug in pp_sort.

Attached also is a file (not a patch) with perldelta entries, one of
which is applicable even without my patch.
Attachments: fourth attempt.diff (16.4 KB)
  perldelta entries.text (0.63 KB)


davem at iabyn

May 17, 2010, 4:03 AM

Post #7 of 12 (292 views)
Permalink
Re: [perl #68192] Tied scalar numeric FETCH corruption after STORE()ing a reference [In reply to]

On Sun, May 16, 2010 at 12:00:55PM -0700, Father Chrysostomos wrote:
>
> On Jan 17, 2010, at 2:26 PM, Father Chrysostomos wrote:
>
>>
>> On Dec 13, 2009, at 12:58 PM, Father Chrysostomos wrote:
>>
>>>
>>> On Nov 15, 2009, at 1:05 PM, Father Chrysostomos wrote:
>>>
>>>> This is a regression in perl 5.10.0, caused by
>>>> http://perl5.git.perl.org/perl.git/commitdiff/800401ee2a8a5a67ef478227b68426cf701d0116
>>>> . The attached patch fixes this. I’m not sure whether what I’ve
>>>> done to sv_2nv is correct. Could someone please review it?
>>>
>>> Attached is the same patch, updated for 5.11.3.
>>
>> Updated for 5.11.4.
>
> OK, let’s try this again:
>
> sv_2num doesn’t take get-magic into account, so a tied scalar containing
> a reference is not treated as tied, at least not by numeric ops.
>
> In trying to fix this, I found that most ops check for overloading
> before get-magic, which causes the wrong overload methods to be used.
> Fixing this problem fixes the original bug ‘for free’, so I followed
> this approach.

Urgh. I've just spent the last week working in a similar area, although
I've been working and making all overloadable ops respect magic, not just
numeric ones. I've also been reorganising things to reduce the binary and
size and make things faster. I'm not sure yet how much overlap there is
between our two efforts :-(. I'll look into this soon.

--
Hofstadter's Law: It always takes longer than you expect, even when you
take into account Hofstadter's Law.


davem at iabyn

May 21, 2010, 6:44 AM

Post #8 of 12 (286 views)
Permalink
Re: [perl #68192] Tied scalar numeric FETCH corruption after STORE()ing a reference [In reply to]

On Mon, May 17, 2010 at 12:03:41PM +0100, Dave Mitchell wrote:
> On Sun, May 16, 2010 at 12:00:55PM -0700, Father Chrysostomos wrote:
> > OK, let’s try this again:
> >
> > sv_2num doesn’t take get-magic into account, so a tied scalar containing
> > a reference is not treated as tied, at least not by numeric ops.
> >
> > In trying to fix this, I found that most ops check for overloading
> > before get-magic, which causes the wrong overload methods to be used.
> > Fixing this problem fixes the original bug ‘for free’, so I followed
> > this approach.
>
> Urgh. I've just spent the last week working in a similar area, although
> I've been working and making all overloadable ops respect magic, not just
> numeric ones. I've also been reorganising things to reduce the binary and
> size and make things faster. I'm not sure yet how much overlap there is
> between our two efforts :-(. I'll look into this soon.

I've now applied my commit shown below. Its a more general fix than
yours, since it deals with all overloaded ops not just the basic numeric
ones. However, your suggested patch was very useful, since it hadn't
occurred to me to include checks for how many times FETCH was called, and
it also showed me some ones I'd missed (like dPOPnv). So thanks!

The other main thing I did was to take the opportunity to make the code
more efficient. The macros formerly at the start of each op, such as
tryAMAGICbin(add,opASSIGN) incorporated a big blob of similar code into
each function. I've now put most of that into separate helper functions,
with a simple "are any of the args ROK/GMG" test on whether to call the
helper function:

#define tryAMAGICbin_MG(method, flags) STMT_START { \
if ( ((SvFLAGS(TOPm1s)|SvFLAGS(TOPs)) & (SVf_ROK|SVs_GMG)) \
&& Perl_try_amagic_bin(aTHX_ method, flags)) \
return NORMAL; \
} STMT_END


This means that simple data (not tied, not overloaded etc)
will take a quicker code path. As a demonstration, the code below (which
heavily tests pp_add with simple args) is reduced from (lowest time of 5
runs):

elapsed=6.987520

to

elapsed=5.904883

a saving of 15%.

#!/usr/bin/perl -w

use Time::HiRes qw(gettimeofday tv_interval);


$t0 = [gettimeofday];
my $x = 0;
for (1..5_000_000) {
$x+= 1;
$x+= 1;
$x+= 1;
$x+= 1;
$x+= 1;
$x+= 1;
$x+= 1;
$x+= 1;
$x+= 1;
$x+= 1;
$x+= 1;
$x+= 1;
$x+= 1;
$x+= 1;
$x+= 1;
$x+= 1;
$x+= 1;
$x+= 1;
$x+= 1;
$x+= 1;
}

printf "elapsed=%f\n", tv_interval($t0);


commit 6f1401dc2acd2a2b85df22b0a74e5f7e6e0a33aa
Author: David Mitchell <davem [at] iabyn>
AuthorDate: Fri May 21 14:18:21 2010 +0100
Commit: David Mitchell <davem [at] iabyn>
CommitDate: Fri May 21 14:18:21 2010 +0100

make overload respect get magic

In most places, ops checked their args for overload *before* doing
mg_get(). This meant that, among other issues, tied vars that
returned overloaded objects wouldn't trigger calling the
overloaded method. (Actually, for tied and arrays and hashes, it
still often would since mg_get gets called beforehand in rvalue
context).

This patch does the following:

Makes sure get magic is called first.

Moves most of the overload code formerly included by macros at the
start of each pp function into the separate helper functions
Perl_try_amagic_bin, Perl_try_amagic_un, S_try_amagic_ftest,
with 3 new wrapper macros:
tryAMAGICbin_MG, tryAMAGICun_MG, tryAMAGICftest_MG.
This made the code 3800 bytes smaller.

Makes sure that FETCH is not called multiple times. Much of this
bit was helped by some earlier work from Father Chrysostomos.

Added new functions and macros sv_inc_nomg(), sv_dec_nomg(),
dPOPnv_nomg, dPOPXiirl_ul_nomg, dPOPTOPnnrl_nomg, dPOPTOPiirl_ul_nomg
dPOPTOPiirl_nomg, SvIV_please_nomg, SvNV_nomg (again, some of
these were based on Father Chrysostomos's work).

Fixed the list version of the repeat operator (x): it now only
calls overloaded methods for the scalar version:
(1,2,$overloaded) x 10
no longer erroneously calls
x_method($overloaded,10))

The only thing I haven't checked/fixed yet is overloading the
iterator operator, <>.


Affected files ...

M embed.fnc
M embed.h
M global.sym
M gv.c
M lib/overload.t
M pp.c
M pp.h
M pp_ctl.c
M pp_hot.c
M pp_sys.c
M proto.h
M sv.c
M sv.h


--
Never work with children, animals, or actors.


sprout at cpan

May 23, 2010, 1:02 PM

Post #9 of 12 (268 views)
Permalink
Re: [perl #68192] Tied scalar numeric FETCH corruption after STORE()ing a reference [In reply to]

On May 17, 2010, at 4:03 AM, Dave Mitchell wrote:

> Urgh. I've just spent the last week working in a similar area,
> although
> I've been working and making all overloadable ops respect magic, not
> just
> numeric ones. I've also been reorganising things to reduce the
> binary and
> size and make things faster. I'm not sure yet how much overlap there
> is
> between our two efforts :-(. I'll look into this soon.

I was going to fix all of them, but the first two patches I submitted
(this ticket and 72144) seemed to go unnoticed.

Anyway, I’m glad to see it’s fixed.


davem at iabyn

May 25, 2010, 5:02 AM

Post #10 of 12 (271 views)
Permalink
Re: [perl #68192] Tied scalar numeric FETCH corruption after STORE()ing a reference [In reply to]

On Sun, May 23, 2010 at 01:02:17PM -0700, Father Chrysostomos wrote:
>
> On May 17, 2010, at 4:03 AM, Dave Mitchell wrote:
>
>> Urgh. I've just spent the last week working in a similar area,
>> although
>> I've been working and making all overloadable ops respect magic, not
>> just
>> numeric ones. I've also been reorganising things to reduce the binary
>> and
>> size and make things faster. I'm not sure yet how much overlap there
>> is
>> between our two efforts :-(. I'll look into this soon.
>
> I was going to fix all of them, but the first two patches I submitted
> (this ticket and 72144) seemed to go unnoticed.
>
> Anyway, I’m glad to see it’s fixed.

As you can see in my follow-up to #72144, its wasn't all fixed, so your
contribution was still useful!

To avoid any overlap going forward, Are you aware of any further issues
with overloading that you still intend to work on?

As regards patches going unnoticed, this is more a reflection of the fact
that we as committers are a bit disorganised than any desire to ignore
your fixes. So please, please, keep the patches coming. If after a while
no-one has responded, then its ok to to follow up with a reminder (and
keep reminding if we keep ignoring you).

Note also that RT has recently been changed to flag patches as such, so
it should be easier for committers to locate them in future, viz:

http://rt.perl.org/rt3/NoAuth/perl5/List.html?Field=Type&Value=Patch

--
Counsellor Troi states something other than the blindingly obvious.
-- Things That Never Happen in "Star Trek" #16


sprout at cpan

May 30, 2010, 2:17 PM

Post #11 of 12 (257 views)
Permalink
Re: [perl #68192] Tied scalar numeric FETCH corruption after STORE()ing a reference [In reply to]

On May 25, 2010, at 5:02 AM, Dave Mitchell wrote:

> On Sun, May 23, 2010 at 01:02:17PM -0700, Father Chrysostomos wrote:
>>
>> On May 17, 2010, at 4:03 AM, Dave Mitchell wrote:
>>
>>> Urgh. I've just spent the last week working in a similar area,
>>> although
>>> I've been working and making all overloadable ops respect magic, not
>>> just
>>> numeric ones. I've also been reorganising things to reduce the
>>> binary
>>> and
>>> size and make things faster. I'm not sure yet how much overlap there
>>> is
>>> between our two efforts :-(. I'll look into this soon.
>>
>> I was going to fix all of them, but the first two patches I submitted
>> (this ticket and 72144) seemed to go unnoticed.
>>
>> Anyway, I’m glad to see it’s fixed.
>
> As you can see in my follow-up to #72144, its wasn't all fixed, so
> your
> contribution was still useful!
>
> To avoid any overlap going forward, Are you aware of any further
> issues
> with overloading that you still intend to work on?

Since you asked, I had a quick look and saw that you missed readline
and glob. I’ll fix those, today if possible.

Also, have you seen ticket 71286? (I don’t plan to work on it, but
there is a patch there.)


davem at iabyn

May 30, 2010, 4:30 PM

Post #12 of 12 (257 views)
Permalink
Re: [perl #68192] Tied scalar numeric FETCH corruption after STORE()ing a reference [In reply to]

On Sun, May 30, 2010 at 02:17:15PM -0700, Father Chrysostomos wrote:
> Since you asked, I had a quick look and saw that you missed readline and
> glob. I’ll fix those, today if possible.

I had deliberately left those two, as the glob version is currently broken
in a major way - <$complex_expression> completely by-passes pp_glob and
calls &CORE::GLOBAL::glob instead:

[davem [at] pigeo bleed]$ cone -e '<$a[0]>'
1 <0> enter
2 <;> nextstate(main 59 -e:1) v:{
3 <0> pushmark s
4 <#> gv[*a] s
5 <1> rv2av sKR/1
6 <$> const[IV 0] s
7 <2> aelem sKM/LVDEFER,2
8 <$> const[IV 0] sM
9 <#> gv[*CORE::GLOBAL::glob] s
a <1> entersub[t3] vKS/TARG,1
b <@> leave[1 ref] vKP/REFC

So I was intending to discuss at some point with p5p what the correct
semantics of <...> should be as regards overloading.

>
> Also, have you seen ticket 71286? (I don’t plan to work on it, but there
> is a patch there.)

Well, I'm still officially working on tie bugs rather than overload bugs;
I only got involved in overload since the ticket was also tie related.

--
Indomitable in retreat, invincible in advance, insufferable in victory
-- Churchill on Montgomery

Perl porters RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.