panic: pad_findmy_pvn illegal flag bits 0x20000000

[p5pRT]

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

Searchable as RT134061$

[p5pRT]

From @dur-randir

Created by @dur-randir

While fuzzing perl v5.29.9-63-g2496d8f3f7 built with afl and run
under libdislocator, I found the following program

use utf8; eval "sort \x{100}%"; warn $@​;

to emit 'panic​: pad_findmy_pvn illegal flag bits 0x20000000 at (eval
1) line 1.' diagnostics.
This is a regression between 5.20 and 5.22, bisect points to

commit 2502ffd
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Mon Nov 24 00​:00​:51 2014 -0800

  Make pad names always UTF8

  Prior to 5.16, pad names never used the UTF8 flag, and all non-ASCII
  pad names were in UTF8. Because the latter was consistently true,
  everything just worked anyway.

  In 5.16, UTF8 handling was done ‘properly’, so that non-ASCII UTF8
  strings were always accompanied by the UTF8 flag.

  Now, it is still the case that the only non-ASCII names to make their
  way into pad name code are in UTF8. Since ASCII is a subset of UTF8,
  we effectively *always* have UTF8 pad names. So the flag handling is
  actually redundant.

  If we just assume that all pad names are UTF8 (which is true), then
  we don’t need to bother with the flag checking. There is actually
  no reason why we should have two different encodings for storing
  pad names.

  So this commit enforces what has always been the case and removes the
  extra code for converting between Latin-1 and UTF8. Nothing on CPAN
  is using the UTF8 flag with pads, so nothing should break. In fact,
  we never documented padadd_UTF8_NAME.

Perl Info

Flags:
    category=core
    severity=low

Site configuration information for perl 5.29.9:

Configured by dur-randir at Wed Feb 27 14:51:01 MSK 2019.

Summary of my perl5 (revision 5 version 29 subversion 9) configuration:
  Commit id: c1e47bad34ce1d9c84ed57c9b8978bcbd5a02e98
  Platform:
    osname=darwin
    osvers=13.4.0
    archname=darwin-thread-multi-2level
    uname='darwin isengard.local 13.4.0 darwin kernel version 13.4.0:
mon jan 11 18:17:34 pst 2016; root:xnu-2422.115.15~1release_x86_64
x86_64 '
    config_args='-de -Dusedevel -DDEBUGGING -Dusethreads'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=define
    usemultiplicity=define
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
    bincompat5005=undef
  Compiler:
    cc='cc'
    ccflags ='-fno-common -DPERL_DARWIN -mmacosx-version-min=10.9
-DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector
-I/usr/local/include -DPERL_USE_SAFE_PUTENV'
    optimize='-O3 -g'
    cppflags='-fno-common -DPERL_DARWIN -mmacosx-version-min=10.9
-DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector
-I/usr/local/include'
    ccversion=''
    gccversion='4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.56)'
    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='cc'
    ldflags =' -mmacosx-version-min=10.9 -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/clang/6.0/lib
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib
/usr/lib
    libs=-lpthread -lgdbm -ldbm -ldl -lm -lutil -lc
    perllibs=-lpthread -ldl -lm -lutil -lc
    libc=
    so=dylib
    useshrplib=false
    libperl=libperl.a
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=bundle
    d_dlsymun=undef
    ccdlflags=' '
    cccdlflags=' '
    lddlflags=' -mmacosx-version-min=10.9 -bundle -undefined
dynamic_lookup -L/usr/local/lib -fstack-protector'



@INC for perl 5.29.9:
    lib
    /usr/local/lib/perl5/site_perl/5.29.9/darwin-thread-multi-2level
    /usr/local/lib/perl5/site_perl/5.29.9
    /usr/local/lib/perl5/5.29.9/darwin-thread-multi-2level
    /usr/local/lib/perl5/5.29.9


Environment for perl 5.29.9:
    DYLD_LIBRARY_PATH (unset)
    HOME=/Users/dur-randir
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/Users/dur-randir/perlbrew/bin:/Users/dur-randir/perlbrew/perls/perl-5.22.1/bin:/usr/local/bin:/usr/local/sbin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/texbin
    PERLBREW_HOME=/Users/dur-randir/.perlbrew
    PERLBREW_MANPATH=/Users/dur-randir/perlbrew/perls/perl-5.22.1/man
    PERLBREW_PATH=/Users/dur-randir/perlbrew/bin:/Users/dur-randir/perlbrew/perls/perl-5.22.1/bin
    PERLBREW_PERL=perl-5.22.1
    PERLBREW_ROOT=/Users/dur-randir/perlbrew
    PERLBREW_SHELLRC_VERSION=0.84
    PERLBREW_VERSION=0.84
    PERL_BADLANG (unset)
    SHELL=/usr/local/bin/zsh

[p5pRT]

From @arc

Sergey Aleynikov (via RT) <perlbug-followup@​perl.org> wrote​:

use utf8; eval "sort \x{100}%"; warn $@​;
panic​: pad_findmy_pvn illegal flag bits 0x20000000 at (eval 1) line 1.

Patch attached.

Sawyer, is this OK to merge before 5.30?

--
Aaron Crane

[p5pRT]

From @arc

0001-RT-134061-don-t-call-pad_findmy_pvn-with-invalid-fla.patch

From 4d729748798c27c7b7da4487debb12763938d52e Mon Sep 17 00:00:00 2001
From: Aaron Crane <arc@cpan.org>
Date: Fri, 26 Apr 2019 14:02:47 +0100
Subject: [PATCH] RT#134061: don't call pad_findmy_pvn() with invalid flags

---
 op.c                |  2 +-
 t/comp/parser_run.t | 10 +++++++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/op.c b/op.c
index 1862f1b11b..74761ca421 100644
--- a/op.c
+++ b/op.c
@@ -12732,7 +12732,7 @@ Perl_ck_sort(pTHX_ OP *o)
 	    *tmpbuf = '&';
 	    assert (len < 256);
 	    Copy(name, tmpbuf+1, len, char);
-	    off = pad_findmy_pvn(tmpbuf, len+1, SvUTF8(kSVOP_sv));
+	    off = pad_findmy_pvn(tmpbuf, len+1, 0);
 	    if (off != NOT_IN_PAD) {
 		if (PAD_COMPNAME_FLAGS_isOUR(off)) {
 		    SV * const fq =
diff --git a/t/comp/parser_run.t b/t/comp/parser_run.t
index 79b669d807..4179880497 100644
--- a/t/comp/parser_run.t
+++ b/t/comp/parser_run.t
@@ -10,7 +10,7 @@ BEGIN {
     set_up_inc( qw(. ../lib ) );
 }
 
-plan(5);
+plan(6);
 
 # [perl #130814] can reallocate lineptr while looking ahead for
 # "Missing $ on loop variable" diagnostic.
@@ -55,5 +55,13 @@ syntax error at - line 1, at EOF
 Execution of - aborted due to compilation errors.
 EXPECTED
 
+fresh_perl_is(<<'EOS', <<'EXPECT', {}, 'no panic in pad_findmy_pvn (#134061)');
+use utf8;
+eval "sort \x{100}%";
+die $@;
+EOS
+syntax error at (eval 1) line 1, at EOF
+EXPECT
+
 __END__
 # ex: set ts=8 sts=4 sw=4 et:
-- 
2.18.0

[p5pRT]

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

[p5pRT]

From @jkeenan

On Fri, 26 Apr 2019 13​:06​:15 GMT, arc wrote​:

Sergey Aleynikov (via RT) <perlbug-followup@​perl.org> wrote​:

use utf8; eval "sort \x{100}%"; warn $@​;
panic​: pad_findmy_pvn illegal flag bits 0x20000000 at (eval 1) line 1.

Patch attached.

Sawyer, is this OK to merge before 5.30?

My two cents​: At this point in the dev cycle we should not be making changes in *.c or *.h files. The potential ramifications of changes to the guts are vast. Our ability to thoroughly assess the impact in the time remaining to production release is very limited.

That's why we have code freezes.

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

[p5pRT]

From @jkeenan

On Fri, 26 Apr 2019 13​:49​:01 GMT, jkeenan wrote​:

On Fri, 26 Apr 2019 13​:06​:15 GMT, arc wrote​:

Sergey Aleynikov (via RT) <perlbug-followup@​perl.org> wrote​:

use utf8; eval "sort \x{100}%"; warn $@​;
panic​: pad_findmy_pvn illegal flag bits 0x20000000 at (eval 1) line
1.

Patch attached.

Sawyer, is this OK to merge before 5.30?

My two cents​: At this point in the dev cycle we should not be making
changes in *.c or *.h files. The potential ramifications of changes
to the guts are vast. Our ability to thoroughly assess the impact in
the time remaining to production release is very limited.

That's why we have code freezes.

I would like to add my thanks to Sergey Aleynikov for the tremendous work he has done during this development cycle at identifying weaknesses in our source code with his fuzzing work. See, for example, all the items added to our "fuzzer-detected issues" META ticket (https://rt-archive.perl.org/perl5/Ticket/Display.html?id=126546) which come from his work.

Nonetheless, we have generally not been treating these bug reports as blockers for perl-5.30, whether or not we have been able to write patches for them. If we're not treating this particular RT as a blocker, then we should not be applying a patch for it during code freeze.

Thank you very much.

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

[p5pRT]

From @khwilliamson

On 4/26/19 7​:54 AM, James E Keenan via RT wrote​:

On Fri, 26 Apr 2019 13​:49​:01 GMT, jkeenan wrote​:

On Fri, 26 Apr 2019 13​:06​:15 GMT, arc wrote​:

Sergey Aleynikov (via RT) <perlbug-followup@​perl.org> wrote​:

use utf8; eval "sort \x{100}%"; warn $@​;
panic​: pad_findmy_pvn illegal flag bits 0x20000000 at (eval 1) line
1.

Patch attached.

Sawyer, is this OK to merge before 5.30?

My two cents​: At this point in the dev cycle we should not be making
changes in *.c or *.h files. The potential ramifications of changes
to the guts are vast. Our ability to thoroughly assess the impact in
the time remaining to production release is very limited.

That's why we have code freezes.

I would like to add my thanks to Sergey Aleynikov for the tremendous work he has done during this development cycle at identifying weaknesses in our source code with his fuzzing work. See, for example, all the items added to our "fuzzer-detected issues" META ticket (https://rt-archive.perl.org/perl5/Ticket/Display.html?id=126546) which come from his work.

Nonetheless, we have generally not been treating these bug reports as blockers for perl-5.30, whether or not we have been able to write patches for them. If we're not treating this particular RT as a blocker, then we should not be applying a patch for it during code freeze.

It is my experience that these reports from Sergey, once investigated,
show real underlying problems that should be fixed; they don't tend to
be just edge cases that won't ever come up in real code, but symptomatic
of real code potential issues once reduced to simpler test cases

The ones that bisect to this development cycle might actually ought to
be blockers, depending on the outcome of an investigation.

But my point is that they really should be investigated to be sure.
I've looked so far at all but two of the ones for the areas I have
expertise in.

Thank you very much.

[p5pRT]

From @xsawyerx

On Fri, 26 Apr 2019 06​:06​:15 -0700, arc wrote​:

Sergey Aleynikov (via RT) <perlbug-followup@​perl.org> wrote​:

use utf8; eval "sort \x{100}%"; warn $@​;
panic​: pad_findmy_pvn illegal flag bits 0x20000000 at (eval 1) line 1.

Patch attached.

Sawyer, is this OK to merge before 5.30?

I didn't see an email on this response.

Generally, the answer would be no. In this case, it seems like a very small patch (with a test) that might just be okay enough. My only concern is whether this could reintroduce some breakage for which we do not have a test? Will it break some CPAN module that depends on this incorrect behavior? (I'm not sure how one even could do that, but... maybe?)

I would prefer another person with strong UTF background (say, Karl) to back this up.

Regardless, Sergey's work and contributions are very much appreciated, both by me and by the other core developers. Thank you for helping us create a better Perl. :)

[p5pRT]

From @tonycoz

On Fri, 26 Apr 2019 06​:06​:15 -0700, arc wrote​:

Sergey Aleynikov (via RT) <perlbug-followup@​perl.org> wrote​:

use utf8; eval "sort \x{100}%"; warn $@​;
panic​: pad_findmy_pvn illegal flag bits 0x20000000 at (eval 1) line 1.

Patch attached.

Sawyer, is this OK to merge before 5.30?

I'm not sure the patch is correct.

With sprout's change pad_findmy_pvn() now expects a UTF-8 string.

Is there anything that forces the SV to UTF-8?

Tony

[p5pRT]

From @tonycoz

On Sat, 27 Apr 2019 02​:31​:33 -0700, tonyc wrote​:

On Fri, 26 Apr 2019 06​:06​:15 -0700, arc wrote​:

Sergey Aleynikov (via RT) <perlbug-followup@​perl.org> wrote​:

use utf8; eval "sort \x{100}%"; warn $@​;
panic​: pad_findmy_pvn illegal flag bits 0x20000000 at (eval 1) line 1.

Patch attached.

Sawyer, is this OK to merge before 5.30?

I'm not sure the patch is correct.

With sprout's change pad_findmy_pvn() now expects a UTF-8 string.

Is there anything that forces the SV to UTF-8?

There is when it matters.

Non-ASCII barewords are only allowed under C<use utf8;>, and when use utf8 is on the SV will be UTF-8 encoded.

Without the use utf8 only invariant strings can be in the SV and so it's safe to pass the string to pad_findmy_pvn() without upgrading it.

So my concern here was unjustified.

Tony

[p5pRT]

From @xsawyerx

On 5/1/19 4​:46 AM, Tony Cook via RT wrote​:

On Sat, 27 Apr 2019 02​:31​:33 -0700, tonyc wrote​:

On Fri, 26 Apr 2019 06​:06​:15 -0700, arc wrote​:

Sergey Aleynikov (via RT) <perlbug-followup@​perl.org> wrote​:

use utf8; eval "sort \x{100}%"; warn $@​;
panic​: pad_findmy_pvn illegal flag bits 0x20000000 at (eval 1) line 1.
Patch attached.

Sawyer, is this OK to merge before 5.30?
I'm not sure the patch is correct.

With sprout's change pad_findmy_pvn() now expects a UTF-8 string.

Is there anything that forces the SV to UTF-8?
There is when it matters.

Non-ASCII barewords are only allowed under C<use utf8;>, and when use utf8 is on the SV will be UTF-8 encoded.

Without the use utf8 only invariant strings can be in the SV and so it's safe to pass the string to pad_findmy_pvn() without upgrading it.

So my concern here was unjustified.

So you feel confident about merging it now?

[p5pRT]

From @tonycoz

On Thu, May 02, 2019 at 03​:19​:47PM +0300, Sawyer X wrote​:

On 5/1/19 4​:46 AM, Tony Cook via RT wrote​:

On Sat, 27 Apr 2019 02​:31​:33 -0700, tonyc wrote​:

On Fri, 26 Apr 2019 06​:06​:15 -0700, arc wrote​:

Sergey Aleynikov (via RT) <perlbug-followup@​perl.org> wrote​:

use utf8; eval "sort \x{100}%"; warn $@​;
panic​: pad_findmy_pvn illegal flag bits 0x20000000 at (eval 1) line 1.
Patch attached.

Sawyer, is this OK to merge before 5.30?
I'm not sure the patch is correct.

With sprout's change pad_findmy_pvn() now expects a UTF-8 string.

Is there anything that forces the SV to UTF-8?
There is when it matters.

Non-ASCII barewords are only allowed under C<use utf8;>, and when use utf8 is on the SV will be UTF-8 encoded.

Without the use utf8 only invariant strings can be in the SV and so it's safe to pass the string to pad_findmy_pvn() without upgrading it.

So my concern here was unjustified.

So you feel confident about merging it now?

Yes.

Tony

[p5pRT]

From @xsawyerx

Then go for it.

On 5/2/19 4​:18 PM, Tony Cook wrote​:

On Thu, May 02, 2019 at 03​:19​:47PM +0300, Sawyer X wrote​:

On 5/1/19 4​:46 AM, Tony Cook via RT wrote​:

On Sat, 27 Apr 2019 02​:31​:33 -0700, tonyc wrote​:

On Fri, 26 Apr 2019 06​:06​:15 -0700, arc wrote​:

Sergey Aleynikov (via RT) <perlbug-followup@​perl.org> wrote​:

use utf8; eval "sort \x{100}%"; warn $@​;
panic​: pad_findmy_pvn illegal flag bits 0x20000000 at (eval 1) line 1.
Patch attached.

Sawyer, is this OK to merge before 5.30?
I'm not sure the patch is correct.

With sprout's change pad_findmy_pvn() now expects a UTF-8 string.

Is there anything that forces the SV to UTF-8?
There is when it matters.

Non-ASCII barewords are only allowed under C<use utf8;>, and when use utf8 is on the SV will be UTF-8 encoded.

Without the use utf8 only invariant strings can be in the SV and so it's safe to pass the string to pad_findmy_pvn() without upgrading it.

So my concern here was unjustified.

So you feel confident about merging it now?
Yes.

Tony

[p5pRT]

From @khwilliamson

On 5/2/19 7​:25 AM, Sawyer X wrote​:

Then go for it.

We need a commit message before it can be merged.

On 5/2/19 4​:18 PM, Tony Cook wrote​:

On Thu, May 02, 2019 at 03​:19​:47PM +0300, Sawyer X wrote​:

On 5/1/19 4​:46 AM, Tony Cook via RT wrote​:

On Sat, 27 Apr 2019 02​:31​:33 -0700, tonyc wrote​:

On Fri, 26 Apr 2019 06​:06​:15 -0700, arc wrote​:

Sergey Aleynikov (via RT) <perlbug-followup@​perl.org> wrote​:

use utf8; eval "sort \x{100}%"; warn $@​;
panic​: pad_findmy_pvn illegal flag bits 0x20000000 at (eval 1) line 1.
Patch attached.

Sawyer, is this OK to merge before 5.30?
I'm not sure the patch is correct.

With sprout's change pad_findmy_pvn() now expects a UTF-8 string.

Is there anything that forces the SV to UTF-8?
There is when it matters.

Non-ASCII barewords are only allowed under C<use utf8;>, and when use utf8 is on the SV will be UTF-8 encoded.

Without the use utf8 only invariant strings can be in the SV and so it's safe to pass the string to pad_findmy_pvn() without upgrading it.

So my concern here was unjustified.

So you feel confident about merging it now?
Yes.

Tony

[p5pRT]

From @tonycoz

On Thu, 02 May 2019 09​:06​:26 -0700, public@​khwilliamson.com wrote​:

On 5/2/19 7​:25 AM, Sawyer X wrote​:

Then go for it.

We need a commit message before it can be merged.

It had a commit message, I didn't immediately apply it because it was 11​:18pm (and it turned out to fail test_porting).

Applied as 9a48f2a.

porting/test_bootstrap.t disallows most use of use in t/comp, so I moved the test to t/uni/parser.t in 5a62190.

Tony