Negative offset to vec in lvalue context #12001
Comments
From pm.20.browseruk@xoxy.netCreated by pm.20.browseruk@xoxy.netvec() doesn't like offsets between 2**31 and 2**32-1 and emits: perl -E"say $]; $v = ''; eval{ vec( $v, Negative offset to vec in lvalue context at -e line 1. Negative offset to vec in lvalue context at -e line 1. Also confirmed on bleed/linux-x64 Perl Info |
From @tonycozFrom looking at the code this happens because do_vecset() uses I32 for do_vecget() has a similar issue, since it's offset parameter is an I32. |
|
The RT System itself - Status changed from 'new' to 'open' |
From @tonycozOn Tue, Mar 13, 2012 at 10:38:35PM -0700, Tony Cook via RT wrote:
Attached is a fix, using the PERL_TEST_MEMORY from my patch in RT #72784. Intended for post 5.16. Tony |
From @tonycoz0001-rt-111730-TODO-tests-for-vec-with-large-offsets.patchFrom 3259dda591d8c770ec2d6eac9b182cfa16ef82af Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Fri, 23 Mar 2012 23:36:27 +0100
Subject: [PATCH 1/2] [rt #111730] TODO tests for vec() with large offsets
---
MANIFEST | 1 +
t/bigmem/vec.t | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 37 insertions(+), 0 deletions(-)
create mode 100644 t/bigmem/vec.t
diff --git a/MANIFEST b/MANIFEST
index dc01fdb..ef44268 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -4969,6 +4969,7 @@ t/base/term.t See if various terms work
t/base/while.t See if while work
t/benchmark/rt26188-speed-up-keys-on-empty-hash.t Benchmark if keys on empty hashes is fast enough
t/bigmem/read.t Check read() handles large offsets
+t/bigmem/vec.t Check vec() handles large offsets
t/cmd/elsif.t See if else-if works
t/cmd/for.t See if for loops work
t/cmd/mod.t See if statement modifiers work
diff --git a/t/bigmem/vec.t b/t/bigmem/vec.t
new file mode 100644
index 0000000..ccdbb9b
--- /dev/null
+++ b/t/bigmem/vec.t
@@ -0,0 +1,36 @@
+#!perl
+BEGIN {
+ chdir 't';
+ unshift @INC, "../lib";
+}
+
+use strict;
+require './test.pl';
+use Config qw(%Config);
+
+$ENV{PERL_TEST_MEMORY} >= 1
+ or skip_all("Need ~1Gb for this test");
+$Config{ptrsize} >= 8
+ or skip_all("Need 64-bit pointers for this test");
+
+plan(7);
+
+# RT #111730: Negative offset to vec in lvalue context
+local $::TODO = "RT #111730 - vec uses I32 for offsets";
+
+my $v = "";
+ok(scalar eval { vec($v, 0x80000000, 1) = 1 }, "set a bit at a large offset");
+ok(vec($v, 0x80000000, 1), "check a bit at a large offset");
+{ local $::TODO; # succeeds but shouldn't at this point
+ok(scalar eval { vec($v, 0x100000000, 1) = 1 },
+ "set a bit at a larger offset");
+ok(vec($v, 0x100000000, 1), "check a bit at a larger offset");
+
+# real out of range values
+ok(!eval { vec($v, -0x80000000, 1) = 1 },
+ "shouldn't be able to set at a large negative offset");
+}
+ok(!eval { vec($v, -0x100000000, 1) = 1 },
+ "shouldn't be able to set at a larger negative offset");
+
+ok(!vec($v, 0, 1), "make sure we didn't wrap");
--
1.7.2.5
|
From @tonycoz0002-rt-111730-don-t-use-I32-for-offsets-in-vec.patchFrom 4dd9a80bfc60f1266dd827fe2a22e375f7deb06e Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Sat, 24 Mar 2012 00:27:52 +0100
Subject: [PATCH 2/2] [rt #111730] don't use I32 for offsets in vec()
do_vecset() do_vecget() used I32 for the offset, which meant that
offsets outside the -2Gb - +2Gb offset were truncated, resulting in
various misbehaviours.
size is still an I32 in each case, but that's limited to small numbers
- powers of 2 from 1 to a current maximum of 64, so I didn't see much
value to changing that.
---
doop.c | 4 ++--
embed.fnc | 2 +-
proto.h | 2 +-
t/bigmem/vec.t | 4 +---
4 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/doop.c b/doop.c
index f130477..cac9e1a 100644
--- a/doop.c
+++ b/doop.c
@@ -760,7 +760,7 @@ Perl_do_sprintf(pTHX_ SV *sv, I32 len, SV **sarg)
/* currently converts input to bytes if possible, but doesn't sweat failure */
UV
-Perl_do_vecget(pTHX_ SV *sv, I32 offset, I32 size)
+Perl_do_vecget(pTHX_ SV *sv, SSize_t offset, I32 size)
{
dVAR;
STRLEN srclen, len, uoffset, bitoffs = 0;
@@ -908,7 +908,7 @@ void
Perl_do_vecset(pTHX_ SV *sv)
{
dVAR;
- register I32 offset, bitoffs = 0;
+ register SSize_t offset, bitoffs = 0;
register I32 size;
register unsigned char *s;
register UV lval;
diff --git a/embed.fnc b/embed.fnc
index 8e3527d..a7f7dc3 100644
--- a/embed.fnc
+++ b/embed.fnc
@@ -379,7 +379,7 @@ pR |Off_t |do_tell |NN GV* gv
: Defined in doop.c, used only in pp.c
p |I32 |do_trans |NN SV* sv
: Used in my.c and pp.c
-p |UV |do_vecget |NN SV* sv|I32 offset|I32 size
+p |UV |do_vecget |NN SV* sv|SSize_t offset|I32 size
: Defined in doop.c, used only in mg.c (with /* XXX slurp this routine */)
p |void |do_vecset |NN SV* sv
: Defined in doop.c, used only in pp.c
diff --git a/proto.h b/proto.h
index 781719b..e43c437 100644
--- a/proto.h
+++ b/proto.h
@@ -893,7 +893,7 @@ PERL_CALLCONV I32 Perl_do_trans(pTHX_ SV* sv)
#define PERL_ARGS_ASSERT_DO_TRANS \
assert(sv)
-PERL_CALLCONV UV Perl_do_vecget(pTHX_ SV* sv, I32 offset, I32 size)
+PERL_CALLCONV UV Perl_do_vecget(pTHX_ SV* sv, SSize_t offset, I32 size)
__attribute__nonnull__(pTHX_1);
#define PERL_ARGS_ASSERT_DO_VECGET \
assert(sv)
diff --git a/t/bigmem/vec.t b/t/bigmem/vec.t
index ccdbb9b..bf3c513 100644
--- a/t/bigmem/vec.t
+++ b/t/bigmem/vec.t
@@ -16,12 +16,11 @@ $Config{ptrsize} >= 8
plan(7);
# RT #111730: Negative offset to vec in lvalue context
-local $::TODO = "RT #111730 - vec uses I32 for offsets";
my $v = "";
ok(scalar eval { vec($v, 0x80000000, 1) = 1 }, "set a bit at a large offset");
ok(vec($v, 0x80000000, 1), "check a bit at a large offset");
-{ local $::TODO; # succeeds but shouldn't at this point
+
ok(scalar eval { vec($v, 0x100000000, 1) = 1 },
"set a bit at a larger offset");
ok(vec($v, 0x100000000, 1), "check a bit at a larger offset");
@@ -29,7 +28,6 @@ ok(vec($v, 0x100000000, 1), "check a bit at a larger offset");
# real out of range values
ok(!eval { vec($v, -0x80000000, 1) = 1 },
"shouldn't be able to set at a large negative offset");
-}
ok(!eval { vec($v, -0x100000000, 1) = 1 },
"shouldn't be able to set at a larger negative offset");
--
1.7.2.5
|
From @tonycozOn Mon, Mar 26, 2012 at 03:42:57PM +0100, Nicholas Clark wrote:
I'll re-work it. Though this brings up a problem I see as being related - SvIV(), If I call SvUV() on an sv containing a negative number - should it Or SvIV() with 32-bit IV on 0x80000000. If I do $x[0x100000001] (too big the I32 currently used for array Tony |
From @nwc10On Tue, Mar 27, 2012 at 02:54:23PM +1100, Tony Cook wrote:
Yes, they don't.
At this stage, I'm not comfortable that the can change. They're documented =for apidoc Am|IV|SvIV|SV* sv =for apidoc Am|UV|SvUV|SV* sv and "coerce" seems to be the operative word - smash as hard as necessary to I'm not aware of an existing terse effective way of doing range checking, Nicholas Clark |
From @tonycozOn Sat, Mar 24, 2012 at 02:01:13PM +1100, Tony Cook wrote:
Alternate version changing I32 to int for size. Tony |
From @tonycoz0001-rt-111730-TODO-tests-for-vec-with-large-offsets.patchFrom a6e24590df0552dd288e6e9e508095adfd969bf9 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Fri, 23 Mar 2012 23:36:27 +0100
Subject: [PATCH 1/2] [rt #111730] TODO tests for vec() with large offsets
---
MANIFEST | 1 +
t/bigmem/vec.t | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 37 insertions(+), 0 deletions(-)
create mode 100644 t/bigmem/vec.t
diff --git a/MANIFEST b/MANIFEST
index 3a24a61..e0e24d9 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -4970,6 +4970,7 @@ t/base/term.t See if various terms work
t/base/while.t See if while work
t/benchmark/rt26188-speed-up-keys-on-empty-hash.t Benchmark if keys on empty hashes is fast enough
t/bigmem/read.t Check read() handles large offsets
+t/bigmem/vec.t Check vec() handles large offsets
t/cmd/elsif.t See if else-if works
t/cmd/for.t See if for loops work
t/cmd/mod.t See if statement modifiers work
diff --git a/t/bigmem/vec.t b/t/bigmem/vec.t
new file mode 100644
index 0000000..ccdbb9b
--- /dev/null
+++ b/t/bigmem/vec.t
@@ -0,0 +1,36 @@
+#!perl
+BEGIN {
+ chdir 't';
+ unshift @INC, "../lib";
+}
+
+use strict;
+require './test.pl';
+use Config qw(%Config);
+
+$ENV{PERL_TEST_MEMORY} >= 1
+ or skip_all("Need ~1Gb for this test");
+$Config{ptrsize} >= 8
+ or skip_all("Need 64-bit pointers for this test");
+
+plan(7);
+
+# RT #111730: Negative offset to vec in lvalue context
+local $::TODO = "RT #111730 - vec uses I32 for offsets";
+
+my $v = "";
+ok(scalar eval { vec($v, 0x80000000, 1) = 1 }, "set a bit at a large offset");
+ok(vec($v, 0x80000000, 1), "check a bit at a large offset");
+{ local $::TODO; # succeeds but shouldn't at this point
+ok(scalar eval { vec($v, 0x100000000, 1) = 1 },
+ "set a bit at a larger offset");
+ok(vec($v, 0x100000000, 1), "check a bit at a larger offset");
+
+# real out of range values
+ok(!eval { vec($v, -0x80000000, 1) = 1 },
+ "shouldn't be able to set at a large negative offset");
+}
+ok(!eval { vec($v, -0x100000000, 1) = 1 },
+ "shouldn't be able to set at a larger negative offset");
+
+ok(!vec($v, 0, 1), "make sure we didn't wrap");
--
1.7.2.5
|
From @tonycoz0002-rt-111730-don-t-use-I32-for-offsets-in-vec.patchFrom 572e8228b9a52b3cccd82b78eab825833a3cff81 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Sat, 24 Mar 2012 00:27:52 +0100
Subject: [PATCH 2/2] [rt #111730] don't use I32 for offsets in vec()
do_vecset() do_vecget() used I32 for the offset, which meant that
offsets outside the -2Gb - +2Gb offset were truncated, resulting in
various misbehaviours.
---
doop.c | 6 +++---
embed.fnc | 2 +-
proto.h | 2 +-
t/bigmem/vec.t | 4 +---
4 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/doop.c b/doop.c
index f130477..a562536 100644
--- a/doop.c
+++ b/doop.c
@@ -760,7 +760,7 @@ Perl_do_sprintf(pTHX_ SV *sv, I32 len, SV **sarg)
/* currently converts input to bytes if possible, but doesn't sweat failure */
UV
-Perl_do_vecget(pTHX_ SV *sv, I32 offset, I32 size)
+Perl_do_vecget(pTHX_ SV *sv, SSize_t offset, int size)
{
dVAR;
STRLEN srclen, len, uoffset, bitoffs = 0;
@@ -908,8 +908,8 @@ void
Perl_do_vecset(pTHX_ SV *sv)
{
dVAR;
- register I32 offset, bitoffs = 0;
- register I32 size;
+ register SSize_t offset, bitoffs = 0;
+ register int size;
register unsigned char *s;
register UV lval;
I32 mask;
diff --git a/embed.fnc b/embed.fnc
index a1dee4d..5f7b73c 100644
--- a/embed.fnc
+++ b/embed.fnc
@@ -379,7 +379,7 @@ pR |Off_t |do_tell |NN GV* gv
: Defined in doop.c, used only in pp.c
p |I32 |do_trans |NN SV* sv
: Used in my.c and pp.c
-p |UV |do_vecget |NN SV* sv|I32 offset|I32 size
+p |UV |do_vecget |NN SV* sv|SSize_t offset|int size
: Defined in doop.c, used only in mg.c (with /* XXX slurp this routine */)
p |void |do_vecset |NN SV* sv
: Defined in doop.c, used only in pp.c
diff --git a/proto.h b/proto.h
index a9bd7c5..ac9148f 100644
--- a/proto.h
+++ b/proto.h
@@ -893,7 +893,7 @@ PERL_CALLCONV I32 Perl_do_trans(pTHX_ SV* sv)
#define PERL_ARGS_ASSERT_DO_TRANS \
assert(sv)
-PERL_CALLCONV UV Perl_do_vecget(pTHX_ SV* sv, I32 offset, I32 size)
+PERL_CALLCONV UV Perl_do_vecget(pTHX_ SV* sv, SSize_t offset, int size)
__attribute__nonnull__(pTHX_1);
#define PERL_ARGS_ASSERT_DO_VECGET \
assert(sv)
diff --git a/t/bigmem/vec.t b/t/bigmem/vec.t
index ccdbb9b..bf3c513 100644
--- a/t/bigmem/vec.t
+++ b/t/bigmem/vec.t
@@ -16,12 +16,11 @@ $Config{ptrsize} >= 8
plan(7);
# RT #111730: Negative offset to vec in lvalue context
-local $::TODO = "RT #111730 - vec uses I32 for offsets";
my $v = "";
ok(scalar eval { vec($v, 0x80000000, 1) = 1 }, "set a bit at a large offset");
ok(vec($v, 0x80000000, 1), "check a bit at a large offset");
-{ local $::TODO; # succeeds but shouldn't at this point
+
ok(scalar eval { vec($v, 0x100000000, 1) = 1 },
"set a bit at a larger offset");
ok(vec($v, 0x100000000, 1), "check a bit at a larger offset");
@@ -29,7 +28,6 @@ ok(vec($v, 0x100000000, 1), "check a bit at a larger offset");
# real out of range values
ok(!eval { vec($v, -0x80000000, 1) = 1 },
"shouldn't be able to set at a large negative offset");
-}
ok(!eval { vec($v, -0x100000000, 1) = 1 },
"shouldn't be able to set at a larger negative offset");
--
1.7.2.5
|
From @tonycozOn Wed, Mar 28, 2012 at 11:40:50AM +0100, Nicholas Clark wrote:
...
I was thinking more in terms of a _flags variant that the core would /* eg. in pp_aelem, in a mythical non-I32 world */ or a wrapper to make that suck less: SSize_t elem = SvIV_SSize_t(elemsv); which could warn or croak. Of course, all of this gets fun converting 64-bit NVs to 64-bit IVs. Tony |
From @cpansproutOn Fri Mar 30 03:36:10 2012, tonyc wrote:
Thank you. Applied as 803e7e8 and fcb8da0. -- Father Chrysostomos |
|
@cpansprout - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#111730 (status was 'resolved')
Searchable as RT111730$
The text was updated successfully, but these errors were encountered: