Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Segmentation fault: "evalbytes S" #15586

Closed
p5pRT opened this issue Sep 4, 2016 · 21 comments
Closed

Segmentation fault: "evalbytes S" #15586

p5pRT opened this issue Sep 4, 2016 · 21 comments

Comments

@p5pRT
Copy link

p5pRT commented Sep 4, 2016

Migrated from rt.perl.org#129196 (status was 'resolved')

Searchable as RT129196$

@p5pRT
Copy link
Author

p5pRT commented Sep 4, 2016

From @dcollinsn

./miniperl -e 'CORE​::evalbytes S'
Segmentation fault

==57556== Invalid read of size 4
==57556== at 0x4BDCE7​: Perl_yylex (toke.c​:7033)
==57556== by 0x4EDF1C​: Perl_yyparse (perly.c​:334)
==57556== by 0x4778BC​: S_parse_body (perl.c​:2373)
==57556== by 0x4778BC​: perl_parse (perl.c​:1689)
==57556== by 0x4231E9​: main (perlmain.c​:121)
==57556== Address 0x803fbc is not stack'd, malloc'd or (recently) free'd
==57556==
==57556==
==57556== Process terminating with default action of signal 11 (SIGSEGV)
==57556== Access not within mapped region at address 0x803FBC
==57556== at 0x4BDCE7​: Perl_yylex (toke.c​:7033)
==57556== by 0x4EDF1C​: Perl_yyparse (perly.c​:334)
==57556== by 0x4778BC​: S_parse_body (perl.c​:2373)
==57556== by 0x4778BC​: perl_parse (perl.c​:1689)
==57556== by 0x4231E9​: main (perlmain.c​:121)
==57556== If you believe this happened as a result of a stack
==57556== overflow in your program's main thread (unlikely but
==57556== possible), you can try to increase the size of the
==57556== main thread stack using the --main-stacksize= flag.
==57556== The main thread stack size used in this run was 8388608.
==57556==
==57556== HEAP SUMMARY​:
==57556== in use at exit​: 116,564 bytes in 581 blocks
==57556== total heap usage​: 667 allocs, 86 frees, 126,756 bytes allocated
==57556==
==57556== LEAK SUMMARY​:
==57556== definitely lost​: 232 bytes in 1 blocks
==57556== indirectly lost​: 2,260 bytes in 27 blocks
==57556== possibly lost​: 24 bytes in 1 blocks
==57556== still reachable​: 114,048 bytes in 552 blocks
==57556== suppressed​: 0 bytes in 0 blocks
==57556== Rerun with --leak-check=full to see details of leaked memory
==57556==
==57556== For counts of detected and suppressed errors, rerun with​: -v
==57556== ERROR SUMMARY​: 1 errors from 1 contexts (suppressed​: 0 from 0)
Segmentation fault

(gdb) run
Starting program​: /usr/local/perl-afl/bin/perl -e CORE​::evalbytes\ S
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
bt
Program received signal SIGSEGV, Segmentation fault.
0x00000000004bdce7 in Perl_yylex () at toke.c​:7033
7033 || ((PL_opargs[PL_last_lop_op] >> OASHIFT)& 7)
(gdb) bt
#0 0x00000000004bdce7 in Perl_yylex () at toke.c​:7033
#1 0x00000000004edf1d in Perl_yyparse (gramtype=<optimized out>)
  at perly.c​:334
#2 0x00000000004778bd in S_parse_body (env=<optimized out>,
  xsinit=<optimized out>) at perl.c​:2373
#3 perl_parse (my_perl=<optimized out>, xsinit=<optimized out>,
  argc=<optimized out>, argv=<optimized out>, env=<optimized out>)
  at perl.c​:1689
#4 0x00000000004231ea in main (argc=28672, argv=0xfea7, env=<optimized out>)
  at perlmain.c​:121
(gdb)

5dc1327 is the first new commit
commit 5dc1327
Author​: Matthew Horsfall (alh) <wolfsage@​gmail.com>
Date​: Mon Jul 2 22​:20​:39 2012 -0700

  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
:100644 100644 13d7ac2cd01c993e4aa750fa5bbb34365ebe49f4 d9963a9da2eecde0c3eba44e77d3852c0069cf48 M toke.c

--
Respectfully,
Dan Collins

@p5pRT
Copy link
Author

p5pRT commented Sep 4, 2016

From @cpansprout

On Sun Sep 04 11​:06​:08 2016, dcollinsn@​gmail.com wrote​:

./miniperl -e 'CORE​::evalbytes S'
Segmentation fault

It doesn’t crash for me.

(gdb) run
Starting program​: /usr/local/perl-afl/bin/perl -e CORE​::evalbytes\ S
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-
gnu/libthread_db.so.1".
bt
Program received signal SIGSEGV, Segmentation fault.
0x00000000004bdce7 in Perl_yylex () at toke.c​:7033
7033 || ((PL_opargs[PL_last_lop_op] >>
OASHIFT)& 7)

PL_last_lop_op is negative, isn’t it?

5dc1327 is the first new commit
commit 5dc1327
Author​: Matthew Horsfall (alh) <wolfsage@​gmail.com>
Date​: Mon Jul 2 22​:20​:39 2012 -0700

For #16249 - Overwrite PL_last_lop_op when eval() is called.

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

@p5pRT
Copy link
Author

p5pRT commented Sep 4, 2016

From @cpansprout

Inline Patch
diff --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); \

@p5pRT
Copy link
Author

p5pRT commented Sep 4, 2016

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

@p5pRT
Copy link
Author

p5pRT commented Sep 4, 2016

From @dcollinsn

On Sun Sep 04 11​:20​:29 2016, sprout wrote​:

On Sun Sep 04 11​:06​:08 2016, dcollinsn@​gmail.com wrote​:

./miniperl -e 'CORE​::evalbytes S'
Segmentation fault

It doesn’t crash for me.

(gdb) run
Starting program​: /usr/local/perl-afl/bin/perl -e CORE​::evalbytes\ S
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-
gnu/libthread_db.so.1".
bt
Program received signal SIGSEGV, Segmentation fault.
0x00000000004bdce7 in Perl_yylex () at toke.c​:7033
7033 || ((PL_opargs[PL_last_lop_op] >>
OASHIFT)& 7)

PL_last_lop_op is negative, isn’t it?

5dc1327 is the first new commit
commit 5dc1327
Author​: Matthew Horsfall (alh) <wolfsage@​gmail.com>
Date​: Mon Jul 2 22​:20​:39 2012 -0700

For #16249 - Overwrite PL_last_lop_op when eval() is called.

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?

Yup.

(gdb) p PL_parser->last_lop_op
$3 = 65191

Attached patch produces the following​:

$ perl t/op/evalbytes.t
1..9
ok 1 - evalbytes outside feature scope
ok 2 - evalbytes basic sanity check
ok 3 - evalbytes on extra-ASCII bytes
ok 4 - evalbytes on upgraded extra-ASCII
ok 5 - evalbytes ignores outer utf8 pragma
ok 6 - use utf8 within evalbytes
ok 7 - use utf8 within evalbytes on utf8 string
ok 8 - evalbytes croaks on non-bytes
Segmentation fault
$ ./perl -Ilib t/op/evalbytes.t
1..9
ok 1 - evalbytes outside feature scope
ok 2 - evalbytes basic sanity check
ok 3 - evalbytes on extra-ASCII bytes
ok 4 - evalbytes on upgraded extra-ASCII
ok 5 - evalbytes ignores outer utf8 pragma
ok 6 - use utf8 within evalbytes
ok 7 - use utf8 within evalbytes on utf8 string
ok 8 - evalbytes croaks on non-bytes
ok 9 - [RT \#129196] evalbytes S should not segfault

--
Respectfully,
Dan Collins

@p5pRT
Copy link
Author

p5pRT commented Sep 4, 2016

From @dcollinsn

0001-Regression-test-for-RT-129196.patch
From 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

@p5pRT
Copy link
Author

p5pRT commented Sep 4, 2016

From @jkeenan

On Sun Sep 04 11​:47​:06 2016, dcollinsn@​gmail.com wrote​:

On Sun Sep 04 11​:20​:29 2016, sprout wrote​:

On Sun Sep 04 11​:06​:08 2016, dcollinsn@​gmail.com wrote​:

./miniperl -e 'CORE​::evalbytes S'
Segmentation fault

It doesn’t crash for me.

It crashed for me on both Linux and FreeBSD 10.3. "./miniperl", "./perl" and "perl" (5.24.0) all crashed.

[snip]

Attached patch produces the following​:

$ perl t/op/evalbytes.t
1..9
ok 1 - evalbytes outside feature scope
ok 2 - evalbytes basic sanity check
ok 3 - evalbytes on extra-ASCII bytes
ok 4 - evalbytes on upgraded extra-ASCII
ok 5 - evalbytes ignores outer utf8 pragma
ok 6 - use utf8 within evalbytes
ok 7 - use utf8 within evalbytes on utf8 string
ok 8 - evalbytes croaks on non-bytes
Segmentation fault
$ ./perl -Ilib t/op/evalbytes.t
1..9
ok 1 - evalbytes outside feature scope
ok 2 - evalbytes basic sanity check
ok 3 - evalbytes on extra-ASCII bytes
ok 4 - evalbytes on upgraded extra-ASCII
ok 5 - evalbytes ignores outer utf8 pragma
ok 6 - use utf8 within evalbytes
ok 7 - use utf8 within evalbytes on utf8 string
ok 8 - evalbytes croaks on non-bytes
ok 9 - [RT \#129196] evalbytes S should not segfault

When I tried out this patch on both of my OSes, I got failures​:

On Linux, where I ran make test_harness​:

#####
Test Summary Report


op/evalbytes.t (Wstat​: 139 Tests​: 0 Failed​: 0)
  Non-zero wait status​: 139
  Parse errors​: No plan found in TAP output
#####

On FreeBSD, where I ran 'make test_prep' and then said

#####
[perl] $ cd t; ./perl harness -v op/evalbytes.t; cd -
No subtests run

Test Summary Report


op/evalbytes.t (Wstat​: 139 Tests​: 0 Failed​: 0)
  Non-zero wait status​: 139
  Parse errors​: No plan found in TAP output
Files=1, Tests=0, 1 wallclock secs ( 0.01 usr 0.00 sys + 0.00 cusr 0.06 csys = 0.07 CPU)
Result​: FAIL
#####

But on each OS, when I simply run the test file with the newly built perl, I get failures​:

#####
[perl] $ ./perl t/op/evalbytes.t
1..9
ok 1 - evalbytes outside feature scope
ok 2 - evalbytes basic sanity check
ok 3 - evalbytes on extra-ASCII bytes
ok 4 - evalbytes on upgraded extra-ASCII
ok 5 - evalbytes ignores outer utf8 pragma
ok 6 - use utf8 within evalbytes
ok 7 - use utf8 within evalbytes on utf8 string
ok 8 - evalbytes croaks on non-bytes
Segmentation fault (core dumped)
#####

So the patch does not appear to correct the problem on these OSes.

Thank you very much.
--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Sep 4, 2016

From @dcollinsn

To be clear, James, this is after applying Father C's patch? Can you
provide the output of `valgrind ./perl t/op/evalbytes.t`?

On Sun, Sep 4, 2016 at 3​:31 PM, James E Keenan via RT <
perlbug-followup@​perl.org> wrote​:

On Sun Sep 04 11​:47​:06 2016, dcollinsn@​gmail.com wrote​:

On Sun Sep 04 11​:20​:29 2016, sprout wrote​:

On Sun Sep 04 11​:06​:08 2016, dcollinsn@​gmail.com wrote​:

./miniperl -e 'CORE​::evalbytes S'
Segmentation fault

It doesn’t crash for me.

It crashed for me on both Linux and FreeBSD 10.3. "./miniperl", "./perl"
and "perl" (5.24.0) all crashed.

[snip]

Attached patch produces the following​:

$ perl t/op/evalbytes.t
1..9
ok 1 - evalbytes outside feature scope
ok 2 - evalbytes basic sanity check
ok 3 - evalbytes on extra-ASCII bytes
ok 4 - evalbytes on upgraded extra-ASCII
ok 5 - evalbytes ignores outer utf8 pragma
ok 6 - use utf8 within evalbytes
ok 7 - use utf8 within evalbytes on utf8 string
ok 8 - evalbytes croaks on non-bytes
Segmentation fault
$ ./perl -Ilib t/op/evalbytes.t
1..9
ok 1 - evalbytes outside feature scope
ok 2 - evalbytes basic sanity check
ok 3 - evalbytes on extra-ASCII bytes
ok 4 - evalbytes on upgraded extra-ASCII
ok 5 - evalbytes ignores outer utf8 pragma
ok 6 - use utf8 within evalbytes
ok 7 - use utf8 within evalbytes on utf8 string
ok 8 - evalbytes croaks on non-bytes
ok 9 - [RT \#129196] evalbytes S should not segfault

When I tried out this patch on both of my OSes, I got failures​:

On Linux, where I ran make test_harness​:

#####
Test Summary Report
-------------------
op/evalbytes.t (Wstat​:
139 Tests​: 0 Failed​: 0)
Non-zero wait status​: 139
Parse errors​: No plan found in TAP output
#####

On FreeBSD, where I ran 'make test_prep' and then said

#####
[perl] $ cd t; ./perl harness -v op/evalbytes.t; cd -
No subtests run

Test Summary Report
-------------------
op/evalbytes.t (Wstat​: 139 Tests​: 0 Failed​: 0)
Non-zero wait status​: 139
Parse errors​: No plan found in TAP output
Files=1, Tests=0, 1 wallclock secs ( 0.01 usr 0.00 sys + 0.00 cusr
0.06 csys = 0.07 CPU)
Result​: FAIL
#####

But on each OS, when I simply run the test file with the newly built perl,
I get failures​:

#####
[perl] $ ./perl t/op/evalbytes.t
1..9
ok 1 - evalbytes outside feature scope
ok 2 - evalbytes basic sanity check
ok 3 - evalbytes on extra-ASCII bytes
ok 4 - evalbytes on upgraded extra-ASCII
ok 5 - evalbytes ignores outer utf8 pragma
ok 6 - use utf8 within evalbytes
ok 7 - use utf8 within evalbytes on utf8 string
ok 8 - evalbytes croaks on non-bytes
Segmentation fault (core dumped)
#####

So the patch does not appear to correct the problem on these OSes.

Thank you very much.
--
James E Keenan (jkeenan@​cpan.org)

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=129196

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2016

From @jkeenan

On Sun Sep 04 13​:13​:07 2016, dcollinsn@​gmail.com wrote​:

To be clear, James, this is after applying Father C's patch? Can you
provide the output of `valgrind ./perl t/op/evalbytes.t`?

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.

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2016

From @jkeenan

On Sun Sep 04 18​:28​:05 2016, jkeenan wrote​:

On Sun Sep 04 13​:13​:07 2016, dcollinsn@​gmail.com wrote​:

To be clear, James, this is after applying Father C's patch? Can you
provide the output of `valgrind ./perl t/op/evalbytes.t`?

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.

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.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2016

From @cpansprout

On Sun Sep 04 19​:19​:45 2016, jkeenan wrote​:

On Sun Sep 04 18​:28​:05 2016, jkeenan wrote​:

On Sun Sep 04 13​:13​:07 2016, dcollinsn@​gmail.com wrote​:

To be clear, James, this is after applying Father C's patch? Can
you
provide the output of `valgrind ./perl t/op/evalbytes.t`?

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.

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.

Thank you for testing. I have applied both patches, as 9bde562 and a612871 respectively.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2016

@cpansprout - Status changed from 'open' to 'pending release'

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2016

From @iabyn

On Sun, Sep 04, 2016 at 09​:30​:44PM -0700, Father Chrysostomos via RT wrote​:

On Sun Sep 04 19​:19​:45 2016, jkeenan wrote​:

On Sun Sep 04 18​:28​:05 2016, jkeenan wrote​:

On Sun Sep 04 13​:13​:07 2016, dcollinsn@​gmail.com wrote​:

To be clear, James, this is after applying Father C's patch? Can
you
provide the output of `valgrind ./perl t/op/evalbytes.t`?

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.

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.

Thank you for testing. I have applied both patches, as 9bde562 and a612871 respectively.

Around the time this commit was pushed, win32 smokers and Jenkins have
been failing to compile, with

..\toke.c(7586) : error C2105​: '--' needs l-value

However, your commit doesn't touch the affected area of code, which hasn't
changed since 2011​:

7584​: case KEY_evalbytes​:
7585​: PL_expect = XTERM;
7586​: UNIBRACK(-OP_ENTEREVAL);

The error would seem to imply that OP_ENTEREVAL has been defined to a
negative value, but it's been 345 in opnames.h for a month now.

So I can't see what the problem is.

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

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2016

From @ilmari

Dave Mitchell <davem@​iabyn.com> writes​:

On 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
been failing to compile, with

..\toke.c(7586) : error C2105​: '--' needs l-value

However, your commit doesn't touch the affected area of code, which hasn't
changed since 2011​:

7584​: case KEY_evalbytes​:
7585​: PL_expect = XTERM;
7586​: UNIBRACK(-OP_ENTEREVAL);

The error would seem to imply that OP_ENTEREVAL has been defined to a
negative value, but it's been 345 in opnames.h for a month now.

So I can't see what the problem is.

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)'
(and '(f)', for consistency) shold fix it.

--
"The surreality of the universe tends towards a maximum" -- Skud's Law
"Never formulate a law or axiom that you're not prepared to live with
the consequences of." -- Skud's Meta-Law

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2016

From zefram@fysh.org

Dave Mitchell wrote​:

..\toke.c(7586) : error C2105​: '--' needs l-value
...
7586​: UNIBRACK(-OP_ENTEREVAL);

The error would seem to imply that OP_ENTEREVAL has been defined to a
negative value,

That's not how the C preprocessor works. Macro expansion can't paste
two adjacent "-" tokens into a "--". In any case, the OP_ values have
never been negative.

So I can't see what the problem is.

I too have no idea where this "--" could be coming from.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2016

From @iabyn

On Mon, Sep 05, 2016 at 02​:48​:40PM +0100, Dagfinn Ilmari Mannsåker wrote​:

Dave Mitchell <davem@​iabyn.com> writes​:

On 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
been failing to compile, with

..\toke.c(7586) : error C2105​: '--' needs l-value

However, your commit doesn't touch the affected area of code, which hasn't
changed since 2011​:

7584​: case KEY_evalbytes​:
7585​: PL_expect = XTERM;
7586​: UNIBRACK(-OP_ENTEREVAL);

The error would seem to imply that OP_ENTEREVAL has been defined to a
negative value, but it's been 345 in opnames.h for a month now.

So I can't see what the problem is.

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)'
(and '(f)', for consistency) shold fix it.

I've just done that as v5.25.4-84-g0af40c7

On Mon, Sep 05, 2016 at 02​:55​:25PM +0100, Zefram wrote​:

Dave Mitchell wrote​:

..\toke.c(7586) : error C2105​: '--' needs l-value
...
7586​: UNIBRACK(-OP_ENTEREVAL);

The error would seem to imply that OP_ENTEREVAL has been defined to a
negative value,

That's not how the C preprocessor works. Macro expansion can't paste
two adjacent "-" tokens into a "--". In any case, the OP_ values have
never been negative.

I guess that's a bug in the win C compiler? Which would explain why the
problem hasn't surfaced on other platforms.

--
The crew of the Enterprise encounter an alien life form which is
surprisingly neither humanoid nor made from pure energy.
  -- Things That Never Happen in "Star Trek" #22

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2016

From @ilmari

Dave Mitchell <davem@​iabyn.com> writes​:

On Mon, Sep 05, 2016 at 02​:55​:25PM +0100, Zefram wrote​:

Dave Mitchell wrote​:

..\toke.c(7586) : error C2105​: '--' needs l-value
...
7586​: UNIBRACK(-OP_ENTEREVAL);

The error would seem to imply that OP_ENTEREVAL has been defined to a
negative value,

That's not how the C preprocessor works. Macro expansion can't paste
two adjacent "-" tokens into a "--". In any case, the OP_ values have
never been negative.

I guess that's a bug in the win C compiler? Which would explain why the
problem hasn't surfaced on other platforms.

Looking at the preprocessor output (via 'make toke.i') from gcc, it
actually expands to​:

  (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
the macro definition. Presumably the MSVC preprocessor fails to insert
it. The parentheses should still be there for other compilers, in case
someone uses a more complex expression in future.

--
"A disappointingly low fraction of the human race is,
at any given time, on fire." - Stig Sandbeck Mathisen

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2016

From @cpansprout

On Mon Sep 05 07​:59​:34 2016, davem wrote​:

On Mon, Sep 05, 2016 at 02​:48​:40PM +0100, Dagfinn Ilmari Mannsåker
wrote​:

Dave Mitchell <davem@​iabyn.com> writes​:

On 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
been failing to compile, with

..\toke.c(7586) : error C2105​: '--' needs l-value

However, your commit doesn't touch the affected area of code, which
hasn't
changed since 2011​:

7584​: case KEY_evalbytes​:
7585​: PL_expect = XTERM;
7586​: UNIBRACK(-OP_ENTEREVAL);

The error would seem to imply that OP_ENTEREVAL has been defined to
a
negative value, but it's been 345 in opnames.h for a month now.

So I can't see what the problem is.

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)'
(and '(f)', for consistency) shold fix it.

I've just done that as v5.25.4-84-g0af40c7

On Mon, Sep 05, 2016 at 02​:55​:25PM +0100, Zefram wrote​:

Dave Mitchell wrote​:

..\toke.c(7586) : error C2105​: '--' needs l-value
...
7586​: UNIBRACK(-OP_ENTEREVAL);

The error would seem to imply that OP_ENTEREVAL has been defined to
a
negative value,

That's not how the C preprocessor works. Macro expansion can't paste
two adjacent "-" tokens into a "--". In any case, the OP_ values
have
never been negative.

I guess that's a bug in the win C compiler? Which would explain why
the
problem hasn't surfaced on other platforms.

I see you have beaten me to fixing it in 0af40c7.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Jan 29, 2017

From @dcollinsn

To be clear, James, this is after applying Father C's patch? Can you
provide the output of `valgrind ./perl t/op/evalbytes.t`?

On Sun, Sep 4, 2016 at 3​:31 PM, James E Keenan via RT <
perlbug-followup@​perl.org> wrote​:

On Sun Sep 04 11​:47​:06 2016, dcollinsn@​gmail.com wrote​:

On Sun Sep 04 11​:20​:29 2016, sprout wrote​:

On Sun Sep 04 11​:06​:08 2016, dcollinsn@​gmail.com wrote​:

./miniperl -e 'CORE​::evalbytes S'
Segmentation fault

It doesn’t crash for me.

It crashed for me on both Linux and FreeBSD 10.3. "./miniperl", "./perl"
and "perl" (5.24.0) all crashed.

[snip]

Attached patch produces the following​:

$ perl t/op/evalbytes.t
1..9
ok 1 - evalbytes outside feature scope
ok 2 - evalbytes basic sanity check
ok 3 - evalbytes on extra-ASCII bytes
ok 4 - evalbytes on upgraded extra-ASCII
ok 5 - evalbytes ignores outer utf8 pragma
ok 6 - use utf8 within evalbytes
ok 7 - use utf8 within evalbytes on utf8 string
ok 8 - evalbytes croaks on non-bytes
Segmentation fault
$ ./perl -Ilib t/op/evalbytes.t
1..9
ok 1 - evalbytes outside feature scope
ok 2 - evalbytes basic sanity check
ok 3 - evalbytes on extra-ASCII bytes
ok 4 - evalbytes on upgraded extra-ASCII
ok 5 - evalbytes ignores outer utf8 pragma
ok 6 - use utf8 within evalbytes
ok 7 - use utf8 within evalbytes on utf8 string
ok 8 - evalbytes croaks on non-bytes
ok 9 - [RT \#129196] evalbytes S should not segfault

When I tried out this patch on both of my OSes, I got failures​:

On Linux, where I ran make test_harness​:

#####
Test Summary Report
-------------------
op/evalbytes.t (Wstat​:
139 Tests​: 0 Failed​: 0)
Non-zero wait status​: 139
Parse errors​: No plan found in TAP output
#####

On FreeBSD, where I ran 'make test_prep' and then said

#####
[perl] $ cd t; ./perl harness -v op/evalbytes.t; cd -
No subtests run

Test Summary Report
-------------------
op/evalbytes.t (Wstat​: 139 Tests​: 0 Failed​: 0)
Non-zero wait status​: 139
Parse errors​: No plan found in TAP output
Files=1, Tests=0, 1 wallclock secs ( 0.01 usr 0.00 sys + 0.00 cusr
0.06 csys = 0.07 CPU)
Result​: FAIL
#####

But on each OS, when I simply run the test file with the newly built perl,
I get failures​:

#####
[perl] $ ./perl t/op/evalbytes.t
1..9
ok 1 - evalbytes outside feature scope
ok 2 - evalbytes basic sanity check
ok 3 - evalbytes on extra-ASCII bytes
ok 4 - evalbytes on upgraded extra-ASCII
ok 5 - evalbytes ignores outer utf8 pragma
ok 6 - use utf8 within evalbytes
ok 7 - use utf8 within evalbytes on utf8 string
ok 8 - evalbytes croaks on non-bytes
Segmentation fault (core dumped)
#####

So the patch does not appear to correct the problem on these OSes.

Thank you very much.
--
James E Keenan (jkeenan@​cpan.org)

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=129196

@p5pRT
Copy link
Author

p5pRT commented May 30, 2017

From @khwilliamson

Thank 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
resolved.

Perl 5.26.0 may be downloaded via​:
https://metacpan.org/release/XSAWYERX/perl-5.26.0

If you find that the problem persists, feel free to reopen this ticket.

@p5pRT
Copy link
Author

p5pRT commented May 30, 2017

@khwilliamson - Status changed from 'pending release' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant