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
Perl_sv_2pv_flags: Assertion `((svtype)((sv)->sv_flags & 0xff)) != SVt_PVAV && ((svtype)((sv)->sv_flags & 0xff)) != SVt_PVHV && ((svtype)((sv)->sv_flags & 0xff)) != SVt_PVFM' failed #14668
Comments
From @geeknikBuilt v5.21.12 (v5.21.11-10-ga8f582b) using the following command line: ./Configure -des -Dusedevel -DDEBUGGING -Dcc=afl-gcc -Doptimize=-O2\ -g && AFL_HARDEN=1 make -j6 test-prep Bug found with AFL (http://lcamtuf.coredump.cx/afl) GDB output: Program received signal SIGABRT, Aborted. System Info: Debian 7, Kernel 3.2.65-1+deb7u2 x86_64, GCC 4.9.2, libc 2.13-38+deb7u8 |
From @geeknikI minimized the attached test case with afl-tmin from 469 bytes to 50 bytes, however, the minimized test case didn't cause the assertion failure and caused GDB and Valgrind to hang. Reduced test case: Hexdump: |
From [Unknown Contact. See original ticket]I minimized the attached test case with afl-tmin from 469 bytes to 50 bytes, however, the minimized test case didn't cause the assertion failure and caused GDB and Valgrind to hang. Reduced test case: Hexdump: |
From @iabynOn Thu, Apr 23, 2015 at 01:57:08PM -0700, Brian Carpenter via RT wrote:
I can't reproduce the failure with the original code (no assertion
That hangs because the <> is reading from stdin. -- |
From @hvdsOn Wed Apr 29 04:14:52 2015, davem wrote:
I can, with a plain build of blead. Most of the original code is red herrings, I was able to reduce it to this: % ./miniperl -e '@a = qw{x y}; for (@a) { s{.}{}go =~ @b }' Are you able to reproduce it that way? If not I may be able to make some time to dig more over the weekend. Hugo |
The RT System itself - Status changed from 'new' to 'open' |
From @tonycozOn Wed Apr 29 12:07:25 2015, hv wrote:
The problem seems to be this code in pp_regcomp: /* can't change the optree at runtime either */ before this starts pm point at the match op and PL_curpm points at the subst op. RX_PRELEN() is zero for rx, so pm is updated, and since the subst has the /o flag (PMf_KEEP) the op tree is rewritten to skip the regcomp for the second time around the loop, leaving the raw AV on the stack as the regexp for pp_match. Hugo's example doesn't need the /g flag: $ ./miniperl -e '@a = qw{x y}; for (@a) { s{.}{}o =~ @b }' Tested on a non-threaded perl. Tony |
From @iabynOn Wed, Apr 29, 2015 at 10:22:00PM -0700, Tony Cook via RT wrote:
This bug has in fact been present since 5.0 or before; it's just that The main issue is the mis-implementation and interaction between two In this case, only the C<g> and C<c> flags on the empty pattern are So in something like the following: /./o; if @b is empty, then (since 5.18.0), it evaluates to the empty string, This is fixable, but first we need to agree on what the semantics should for (...) { on executing /$expr/ if $expr evaluates to the null string, should it Conversely, should /$expr/o honour the //o flag even if $expr is null? My opinion is that, like c and g, the o flag should be honoured on the So I propose that: /abc/o; $foo =~ /$null/ matches $foo against "abc" each time round the loop (it currently crashes /abc/; $foo =~ /$null/o always only evaluates $null once; if that happens to be the empty string, Note that nothing in the test suite currently tests these cases as far as -- |
From @tonycozOn Fri May 08 13:58:07 2015, davem wrote:
That makes sense to me. Or we could take /$null/ meaning 'use the last match' out the back and Tony |
From @rjbsOn Fri May 08 13:58:07 2015, davem wrote:
I think I may be missing something here. It sounds like it's legal to say ($x =~ @a) to mean ($x =~ "@a") but that this violates an assertion and so crashes debugging builds. Is this only a non-blocker because it was already broken in 5.20, or for some other reason? My initial reaction is: "won't people running debugging builds be irritated that they can't debug code that is legal otherwise?" As I say, I might be missing something. I don't use debugging builds almost ever.
This seems fine to me as a v5.23 change. -- |
From @iabynOn Mon, May 11, 2015 at 08:20:17PM -0700, Ricardo SIGNES via RT wrote:
No, $x =~ @a is perfectly legal and non-problematic (except that its The issue is that on unthreaded builds going back to 5.000, in something for (....) { where expr evaluates (at least once) to the null string, the optree for This is why I don't think it should be a 5.22 blocker. -- |
From @rjbs* Dave Mitchell <davem@iabyn.com> [2015-05-12T06:22:22]
Got it. Agreed, thanks. -- |
From @ap* Tony Cook via RT <perlbug-followup@perl.org> [2015-05-12 02:30]:
Me too. -- |
From @geeknikperl -e 'sub C0000(){0}C0000 C0000' causes this assertion failure in v5.23.7 (v5.23.6-119-gbc5be89). |
From [Unknown Contact. See original ticket]perl -e 'sub C0000(){0}C0000 C0000' causes this assertion failure in v5.23.7 (v5.23.6-119-gbc5be89). |
From @iabynOn Mon, May 11, 2015 at 05:29:05PM -0700, Tony Cook via RT wrote:
How about we make a distinction between literal //, and /$null/? Then make -- |
From @rjbs* Dave Mitchell <davem@iabyn.com> [2016-01-19T10:57:17]
So, I bet this was heavily debated back in the year 2011, for #96230... but I Then I guessed it was debated during the discussion of changing "split / /". In review, though, I think this won't end up being worth it. Dave's suggestion -- |
From @geeknikTriggered in Perl v5.25.5-8-g3c42ae1 with the following: ./perl -e '$0=s0::;chop@0=~y:::' |
From [Unknown Contact. See original ticket]Triggered in Perl v5.25.5-8-g3c42ae1 with the following: ./perl -e '$0=s0::;chop@0=~y:::' |
From @jkeenanOn Thu, 23 Apr 2015 09:31:36 GMT, brian.carpenter@gmail.com wrote:
This link is now giving a 404. Can someone provide a better link so that we can learn about "AFL"? Thanks.
-- |
From @geeknikI just checked http://lcamtuf.coredump.cx/afl and it loads just fine with |
From @jkeenanOn Mon, 21 Nov 2016 23:14:13 GMT, brian.carpenter@gmail.com wrote:
Yes, you are correct. For some reason in my browser the link was picking up the trailing ')', which resulted in the 404. Thank you very much. |
From @hvdsOn Sun, 25 Sep 2016 15:01:37 -0700, brian.carpenter@gmail.com wrote:
This is an unrelated issue. I've created #130198 to track it, with some analysis. Hugo |
From @tonycozOn Fri, 08 May 2015 13:58:07 -0700, davem wrote:
It wasn't clear to me from your post, but this actually behaves differently between threaded and non-threaded builds - for threaded builds it behaves as you describe above - /o is honoured on the op with /o and isn't inherited by and empty match without the /o.
The attached covers both these cases, I think. Tony |
From @tonycoz0001-perl-124368-make-foo-o-null-act-consistently.patchFrom 154ad615384c3333a75c9915621cac9ddcbd49aa Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Tue, 4 Jul 2017 11:44:06 +1000
Subject: (perl #124368) make /foo/o; /$null/ act consistently
Previously the /o would be inherited by the second match if the first
match was successful, but only on non-threaded builds.
The op-tree rewriting done on non-threaded builds could also confuse
the interpreter, possibly resulting in the match op receiving
the argument intended for the regcomp op.
---
pp_ctl.c | 10 ++--------
t/re/pat.t | 17 ++++++++++++++++-
2 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/pp_ctl.c b/pp_ctl.c
index db5eabe..d903a4c 100644
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -163,15 +163,9 @@ PP(pp_regcomp)
/* handle the empty pattern */
if (!RX_PRELEN(PM_GETRE(pm)) && PL_curpm) {
if (PL_curpm == PL_reg_curpm) {
- if (PL_curpm_under) {
- if (PL_curpm_under == PL_reg_curpm) {
- Perl_croak(aTHX_ "Infinite recursion via empty pattern");
- } else {
- pm = PL_curpm_under;
- }
+ if (PL_curpm_under && PL_curpm_under == PL_reg_curpm) {
+ Perl_croak(aTHX_ "Infinite recursion via empty pattern");
}
- } else {
- pm = PL_curpm;
}
}
diff --git a/t/re/pat.t b/t/re/pat.t
index fb6d4c4..cf97ecd 100644
--- a/t/re/pat.t
+++ b/t/re/pat.t
@@ -23,7 +23,7 @@ BEGIN {
skip_all('no re module') unless defined &DynaLoader::boot_DynaLoader;
skip_all_without_unicode_tables();
-plan tests => 837; # Update this when adding/deleting tests.
+plan tests => 843; # Update this when adding/deleting tests.
run_tests() unless caller;
@@ -138,6 +138,21 @@ sub run_tests {
$null = "";
$xyz =~ /$null/;
is($&, $xyz, $message);
+
+ # each entry: regexp, match string, $&, //o match success
+ my @tests =
+ (
+ [ "", "xy", "x", 1 ],
+ [ "y", "yz", "y", !1 ],
+ );
+ for my $test (@tests) {
+ my ($re, $str, $matched, $omatch) = @$test;
+ $xyz =~ /x/o;
+ ok($str =~ /$re/, "$str matches /$re/");
+ is($&, $matched, "on $matched");
+ $xyz =~ /x/o;
+ is($str =~ /$re/o, $omatch, "$str matches /$re/o (or not)");
+ }
}
{
--
2.1.4
|
From @tonycozOn Mon, 03 Jul 2017 19:04:51 -0700, tonyc wrote:
Applied as 3cb4cde. 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 yesterday of Perl 5.28.0, this and 185 other issues have been Perl 5.28.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#124368 (status was 'resolved')
Searchable as RT124368$
The text was updated successfully, but these errors were encountered: