panic: pad_findmy_pvn illegal flag bits 0x20000000 #16979
Comments
From @dur-randirCreated by @dur-randirWhile fuzzing perl v5.29.9-63-g2496d8f3f7 built with afl and run use utf8; eval "sort \x{100}%"; warn $@; to emit 'panic: pad_findmy_pvn illegal flag bits 0x20000000 at (eval commit 2502ffd Make pad names always UTF8 Prior to 5.16, pad names never used the UTF8 flag, and all non-ASCII In 5.16, UTF8 handling was done ‘properly’, so that non-ASCII UTF8 Now, it is still the case that the only non-ASCII names to make their If we just assume that all pad names are UTF8 (which is true), then So this commit enforces what has always been the case and removes the Perl Info |
From @arcSergey Aleynikov (via RT) <perlbug-followup@perl.org> wrote:
Patch attached. Sawyer, is this OK to merge before 5.30? -- |
From @arc0001-RT-134061-don-t-call-pad_findmy_pvn-with-invalid-fla.patchFrom 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
|
|
The RT System itself - Status changed from 'new' to 'open' |
From @jkeenanOn Fri, 26 Apr 2019 13:06:15 GMT, arc wrote:
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. -- |
From @jkeenanOn Fri, 26 Apr 2019 13:49:01 GMT, jkeenan wrote:
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. -- |
From @khwilliamsonOn 4/26/19 7:54 AM, James E Keenan via RT wrote:
It is my experience that these reports from Sergey, once investigated, The ones that bisect to this development cycle might actually ought to But my point is that they really should be investigated to be sure.
|
From @xsawyerxOn Fri, 26 Apr 2019 06:06:15 -0700, arc wrote:
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. :) |
From @tonycozOn Fri, 26 Apr 2019 06:06:15 -0700, arc wrote:
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 |
From @tonycozOn Sat, 27 Apr 2019 02:31:33 -0700, tonyc wrote:
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 |
From @xsawyerxOn 5/1/19 4:46 AM, Tony Cook via RT wrote:
So you feel confident about merging it now? |
From @tonycozOn Thu, May 02, 2019 at 03:19:47PM +0300, Sawyer X wrote:
Yes. Tony |
From @xsawyerxThen go for it. On 5/2/19 4:18 PM, Tony Cook wrote:
|
From @khwilliamsonOn 5/2/19 7:25 AM, Sawyer X wrote:
We need a commit message before it can be merged.
|
From @tonycozOn Thu, 02 May 2019 09:06:26 -0700, public@khwilliamson.com wrote:
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 |
|
@tonycoz - Status changed from 'open' to 'pending release' |
From @khwilliamsonThank you for filing this report. You have helped make Perl better. With the release today of Perl 5.30.0, this and 160 other issues have been Perl 5.30.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#134061 (status was 'resolved')
Searchable as RT134061$
The text was updated successfully, but these errors were encountered: