Perl / perl5

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

Storable::freeze mutates UVs into PVs #19296

Open karenetheridge opened 2 years ago

karenetheridge commented 2 years ago

repro case:

perl -MStorable -MDevel::Peek  -wle'my $value = 2**63; Dump($value); print ""; my $frozen = Storable::freeze(\$value); Dump($value)'

SV = IV(0x7faaec825840) at 0x7faaec825850
  REFCNT = 1
  FLAGS = (IOK,pIOK,IsUV)
  UV = 9223372036854775808

SV = PVIV(0x7faaec826c20) at 0x7faaec825850
  REFCNT = 1
  FLAGS = (IOK,POK,pIOK,pPOK,IsUV)
  UV = 9223372036854775808
  PV = 0x7faaec40cfa0 "9223372036854775808"\0
  CUR = 19
  LEN = 21

expected: Storable should not mutate its source value.

This is with Storable 3.25 on perl 5.35.7, OSX (darwin), with ivsize=8.

karenetheridge commented 2 years ago

This happens deep in the object too, so it's not possible to work around this by simply saving a copy before freezing it:

$; perl -MStorable -MDevel::Peek  -wle'my $value = { foo => 9223372036854775808 }; Dump($value->{foo}); print ""; my $frozen = Storable::freeze(\$value); Dump($value->{foo})'
SV = IV(0x7fc60900af80) at 0x7fc60900af90
  REFCNT = 1
  FLAGS = (IOK,pIOK,IsUV)
  UV = 9223372036854775808

SV = PVIV(0x7fc609026c20) at 0x7fc60900af90
  REFCNT = 1
  FLAGS = (IOK,POK,pIOK,pPOK,IsUV)
  UV = 9223372036854775808
  PV = 0x7fc608c0cf40 "9223372036854775808"\0
  CUR = 19
  LEN = 21
karenetheridge commented 2 years ago

And going through a full freeze-thaw cycle mutates the flags even more, stripping the UV flag:

perl -MStorable -MDevel::Peek -wle'my $value = 9223372036854775808; Dump($value); print ""; my $new = ${Storable::dclone(\$value)}; Dump($new)'

SV = IV(0x7fe07780dee8) at 0x7fe07780def8
  REFCNT = 1
  FLAGS = (IOK,pIOK,IsUV)
  UV = 9223372036854775808

SV = PV(0x7fe07800b080) at 0x7fe07780de08
  REFCNT = 1
  FLAGS = (POK,IsCOW,pPOK)
  PV = 0x7fe07752a950 "9223372036854775808"\0
  CUR = 19
  LEN = 21
  COW_REFCNT = 0
sisyphus commented 2 years ago

Oddly, for the first pf @karenetheridge's oneliners, I get something quite different on Linux:

$ perl -MStorable -MDevel::Peek  -wle'my $value = 2**63; Dump($value); print ""; my $frozen = Storable::freeze(\$value); Dump($value)'
SV = NV(0x55e42f01b758) at 0x55e42f01b770
  REFCNT = 1
  FLAGS = (NOK,pNOK)
  NV = 9.22337203685478e+18

SV = PVNV(0x55e42eff7420) at 0x55e42f01b770
  REFCNT = 1
  FLAGS = (NOK,pIOK,pNOK,IsUV)
  UV = 9223372036854775808
  NV = 9.22337203685478e+18
  PV = 0

On Windows, I also see the same output as for Linux. And it makes no difference whether I use perl-5.34.0/Storable-3.23 or perl-5.35.7/Storable-3.25. IME, when nvsize is 8, 2**63 always returns an NV, irrespective of ivsize. However, my personal experience doesn't include OSX(darwin) ... and my observation is perhaps quite tangential to the issue raised here.

The outputs of the next two one-liners, as reported by @karenetheridge are in agreement with what I see on both perl-5.34.0/Storable-3.23 and perl-5.35.7/Storable-3.25 (on both Windows 7 and Ubuntu-20.04.3 LTS). (I thought it might be helpful to point out that the behaviour seems to have been around for a while.)

karenetheridge commented 2 years ago

I expect you will see different behaviour depending on your ivsize and nvsize. It would appear that the UV flag is set on an IV when the number is too large to be stored as a signed int, but can still be accomodated if it's an unsigned int -- and for my architecture, the range for a signed int is -2**63 to 2**63 -1, so a number one larger (2**63) will be stored with the UV flag instead.

On some of BinGOs's systems (http://paste.scsys.co.uk/596349 *), where ivsize=4, the line is at 2**31 -1 -> 2**31.

* paste included as it will expire in a month:

cpan@dagobah:~$ /home/cpan/pit/thr/perl-5.28.3/bin/perl -MDevel::Peek -wle'Dump(2147483648)'
SV = IV(0x227421c) at 0x2274220
  REFCNT = 1
  FLAGS = (IOK,READONLY,PROTECT,pIOK,IsUV)
  UV = 2147483648
sisyphus commented 2 years ago

Yes, on 32-bit Windows, I see the same:

C:\>perl -MDevel::Peek -wle"Dump(2147483648)"
SV = IV(0x57ed40) at 0x57ed44
  REFCNT = 1
  FLAGS = (IOK,READONLY,PROTECT,pIOK,IsUV)
  UV = 2147483648

But, when I create the value as 2**31 it is created as an NV:

C:\>perl -MDevel::Peek -wle"Dump(2 ** 31)"
SV = NV(0x1d14b7c) at 0x1d0e55c
  REFCNT = 1
  FLAGS = (PADTMP,NOK,READONLY,PROTECT,pNOK)
  NV = 2147483648

In short, I have believed for quite some time that 2 63 will always be an NV, irrespective of the plethora of IVSIZE x NVSIZE x archname permutations. - and even when 1 << 63 is a UV. (That's the way it has always been IME.) In fact, I've written tests where I've chosen between 2 $i and 1 << $i based on whether I want to create an NV or an IV/UV. I'm indebted to your first one-liner for showing me the error of my ways.

As to the actual issue that you've raised, I agree that the flags should not be altered - and I can see that the "string_readlen" label in Storable.xs is responsible for adding the POK and pPOK flags to the UVs. I don't yet know how best to address that ... and I haven't even got to looking at how the IsUV flag is going AWAL. I'll keep poking at it, but it's probably a job for a keener mind than mine.