perl.c:528: perl_destruct: Assertion `PL_scopestack_ix == 1' failed

[p5pRT]

Migrated from rt.perl.org#130585 (status was 'pending release')

Searchable as RT130585$

[p5pRT]

From @dur-randir

Created by @dur-randir

While fuzzing perl v5.25.8-216-gfbceb79751 built with afl and run
under libdislocator, I found the following program

qr!@​{s{0})(?{!

to cause an assertion failure, even when run under -c for a syntax
check. This is a regression between v5.16.3 and v5.18.0, bisect points
to

491453b is the first bad commit
commit 491453b
Author​: David Mitchell <davem@​iabyn.com>
Date​: Wed Apr 17 17​:51​:16 2013 +0100

  Handle /@​a/ array expansion within regex engine

  Previously /a@​{b}c/ would be parsed as

  regcomp('a', join($", @​b), 'c')

  This means that the array was flattened and its contents stringified before
  hitting the regex engine.

  Change it so that it is parsed as

  regcomp('a', @​b, 'c')

  (but where the array isn't flattened, but rather just the AV itself is
  pushed onto the stack, c.f. push @​b, ....).

  This means that the regex engine itself can process any qr// objects
  within the array, and correctly extract out any previously-compiled
  code blocks (thus preserving the correct closure behaviour). This is
  an improvement on 5.16.x behaviour, and brings it into line with the
  newish 5.17.x behaviour for *scalar* vars where they happen to hold
  regex objects.

  It also fixes a regression from 5.16.x, which meant that you suddenly
  needed a 'use re eval' in scope if any of the elements of the array were
  a qr// object with code blocks (RT #115004).

GDB info about the crash location​:

(gdb) bt
#0 __GI_raise (sig=sig@​entry=6) at ../sysdeps/unix/sysv/linux/raise.c​:58
#1 0x00007f2d6ecf940a in __GI_abort () at abort.c​:89
#2 0x00007f2d6ecf0e47 in __assert_fail_base (fmt=<optimized out>,
assertion=assertion@​entry=0x7f2d703af467 "PL_scopestack_ix == 1",
  file=file@​entry=0x7f2d703af370 "perl.c", line=line@​entry=571,
function=function@​entry=0x7f2d703b1a08 <__PRETTY_FUNCTION__.14855>
"perl_destruct")
  at assert.c​:92
#3 0x00007f2d6ecf0ef2 in __GI___assert_fail (assertion=0x7f2d703af467
"PL_scopestack_ix == 1", file=0x7f2d703af370 "perl.c", line=571,
  function=0x7f2d703b1a08 <__PRETTY_FUNCTION__.14855>
"perl_destruct") at assert.c​:101
#4 0x00007f2d7006fff7 in perl_destruct (my_perl=0x7f2d721f4010) at perl.c​:571
#5 0x00007f2d7002fdc7 in main (argc=2, argv=0x7ffcc6177818,
env=0x7ffcc6177830) at perlmain.c​:134
(gdb) f 4
#4 0x00007f2d7006fff7 in perl_destruct (my_perl=0x7f2d721f4010) at perl.c​:571
571 assert(PL_scopestack_ix == 1);
(gdb) p PL_scopestack_ix
$1 = 2

Perl Info

Flags:
    category=core
    severity=medium

Site configuration information for perl 5.25.9:

Configured by root at Sat Jan 14 02:25:05 MSK 2017.

Summary of my perl5 (revision 5 version 25 subversion 9) configuration:
  Commit id: cbe2fc5001aa59cdc73e04cc35e097a2ecfbeec0
  Platform:
    osname=linux
    osvers=3.16.0-4-amd64
    archname=x86_64-linux
    uname='linux dorothy 3.16.0-4-amd64 #1 smp debian 3.16.36-1+deb8u2
(2016-10-19) x86_64 gnulinux '
    config_args='-des -Dusedevel -DDEBUGGING -Dcc=afl-clang-fast
-Doptimize=-O0 -g -ggdb3'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=undef
    usemultiplicity=undef
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    bincompat5005=undef
  Compiler:
    cc='afl-clang-fast'
    ccflags ='-DDEBUGGING -fno-strict-aliasing -pipe
-fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE
-D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2'
    optimize='-O0 -g -ggdb3'
    cppflags='-DDEBUGGING -fno-strict-aliasing -pipe
-fstack-protector-strong -I/usr/local/include'
    ccversion=''
    gccversion='4.2.1 Compatible Clang 3.9.1 (tags/RELEASE_391/rc2)'
    gccosandvers=''
    intsize=4
    longsize=8
    ptrsize=8
    doublesize=8
    byteorder=12345678
    doublekind=3
    d_longlong=define
    longlongsize=8
    d_longdbl=define
    longdblsize=16
    longdblkind=3
    ivtype='long'
    ivsize=8
    nvtype='double'
    nvsize=8
    Off_t='off_t'
    lseeksize=8
    alignbytes=8
    prototype=define
  Linker and Libraries:
    ld='afl-clang-fast'
    ldflags =' -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib/llvm-3.9/bin/../lib/clang/3.9.1/lib
/usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu
/lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib
    libs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.24.so
    so=so
    useshrplib=false
    libperl=libperl.a
    gnulibc_version='2.24'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-E'
    cccdlflags='-fPIC'
    lddlflags='-shared -O0 -g -ggdb3 -L/usr/local/lib -fstack-protector-strong'



@INC for perl 5.25.9:
    lib
    /usr/local/lib/perl5/site_perl/5.25.9/x86_64-linux
    /usr/local/lib/perl5/site_perl/5.25.9
    /usr/local/lib/perl5/5.25.9/x86_64-linux
    /usr/local/lib/perl5/5.25.9


Environment for perl 5.25.9:
    HOME=/home/afl
    LANG=en_US.UTF-8
    LANGUAGE=en_US:en
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/afl/perlbrew/bin:/home/afl/perlbrew/perls/perl-5.22.1/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games
    PERLBREW_BASHRC_VERSION=0.78
    PERLBREW_HOME=/home/afl/.perlbrew
    PERLBREW_MANPATH=/home/afl/perlbrew/perls/perl-5.22.1/man
    PERLBREW_PATH=/home/afl/perlbrew/bin:/home/afl/perlbrew/perls/perl-5.22.1/bin
    PERLBREW_PERL=perl-5.22.1
    PERLBREW_ROOT=/home/afl/perlbrew
    PERLBREW_VERSION=0.78
    PERL_BADLANG (unset)
    SHELL=/usr/bin/zsh

[p5pRT]

From @hvds

On Wed, 18 Jan 2017 17​:08​:36 -0800, randir wrote​:

While fuzzing perl v5.25.8-216-gfbceb79751 built with afl and run
under libdislocator, I found the following program

qr!@​{s{0})(?{!

to cause an assertion failure, even when run under -c for a syntax
check. This is a regression between v5.16.3 and v5.18.0, bisect points
to

491453b is the first bad commit
commit 491453b
Author​: David Mitchell <davem@​iabyn.com>
Date​: Wed Apr 17 17​:51​:16 2013 +0100

Handle /@​a/ array expansion within regex engine
[snip]
#3 0x00007f2d6ecf0ef2 in __GI___assert_fail (assertion=0x7f2d703af467
"PL_scopestack_ix == 1", file=0x7f2d703af370 "perl.c", line=571,
function=0x7f2d703b1a08 <__PRETTY_FUNCTION__.14855>
"perl_destruct") at assert.c​:101
#4 0x00007f2d7006fff7 in perl_destruct (my_perl=0x7f2d721f4010) at
perl.c​:571
#5 0x00007f2d7002fdc7 in main (argc=2, argv=0x7ffcc6177818,
env=0x7ffcc6177830) at perlmain.c​:134
(gdb) f 4
#4 0x00007f2d7006fff7 in perl_destruct (my_perl=0x7f2d721f4010) at
perl.c​:571
571 assert(PL_scopestack_ix == 1);
(gdb) p PL_scopestack_ix
$1 = 2

The scopestack imbalance here occurs because we call sublex_start() but never a corresponding sublex_done(). I don't currently have a clue what's intended to ensure we will reach sublex_done, but I'll try to make some more progress if nobody else jumps in.

The only part of 491453b that gets hit by ./miniperl -ce 'qr!@​{s{0})(?{!' is the toke.c chunk causing PL_lex_dojoin to become FALSE instead of TRUE.

Hugo

[p5pRT]

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

[p5pRT]

From @tonycoz

On Fri, 20 Jan 2017 06​:43​:12 -0800, hv wrote​:

On Wed, 18 Jan 2017 17​:08​:36 -0800, randir wrote​:

While fuzzing perl v5.25.8-216-gfbceb79751 built with afl and run
under libdislocator, I found the following program

qr!@​{s{0})(?{!

to cause an assertion failure, even when run under -c for a syntax
check. This is a regression between v5.16.3 and v5.18.0, bisect
points
to

491453b is the first bad commit
commit 491453b
Author​: David Mitchell <davem@​iabyn.com>
Date​: Wed Apr 17 17​:51​:16 2013 +0100

Handle /@​a/ array expansion within regex engine
[snip]
#3 0x00007f2d6ecf0ef2 in __GI___assert_fail
(assertion=0x7f2d703af467
"PL_scopestack_ix == 1", file=0x7f2d703af370 "perl.c", line=571,
function=0x7f2d703b1a08 <__PRETTY_FUNCTION__.14855>
"perl_destruct") at assert.c​:101
#4 0x00007f2d7006fff7 in perl_destruct (my_perl=0x7f2d721f4010) at
perl.c​:571
#5 0x00007f2d7002fdc7 in main (argc=2, argv=0x7ffcc6177818,
env=0x7ffcc6177830) at perlmain.c​:134
(gdb) f 4
#4 0x00007f2d7006fff7 in perl_destruct (my_perl=0x7f2d721f4010) at
perl.c​:571
571 assert(PL_scopestack_ix == 1);
(gdb) p PL_scopestack_ix
$1 = 2

The scopestack imbalance here occurs because we call sublex_start()
but never a corresponding sublex_done(). I don't currently have a clue
what's intended to ensure we will reach sublex_done, but I'll try to
make some more progress if nobody else jumps in.

The only part of 491453b that gets hit by ./miniperl -ce
'qr!@​{s{0})(?{!' is the toke.c chunk causing PL_lex_dojoin to become
FALSE instead of TRUE.

The closing ) is confusing the parser.

Patch which *might* fix it (passes all tests in a DEBUGGING build) attached.

Tony

[p5pRT]

From @tonycoz

0001-prevent-a-spurious-in-a-subparse-confusing-the-parse.patch

[p5pRT]

From @hvds

On Mon, 11 Feb 2019 21​:32​:43 -0800, tonyc wrote​:

On Fri, 20 Jan 2017 06​:43​:12 -0800, hv wrote​:

The scopestack imbalance here occurs because we call sublex_start()
but never a corresponding sublex_done().
[...]
The closing ) is confusing the parser.

Patch which *might* fix it (passes all tests in a DEBUGGING build) attached.

It looks credible to me, and a good solution. What are your concerns about it?

I know very little about the grammar, but I'm curious why "FUNC SUBLEXSTART optexpr SUBLEXEND" is in 'term' rather than next to "FUNC '(' optexpr ')'" in 'listop' - I'm assuming the cases hitting the former with the patch would previously have hit the latter.

Is it possible to construct an additional croak/toke test for the \U case, or does the means of construction imply it cannot trigger failure the same way?

Hugo

[p5pRT]

From @tonycoz

On Tue, 12 Feb 2019 01​:40​:32 -0800, hv wrote​:

On Mon, 11 Feb 2019 21​:32​:43 -0800, tonyc wrote​:

On Fri, 20 Jan 2017 06​:43​:12 -0800, hv wrote​:

The scopestack imbalance here occurs because we call sublex_start()
but never a corresponding sublex_done().
[...]
The closing ) is confusing the parser.

Patch which *might* fix it (passes all tests in a DEBUGGING build)
attached.

It looks credible to me, and a good solution. What are your concerns
about it?

Mostly I was confused by the parser doing this​:

$ ./perl -Ilib -MO=Deparse -e 'qq/@​{s{0}}/'
join $", @​s{'0'};
-e syntax OK

while I expected the s{0} to be treated as the beginning of a s///.

I know very little about the grammar, but I'm curious why "FUNC
SUBLEXSTART optexpr SUBLEXEND" is in 'term' rather than next to "FUNC
'(' optexpr ')'" in 'listop' - I'm assuming the cases hitting the
former with the patch would previously have hit the latter.

This is from the history of the patch. Originally I had an extra production called sublexexpr that I'd added to term, since first rule I converted to use the new tokens was the '(' expr ')' case for term.

But that rule isn't used - subparses always generate something like the FUNC() call (where FUNC might be op_stringify for "..."), and I ended up adding the FUNC rule to sublexexpr too.

Testing found that the {expr} sublex token rule wasn't being used so I removed it, and finally added the remaining rule to term. Which is probably the wrong place.

Is it possible to construct an additional croak/toke test for the \U
case, or does the means of construction imply it cannot trigger
failure the same way?

I think it can, though I couldn't reproduce it with an unpatched blead.

I've attached an updated patch that moves the rule, and some attempts at making qq() reproduce the problem, though I couldn't make them crash.

Tony

[p5pRT]

From @tonycoz

0001-prevent-a-spurious-in-a-subparse-confusing-the-parse.patch

[p5pRT]

From @tonycoz

On Tue, 12 Feb 2019 15​:03​:01 -0800, tonyc wrote​:

On Tue, 12 Feb 2019 01​:40​:32 -0800, hv wrote​:

On Mon, 11 Feb 2019 21​:32​:43 -0800, tonyc wrote​:

On Fri, 20 Jan 2017 06​:43​:12 -0800, hv wrote​:

The scopestack imbalance here occurs because we call
sublex_start()
but never a corresponding sublex_done().
[...]
The closing ) is confusing the parser.

Patch which *might* fix it (passes all tests in a DEBUGGING build)
attached.

It looks credible to me, and a good solution. What are your concerns
about it?

Mostly I was confused by the parser doing this​:

$ ./perl -Ilib -MO=Deparse -e 'qq/@​{s{0}}/'
join $", @​s{'0'};
-e syntax OK

while I expected the s{0} to be treated as the beginning of a s///.

I know very little about the grammar, but I'm curious why "FUNC
SUBLEXSTART optexpr SUBLEXEND" is in 'term' rather than next to "FUNC
'(' optexpr ')'" in 'listop' - I'm assuming the cases hitting the
former with the patch would previously have hit the latter.

This is from the history of the patch. Originally I had an extra
production called sublexexpr that I'd added to term, since first rule
I converted to use the new tokens was the '(' expr ')' case for term.

But that rule isn't used - subparses always generate something like
the FUNC() call (where FUNC might be op_stringify for "..."), and I
ended up adding the FUNC rule to sublexexpr too.

Testing found that the {expr} sublex token rule wasn't being used so I
removed it, and finally added the remaining rule to term. Which is
probably the wrong place.

Is it possible to construct an additional croak/toke test for the \U
case, or does the means of construction imply it cannot trigger
failure the same way?

I think it can, though I couldn't reproduce it with an unpatched
blead.

I've attached an updated patch that moves the rule, and some attempts
at making qq() reproduce the problem, though I couldn't make them
crash.

Tony

Applied as 69afcc2.

Tony

[p5pRT]

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