From @cpansprout

(as noted at <https://rt.cpan.org/Ticket/Display.html?id=66951>)

This is a real problem, but I don’t know if there is any realistic fix.

This module does $line != $line, where $line is tied, so it produces a different random value at each invocation. But now pp_ne calls get-magic up front, to check for overloading, and the comparison part skips it. Since get-magic works by writing the returned value to the scalar itself, by the time it gets to the comparison part, it’s just two pointers to the same scalar, which will always compare equal.

Since Perl_try_amagic_bin always calls magic just once if the two operands are the same, maybe we could modify binary operators that use try_amagic_bin to call get-magic once more on the operand if the two operands are the same.

Flags​:
  category=core
  severity=high

Site configuration information for perl 5.13.11​:

Configured by sprout at Thu Mar 24 14​:02​:46 PDT 2011.

Summary of my perl5 (revision 5 version 13 subversion 11) configuration​:
  Snapshot of​: cc13eef
  Platform​:
  osname=darwin, osvers=10.5.0, archname=darwin-thread-multi-2level
  uname='darwin pint.local 10.5.0 darwin kernel version 10.5.0​: fri nov 5 23​:20​:39 pdt 2010; root​:xnu-1504.9.17~1release_i386 i386 '
  config_args='-Dusedevel -de -Duseithreads -Doptimize=-g'
  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 ='-fno-common -DPERL_DARWIN -no-cpp-precomp -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include',
  optimize='-g',
  cppflags='-no-cpp-precomp -fno-common -DPERL_DARWIN -no-cpp-precomp -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
  ccversion='', gccversion='4.2.1 (Apple Inc. build 5664)', gccosandvers=''
  intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
  ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
  alignbytes=8, prototype=define
  Linker and Libraries​:
  ld='env MACOSX_DEPLOYMENT_TARGET=10.3 cc', ldflags =' -fstack-protector -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib
  libs=-ldbm -ldl -lm -lutil -lc
  perllibs=-ldl -lm -lutil -lc
  libc=/usr/lib/libc.dylib, so=dylib, useshrplib=false, libperl=libperl.a
  gnulibc_version=''
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=bundle, d_dlsymun=undef, ccdlflags=' '
  cccdlflags=' ', lddlflags=' -bundle -undefined dynamic_lookup -L/usr/local/lib -fstack-protector'

Locally applied patches​:
 

@​INC for perl 5.13.11​:
  /usr/local/lib/perl5/site_perl/5.13.11/darwin-thread-multi-2level
  /usr/local/lib/perl5/site_perl/5.13.11
  /usr/local/lib/perl5/5.13.11/darwin-thread-multi-2level
  /usr/local/lib/perl5/5.13.11
  /usr/local/lib/perl5/site_perl
  .

Environment for perl 5.13.11​:
  DYLD_LIBRARY_PATH (unset)
  HOME=/Users/sprout
  LANG=en_US.UTF-8
  LANGUAGE (unset)
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
  PATH=/usr/bin​:/bin​:/usr/sbin​:/sbin​:/usr/local/bin​:/usr/X11/bin​:/usr/local/bin
  PERL_BADLANG (unset)
  SHELL=/bin/bash

From @obra

[this ticket is now a 5.14 blocker]

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

From @cpansprout

And Tie-DataUUID-1.00.

From @iabyn

On Sun, Apr 03, 2011 at 04​:37​:44PM -0700, Father Chrysostomos wrote​:

(as noted at <https://rt.cpan.org/Ticket/Display.html?id=66951>)

This is a real problem, but I don’t know if there is any realistic fix.

This module does $line != $line, where $line is tied, so it produces a
different random value at each invocation. But now pp_ne calls get-magic
up front, to check for overloading, and the comparison part skips it.
Since get-magic works by writing the returned value to the scalar
itself, by the time it gets to the comparison part, it’s just two
pointers to the same scalar, which will always compare equal.

Since Perl_try_amagic_bin always calls magic just once if the two
operands are the same, maybe we could modify binary operators that use
try_amagic_bin to call get-magic once more on the operand if the two
operands are the same.

Yuk. Yes, this is a real regression since 5.13.1, and may affect up to 40
binary ops.

For those following at home, the basic change is that where the same SV is
on both sides of a binary op, e.g. $foo . $foo, then get magic is now only
called once; so for example if you have sub FETCH {$i++} then the above
expression evaluates to "11" now rather than "12". Except it doesn't​: as
it happens, concat and add seem to be ok. So it's an unknown number of
ops, including pp_ne and probably others, but not concat or add.

To fix this, new tests will need to be added for all 40 ops, and the
code of each op carefully audited and then possibly fixed to make sure all
paths do the right amount of mg getting.

This will probably take me a couple of days (calendar), and introduces a
risk of breakage.
Its very late in the day to be doing this sort of thing, but on the other
hand it is a significant regression, albeit in an edge case, so I'd vote
for it.

Note that reverting the commit that broke it
(6f1401d) and its followups would be
hard, since quite a bit of code cleanup has happened around that area
since then. Reverting it would probably also break things.

--
Technology is dominated by two types of people​: those who understand what
they do not manage, and those who manage what they do not understand.

From @cpansprout

On Mon Apr 04 15​:32​:06 2011, davem wrote​:

On Sun, Apr 03, 2011 at 04​:37​:44PM -0700, Father Chrysostomos wrote​:

(as noted at <https://rt.cpan.org/Ticket/Display.html?id=66951>)

This is a real problem, but I don’t know if there is any realistic fix.

This module does $line != $line, where $line is tied, so it produces a
different random value at each invocation. But now pp_ne calls get-magic
up front, to check for overloading, and the comparison part skips it.
Since get-magic works by writing the returned value to the scalar
itself, by the time it gets to the comparison part, it’s just two
pointers to the same scalar, which will always compare equal.

Since Perl_try_amagic_bin always calls magic just once if the two
operands are the same, maybe we could modify binary operators that use
try_amagic_bin to call get-magic once more on the operand if the two
operands are the same.

Yuk. Yes, this is a real regression since 5.13.1, and may affect up to 40
binary ops.

For those following at home, the basic change is that where the same SV is
on both sides of a binary op, e.g. $foo . $foo, then get magic is now only
called once; so for example if you have sub FETCH {$i++} then the above
expression evaluates to "11" now rather than "12". Except it doesn't​: as
it happens, concat and add seem to be ok. So it's an unknown number of
ops, including pp_ne and probably others, but not concat or add.

To fix this, new tests will need to be added for all 40 ops, and the
code of each op carefully audited and then possibly fixed to make sure all
paths do the right amount of mg getting.

I’ve just push to-do tests with commit b04496f (but I got the bug
number wrong).

This will probably take me a couple of days (calendar), and introduces a
risk of breakage.
Its very late in the day to be doing this sort of thing, but on the other
hand it is a significant regression, albeit in an edge case, so I'd vote
for it.

Note that reverting the commit that broke it
(6f1401d) and its followups would be
hard, since quite a bit of code cleanup has happened around that area
since then. Reverting it would probably also break things.

From [Unknown Contact. See original ticket]

On Mon Apr 04 15​:32​:06 2011, davem wrote​:

On Sun, Apr 03, 2011 at 04​:37​:44PM -0700, Father Chrysostomos wrote​:

(as noted at <https://rt.cpan.org/Ticket/Display.html?id=66951>)

This is a real problem, but I don’t know if there is any realistic fix.

This module does $line != $line, where $line is tied, so it produces a
different random value at each invocation. But now pp_ne calls get-magic
up front, to check for overloading, and the comparison part skips it.
Since get-magic works by writing the returned value to the scalar
itself, by the time it gets to the comparison part, it’s just two
pointers to the same scalar, which will always compare equal.

Since Perl_try_amagic_bin always calls magic just once if the two
operands are the same, maybe we could modify binary operators that use
try_amagic_bin to call get-magic once more on the operand if the two
operands are the same.

Yuk. Yes, this is a real regression since 5.13.1, and may affect up to 40
binary ops.

For those following at home, the basic change is that where the same SV is
on both sides of a binary op, e.g. $foo . $foo, then get magic is now only
called once; so for example if you have sub FETCH {$i++} then the above
expression evaluates to "11" now rather than "12". Except it doesn't​: as
it happens, concat and add seem to be ok. So it's an unknown number of
ops, including pp_ne and probably others, but not concat or add.

To fix this, new tests will need to be added for all 40 ops, and the
code of each op carefully audited and then possibly fixed to make sure all
paths do the right amount of mg getting.

I’ve just push to-do tests with commit b04496f (but I got the bug
number wrong).

This will probably take me a couple of days (calendar), and introduces a
risk of breakage.
Its very late in the day to be doing this sort of thing, but on the other
hand it is a significant regression, albeit in an edge case, so I'd vote
for it.

Note that reverting the commit that broke it
(6f1401d) and its followups would be
hard, since quite a bit of code cleanup has happened around that area
since then. Reverting it would probably also break things.

From @cpansprout

I’ve now fixed this with commit 75ea7a1.

See http​://perl5.git.perl.org/perl.git/commitdiff/75ea7a1 for more
detail than you want to know.

From [Unknown Contact. See original ticket]

I’ve now fixed this with commit 75ea7a1.

See http​://perl5.git.perl.org/perl.git/commitdiff/75ea7a1 for more
detail than you want to know.

@cpansprout - Status changed from 'open' to 'resolved'

From @iabyn

On Thu, Apr 07, 2011 at 11​:04​:54PM -0700, Father Chrysostomos via RT wrote​:

I’ve now fixed this with commit 75ea7a1.

See http​://perl5.git.perl.org/perl.git/commitdiff/75ea7a1 for more
detail than you want to know.

Thanks for this, looks good.
I added some more tests with

  3216d30
  9c9a250

to include the '^' operator, and to cover mutator op variants (e.g. +=).

--
Wesley Crusher gets beaten up by his classmates for being a smarmy git,
and consequently has a go at making some friends of his own age for a
change.
  -- Things That Never Happen in "Star Trek" #18