Segmentation fault: "evalbytes S" #15586
Comments
From @dcollinsn./miniperl -e 'CORE::evalbytes S' ==57556== Invalid read of size 4 (gdb) run 5dc1327 is the first new commit For #16249 - Overwrite PL_last_lop_op when eval() is called. Otherwise, parsing later on down the road may use the previous value, which, if it was OP_PRINT, causes the parser to fail :040000 040000 10996e189a33007b9736da4167b7104371638bd7 3cd150013621e1b73f35f1e75e65fde7e7e6e1a4 M t -- |
From @cpansproutOn Sun Sep 04 11:06:08 2016, dcollinsn@gmail.com wrote:
It doesn’t crash for me.
PL_last_lop_op is negative, isn’t it?
If you apply the attached, which changes the line that 5dc1327 added, does the crash go away? If so, could you provide a patch that adds a test that fails without my patch and passes with it? -- Father Chrysostomos |
From @cpansproutInline Patchdiff --git a/toke.c b/toke.c
index 2fe8b69..2350703 100644
--- a/toke.c
+++ b/toke.c
@@ -241,7 +241,7 @@ static const char* const lex_state_names[] = {
if (have_x) PL_expect = x; \
PL_bufptr = s; \
PL_last_uni = PL_oldbufptr; \
- PL_last_lop_op = f; \
+ PL_last_lop_op = f < 0 ? -f : f; \
if (*s == '(') \
return REPORT( (int)FUNC1 ); \
s = skipspace(s); \ |
|
The RT System itself - Status changed from 'new' to 'open' |
From @dcollinsnOn Sun Sep 04 11:20:29 2016, sprout wrote:
Yup. (gdb) p PL_parser->last_lop_op Attached patch produces the following: $ perl t/op/evalbytes.t -- |
From @dcollinsn0001-Regression-test-for-RT-129196.patchFrom bf0a9d7bc0087633c577aebe0ee976c4f1edb90d Mon Sep 17 00:00:00 2001
From: Dan Collins <dcollinsn@gmail.com>
Date: Sun, 4 Sep 2016 14:43:41 -0400
Subject: [PATCH] Regression test for RT #129196
---
t/op/evalbytes.t | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/t/op/evalbytes.t b/t/op/evalbytes.t
index 9b77c8e..9a09ba9 100644
--- a/t/op/evalbytes.t
+++ b/t/op/evalbytes.t
@@ -7,7 +7,7 @@ BEGIN {
require './charset_tools.pl';
}
-plan(tests => 8);
+plan(tests => 9);
{
local $SIG{__WARN__} = sub {};
@@ -34,3 +34,7 @@ chop($upcode = "use utf8; $U_100" . chr 256);
is evalbytes $upcode, chr 256, 'use utf8 within evalbytes on utf8 string';
eval { evalbytes chr 256 };
like $@, qr/Wide character/, 'evalbytes croaks on non-bytes';
+
+eval 'evalbytes S';
+ok 1, '[RT #129196] evalbytes S should not segfault';
+
--
2.9.3
|
From @jkeenanOn Sun Sep 04 11:47:06 2016, dcollinsn@gmail.com wrote:
It crashed for me on both Linux and FreeBSD 10.3. "./miniperl", "./perl" and "perl" (5.24.0) all crashed. [snip]
When I tried out this patch on both of my OSes, I got failures: On Linux, where I ran make test_harness: ##### op/evalbytes.t (Wstat: 139 Tests: 0 Failed: 0) On FreeBSD, where I ran 'make test_prep' and then said ##### Test Summary Report op/evalbytes.t (Wstat: 139 Tests: 0 Failed: 0) But on each OS, when I simply run the test file with the newly built perl, I get failures: ##### So the patch does not appear to correct the problem on these OSes. Thank you very much. |
From @dcollinsnTo be clear, James, this is after applying Father C's patch? Can you On Sun, Sep 4, 2016 at 3:31 PM, James E Keenan via RT <
|
From @jkeenanOn Sun Sep 04 13:13:07 2016, dcollinsn@gmail.com wrote:
Hmm, the fact that he didn't save that patch with the sort of name which 'git format-patch' would apply led me to overlook it. Checking now. |
From @jkeenanOn Sun Sep 04 18:28:05 2016, jkeenan wrote:
Okay, now having actually applied the patch in branches on each OS, I am getting PASS for 'make test_harness' and for the t/op/evalbytes.t by itself. Thank you very much. -- |
From @cpansproutOn Sun Sep 04 19:19:45 2016, jkeenan wrote:
Thank you for testing. I have applied both patches, as 9bde562 and a612871 respectively. -- Father Chrysostomos |
|
@cpansprout - Status changed from 'open' to 'pending release' |
From @iabynOn Sun, Sep 04, 2016 at 09:30:44PM -0700, Father Chrysostomos via RT wrote:
Around the time this commit was pushed, win32 smokers and Jenkins have ..\toke.c(7586) : error C2105: '--' needs l-value However, your commit doesn't touch the affected area of code, which hasn't 7584: case KEY_evalbytes: The error would seem to imply that OP_ENTEREVAL has been defined to a So I can't see what the problem is. -- |
From @ilmariDave Mitchell <davem@iabyn.com> writes:
UNIBRACK(f) is defined as UNIUNI3(f,0,0), which as of 9bde562 does PL_last_lop_op = f < 0 ? -f : f; So this expands to '-345 < 0 ? --345 : -345'. Changing it to '-(f)' -- |
From zefram@fysh.orgDave Mitchell wrote:
That's not how the C preprocessor works. Macro expansion can't paste
I too have no idea where this "--" could be coming from. -zefram |
From @iabynOn Mon, Sep 05, 2016 at 02:48:40PM +0100, Dagfinn Ilmari Mannsåker wrote:
I've just done that as v5.25.4-84-g0af40c7 On Mon, Sep 05, 2016 at 02:55:25PM +0100, Zefram wrote:
I guess that's a bug in the win C compiler? Which would explain why the -- |
From @ilmariDave Mitchell <davem@iabyn.com> writes:
Looking at the preprocessor output (via 'make toke.i') from gcc, it (PL_parser->last_lop_op) = -OP_ENTEREVAL < 0 ? - -OP_ENTEREVAL : -OP_ENTEREVAL Note the space between the two minuses, despite there being no space in -- |
From @cpansproutOn Mon Sep 05 07:59:34 2016, davem wrote:
I see you have beaten me to fixing it in 0af40c7. -- Father Chrysostomos |
From @dcollinsnTo be clear, James, this is after applying Father C's patch? Can you On Sun, Sep 4, 2016 at 3:31 PM, James E Keenan via RT <
|
From @khwilliamsonThank you for filing this report. You have helped make Perl better. With the release today of Perl 5.26.0, this and 210 other issues have been Perl 5.26.0 may be downloaded via: If you find that the problem persists, feel free to reopen this ticket. |
|
@khwilliamson - Status changed from 'pending release' to 'resolved' |
Migrated from rt.perl.org#129196 (status was 'resolved')
Searchable as RT129196$
The text was updated successfully, but these errors were encountered: