perl: sv.c:12530: void Perl_sv_vcatpvfn_flags() Assertion '0' failed.

[p5pRT]

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

Searchable as RT132163$

[p5pRT]

From @geeknik

Triggered while fuzzing v5.27.4-28-g60dfa51.

./perl -e '$p00="[\0\\N{U+.}";qr/$p00/'

perl​: sv.c​:12530​: void Perl_sv_vcatpvfn_flags(SV *const, const char
*const, const STRLEN, va_list *const, SV **const, const size_t, _Bool
*const, const U32)​: Assertion `0' failed.
Aborted

[p5pRT]

From @tonycoz

On Mon, 25 Sep 2017 16​:21​:55 -0700, brian.carpenter@​gmail.com wrote​:

Triggered while fuzzing v5.27.4-28-g60dfa51.

./perl -e '$p00="[\0\\N{U+.}";qr/$p00/'

perl​: sv.c​:12530​: void Perl_sv_vcatpvfn_flags(SV *const, const char
*const, const STRLEN, va_list *const, SV **const, const size_t, _Bool
*const, const U32)​: Assertion `0' failed.
Aborted

Backtrace​:

...
#4 0x000000000061d537 in Perl_sv_vcatpvfn_flags (sv=0xacc3c8,
  pat=0x7fffffffc050 "Missing right brace on \\%c{} in regex; marked by <-- HERE in m/%d%lu%4p <-- HERE %d%lu%4p/\n", patlen=91, args=0x7fffffffc038,
  svargs=0x0, sv_count=0, maybe_tainted=0x0, flags=0) at sv.c​:12530
#5 0x00000000006190e0 in Perl_sv_vsetpvfn (sv=0xacc3c8,
  pat=0x7fffffffc050 "Missing right brace on \\%c{} in regex; marked by <-- HERE in m/%d%lu%4p <-- HERE %d%lu%4p/\n", patlen=91, args=0x7fffffffc038,
  svargs=0x0, sv_count=0, maybe_tainted=0x0) at sv.c​:11007
#6 0x000000000056647c in Perl_vmess (
  pat=0x7fffffffc050 "Missing right brace on \\%c{} in regex; marked by <-- HERE in m/%d%lu%4p <-- HERE %d%lu%4p/\n", args=0x7fffffffc038) at util.c​:1487
#7 0x000000000054afa1 in S_re_croak2 (utf8=false,
  pat1=0x7e0488 "Missing right brace on \\%c{}",
  pat2=0x7ded50 " in regex; marked by <-- HERE in m/%d%lu%4p <-- HERE %d%lu%4p/") at regcomp.c​:20050
#8 0x0000000000527f05 in S_grok_bslash_N (pRExC_state=0x7fffffffe0a0,
  node_p=0x7fffffffc4e8, code_point_p=0x0, cp_count=0x0,
  flagp=0x7fffffffc7d0, strict=false, depth=9) at regcomp.c​:12059
#9 0x000000000052ccd1 in S_regatom (pRExC_state=0x7fffffffe0a0,
  flagp=0x7fffffffc7d0, depth=9) at regcomp.c​:12971
#10 0x0000000000525238 in S_regpiece (pRExC_state=0x7fffffffe0a0,
  flagp=0x7fffffffc8fc, depth=8) at regcomp.c​:11708
...

which looks embarassing similar to 131598.

Tony

[p5pRT]

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

[p5pRT]

From @khwilliamson

On 09/25/2017 06​:44 PM, Tony Cook via RT wrote​:

On Mon, 25 Sep 2017 16​:21​:55 -0700, brian.carpenter@​gmail.com wrote​:

Triggered while fuzzing v5.27.4-28-g60dfa51.

./perl -e '$p00="[\0\\N{U+.}";qr/$p00/'

perl​: sv.c​:12530​: void Perl_sv_vcatpvfn_flags(SV *const, const char
*const, const STRLEN, va_list *const, SV **const, const size_t, _Bool
*const, const U32)​: Assertion `0' failed.
Aborted

Backtrace​:

...
#4 0x000000000061d537 in Perl_sv_vcatpvfn_flags (sv=0xacc3c8,
pat=0x7fffffffc050 "Missing right brace on \\%c{} in regex; marked by <-- HERE in m/%d%lu%4p <-- HERE %d%lu%4p/\n", patlen=91, args=0x7fffffffc038,
svargs=0x0, sv_count=0, maybe_tainted=0x0, flags=0) at sv.c​:12530
#5 0x00000000006190e0 in Perl_sv_vsetpvfn (sv=0xacc3c8,
pat=0x7fffffffc050 "Missing right brace on \\%c{} in regex; marked by <-- HERE in m/%d%lu%4p <-- HERE %d%lu%4p/\n", patlen=91, args=0x7fffffffc038,
svargs=0x0, sv_count=0, maybe_tainted=0x0) at sv.c​:11007
#6 0x000000000056647c in Perl_vmess (
pat=0x7fffffffc050 "Missing right brace on \\%c{} in regex; marked by <-- HERE in m/%d%lu%4p <-- HERE %d%lu%4p/\n", args=0x7fffffffc038) at util.c​:1487
#7 0x000000000054afa1 in S_re_croak2 (utf8=false,
pat1=0x7e0488 "Missing right brace on \\%c{}",
pat2=0x7ded50 " in regex; marked by <-- HERE in m/%d%lu%4p <-- HERE %d%lu%4p/") at regcomp.c​:20050
#8 0x0000000000527f05 in S_grok_bslash_N (pRExC_state=0x7fffffffe0a0,
node_p=0x7fffffffc4e8, code_point_p=0x0, cp_count=0x0,
flagp=0x7fffffffc7d0, strict=false, depth=9) at regcomp.c​:12059
#9 0x000000000052ccd1 in S_regatom (pRExC_state=0x7fffffffe0a0,
flagp=0x7fffffffc7d0, depth=9) at regcomp.c​:12971
#10 0x0000000000525238 in S_regpiece (pRExC_state=0x7fffffffe0a0,
flagp=0x7fffffffc8fc, depth=8) at regcomp.c​:11708
...

which looks embarassing similar to 131598.

But it isn't the same cause. I haven't had lately and won't have time
to do much on this project in the next few weeks. My guess on this is
that it is some flaw in REPORT_LOCATION_ARGS()

[p5pRT]

From @atoomic

I agree with Tony Cook this is a problem in REPORT_LOCATION_ARGS,
when calling vFAIL2 from S_grok_bslash_N

1/ minor, we should not use vFAIL2 as the second arg is a 'N', the vFAIL is probably more appropriate there.

2/ I confirm that using FAIL or FAIL2 instead of the vFAIL family fixes the issue

I think the problem is to use a negative offset for the length in REPORT_LOCATION_ARGS (view attached patch)

note, as it a few tests are failing from re/regexp.t & co, can adjust them, but want to confirm that the suggested patch is correct first

On Mon, 25 Sep 2017 21​:48​:24 -0700, public@​khwilliamson.com wrote​:

On 09/25/2017 06​:44 PM, Tony Cook via RT wrote​:

On Mon, 25 Sep 2017 16​:21​:55 -0700, brian.carpenter@​gmail.com wrote​:

Triggered while fuzzing v5.27.4-28-g60dfa51.

./perl -e '$p00="[\0\\N{U+.}";qr/$p00/'

perl​: sv.c​:12530​: void Perl_sv_vcatpvfn_flags(SV *const, const char
*const, const STRLEN, va_list *const, SV **const, const size_t,
_Bool
*const, const U32)​: Assertion `0' failed.
Aborted

Backtrace​:

...
#4 0x000000000061d537 in Perl_sv_vcatpvfn_flags (sv=0xacc3c8,
pat=0x7fffffffc050 "Missing right brace on \\%c{} in regex;
marked by <-- HERE in m/%d%lu%4p <-- HERE %d%lu%4p/\n", patlen=91,
args=0x7fffffffc038,
svargs=0x0, sv_count=0, maybe_tainted=0x0, flags=0) at
sv.c​:12530
#5 0x00000000006190e0 in Perl_sv_vsetpvfn (sv=0xacc3c8,
pat=0x7fffffffc050 "Missing right brace on \\%c{} in regex;
marked by <-- HERE in m/%d%lu%4p <-- HERE %d%lu%4p/\n", patlen=91,
args=0x7fffffffc038,
svargs=0x0, sv_count=0, maybe_tainted=0x0) at sv.c​:11007
#6 0x000000000056647c in Perl_vmess (
pat=0x7fffffffc050 "Missing right brace on \\%c{} in regex;
marked by <-- HERE in m/%d%lu%4p <-- HERE %d%lu%4p/\n",
args=0x7fffffffc038) at util.c​:1487
#7 0x000000000054afa1 in S_re_croak2 (utf8=false,
pat1=0x7e0488 "Missing right brace on \\%c{}",
pat2=0x7ded50 " in regex; marked by <-- HERE in m/%d%lu%4p <--
HERE %d%lu%4p/") at regcomp.c​:20050
#8 0x0000000000527f05 in S_grok_bslash_N
(pRExC_state=0x7fffffffe0a0,
node_p=0x7fffffffc4e8, code_point_p=0x0, cp_count=0x0,
flagp=0x7fffffffc7d0, strict=false, depth=9) at regcomp.c​:12059
#9 0x000000000052ccd1 in S_regatom (pRExC_state=0x7fffffffe0a0,
flagp=0x7fffffffc7d0, depth=9) at regcomp.c​:12971
#10 0x0000000000525238 in S_regpiece (pRExC_state=0x7fffffffe0a0,
flagp=0x7fffffffc8fc, depth=8) at regcomp.c​:11708
...

which looks embarassing similar to 131598.

But it isn't the same cause. I haven't had lately and won't have
time
to do much on this project in the next few weeks. My guess on this is
that it is some flaw in REPORT_LOCATION_ARGS()

[p5pRT]

From @atoomic

0001-Fixup-REPORT_LOCATION_ARGS-to-use-positive-length.patch

From 770dec97adda5c2b37e1f647747a9d1c779cc289 Mon Sep 17 00:00:00 2001
From: Nicolas R <atoomic@cpan.org>
Date: Tue, 26 Sep 2017 12:26:15 -0500
Subject: [PATCH] Fixup REPORT_LOCATION_ARGS to use positive length

Use vFAIL instead of vFAIL2, and adjust the length
when the offset is negative.

RT-132163
---
 regcomp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/regcomp.c b/regcomp.c
index cc0ff96064..38b0fa9e59 100644
--- a/regcomp.c
+++ b/regcomp.c
@@ -628,7 +628,7 @@ static const scan_data_t zero_scan_data = {
     UTF8fARG(UTF,                                                           \
              (xI(xC) > eC) /* Don't run off end */                          \
               ? eC - sC   /* Length before the <--HERE */                   \
-              : xI_offset(xC),                                              \
+              : ( xI_offset(xC) > 0 ? xI_offset(xC) : 0 ),                  \
              sC),         /* The input pattern printed up to the <--HERE */ \
     UTF8fARG(UTF,                                                           \
              (xI(xC) > eC) ? 0 : eC - xI(xC), /* Length after <--HERE */    \
@@ -12056,7 +12056,7 @@ S_grok_bslash_N(pTHX_ RExC_state_t *pRExC_state,
 
     endbrace = strchr(RExC_parse, '}');
     if (! endbrace) { /* no trailing brace */
-        vFAIL2("Missing right brace on \\%c{}", 'N');
+        vFAIL("Missing right brace on xyz \\N{}");
     }
     else if(!(endbrace == RExC_parse		/* nothing between the {} */
               || (endbrace - RExC_parse >= 2	/* U+ (bad hex is checked... */
-- 
2.14.2

[p5pRT]

From @atoomic

Oops, my bad, my previous patch was not clean, which explains why a few tests were failing, this is much better once updated like this.

Only porting/diag.t is failing one test

View updated patch version attached to this message.

On Tue, 26 Sep 2017 10​:31​:43 -0700, atoomic wrote​:

I agree with Tony Cook this is a problem in REPORT_LOCATION_ARGS,
when calling vFAIL2 from S_grok_bslash_N

1/ minor, we should not use vFAIL2 as the second arg is a 'N', the
vFAIL is probably more appropriate there.

2/ I confirm that using FAIL or FAIL2 instead of the vFAIL family
fixes the issue

I think the problem is to use a negative offset for the length in
REPORT_LOCATION_ARGS (view attached patch)

note, as it a few tests are failing from re/regexp.t & co, can adjust
them, but want to confirm that the suggested patch is correct first

On Mon, 25 Sep 2017 21​:48​:24 -0700, public@​khwilliamson.com wrote​:

On 09/25/2017 06​:44 PM, Tony Cook via RT wrote​:

On Mon, 25 Sep 2017 16​:21​:55 -0700, brian.carpenter@​gmail.com
wrote​:

Triggered while fuzzing v5.27.4-28-g60dfa51.

./perl -e '$p00="[\0\\N{U+.}";qr/$p00/'

perl​: sv.c​:12530​: void Perl_sv_vcatpvfn_flags(SV *const, const
char
*const, const STRLEN, va_list *const, SV **const, const size_t,
_Bool
*const, const U32)​: Assertion `0' failed.
Aborted

Backtrace​:

...
#4 0x000000000061d537 in Perl_sv_vcatpvfn_flags (sv=0xacc3c8,
pat=0x7fffffffc050 "Missing right brace on \\%c{} in regex;
marked by <-- HERE in m/%d%lu%4p <-- HERE %d%lu%4p/\n", patlen=91,
args=0x7fffffffc038,
svargs=0x0, sv_count=0, maybe_tainted=0x0, flags=0) at
sv.c​:12530
#5 0x00000000006190e0 in Perl_sv_vsetpvfn (sv=0xacc3c8,
pat=0x7fffffffc050 "Missing right brace on \\%c{} in regex;
marked by <-- HERE in m/%d%lu%4p <-- HERE %d%lu%4p/\n", patlen=91,
args=0x7fffffffc038,
svargs=0x0, sv_count=0, maybe_tainted=0x0) at sv.c​:11007
#6 0x000000000056647c in Perl_vmess (
pat=0x7fffffffc050 "Missing right brace on \\%c{} in regex;
marked by <-- HERE in m/%d%lu%4p <-- HERE %d%lu%4p/\n",
args=0x7fffffffc038) at util.c​:1487
#7 0x000000000054afa1 in S_re_croak2 (utf8=false,
pat1=0x7e0488 "Missing right brace on \\%c{}",
pat2=0x7ded50 " in regex; marked by <-- HERE in m/%d%lu%4p <--
HERE %d%lu%4p/") at regcomp.c​:20050
#8 0x0000000000527f05 in S_grok_bslash_N
(pRExC_state=0x7fffffffe0a0,
node_p=0x7fffffffc4e8, code_point_p=0x0, cp_count=0x0,
flagp=0x7fffffffc7d0, strict=false, depth=9) at
regcomp.c​:12059
#9 0x000000000052ccd1 in S_regatom (pRExC_state=0x7fffffffe0a0,
flagp=0x7fffffffc7d0, depth=9) at regcomp.c​:12971
#10 0x0000000000525238 in S_regpiece (pRExC_state=0x7fffffffe0a0,
flagp=0x7fffffffc8fc, depth=8) at regcomp.c​:11708
...

which looks embarassing similar to 131598.

But it isn't the same cause. I haven't had lately and won't have
time
to do much on this project in the next few weeks. My guess on this
is
that it is some flaw in REPORT_LOCATION_ARGS()

[p5pRT]

From @atoomic

0001-Fixup-REPORT_LOCATION_ARGS-to-use-positive-length.patch

From a43682a478801430f81e9982cd9b7312410fc421 Mon Sep 17 00:00:00 2001
From: Nicolas R <atoomic@cpan.org>
Date: Tue, 26 Sep 2017 12:26:15 -0500
Subject: [PATCH] Fixup REPORT_LOCATION_ARGS to use positive length

Use vFAIL instead of vFAIL2, and adjust the length
when the offset is negative.

RT-132163
---
 regcomp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/regcomp.c b/regcomp.c
index cc0ff96064..b8233ac699 100644
--- a/regcomp.c
+++ b/regcomp.c
@@ -628,7 +628,7 @@ static const scan_data_t zero_scan_data = {
     UTF8fARG(UTF,                                                           \
              (xI(xC) > eC) /* Don't run off end */                          \
               ? eC - sC   /* Length before the <--HERE */                   \
-              : xI_offset(xC),                                              \
+              : ( xI_offset(xC) > 0 ? xI_offset(xC) : 0 ),                  \
              sC),         /* The input pattern printed up to the <--HERE */ \
     UTF8fARG(UTF,                                                           \
              (xI(xC) > eC) ? 0 : eC - xI(xC), /* Length after <--HERE */    \
@@ -12056,7 +12056,7 @@ S_grok_bslash_N(pTHX_ RExC_state_t *pRExC_state,
 
     endbrace = strchr(RExC_parse, '}');
     if (! endbrace) { /* no trailing brace */
-        vFAIL2("Missing right brace on \\%c{}", 'N');
+        vFAIL("Missing right brace on \\N{}");
     }
     else if(!(endbrace == RExC_parse		/* nothing between the {} */
               || (endbrace - RExC_parse >= 2	/* U+ (bad hex is checked... */
-- 
2.14.2

[p5pRT]

From @atoomic

sorry for the spam and extra message, took me some time to understood how that porting/diag.t test was designed
this is now fixed, and the test suite is clean for me, with this patch.

nicolas

On Mon, 25 Sep 2017 17​:44​:06 -0700, tonyc wrote​:

On Mon, 25 Sep 2017 16​:21​:55 -0700, brian.carpenter@​gmail.com wrote​:

Triggered while fuzzing v5.27.4-28-g60dfa51.

./perl -e '$p00="[\0\\N{U+.}";qr/$p00/'

perl​: sv.c​:12530​: void Perl_sv_vcatpvfn_flags(SV *const, const char
*const, const STRLEN, va_list *const, SV **const, const size_t, _Bool
*const, const U32)​: Assertion `0' failed.
Aborted

Backtrace​:

...
#4 0x000000000061d537 in Perl_sv_vcatpvfn_flags (sv=0xacc3c8,
pat=0x7fffffffc050 "Missing right brace on \\%c{} in regex;
marked by <-- HERE in m/%d%lu%4p <-- HERE %d%lu%4p/\n", patlen=91,
args=0x7fffffffc038,
svargs=0x0, sv_count=0, maybe_tainted=0x0, flags=0) at sv.c​:12530
#5 0x00000000006190e0 in Perl_sv_vsetpvfn (sv=0xacc3c8,
pat=0x7fffffffc050 "Missing right brace on \\%c{} in regex;
marked by <-- HERE in m/%d%lu%4p <-- HERE %d%lu%4p/\n", patlen=91,
args=0x7fffffffc038,
svargs=0x0, sv_count=0, maybe_tainted=0x0) at sv.c​:11007
#6 0x000000000056647c in Perl_vmess (
pat=0x7fffffffc050 "Missing right brace on \\%c{} in regex; marked
by <-- HERE in m/%d%lu%4p <-- HERE %d%lu%4p/\n", args=0x7fffffffc038)
at util.c​:1487
#7 0x000000000054afa1 in S_re_croak2 (utf8=false,
pat1=0x7e0488 "Missing right brace on \\%c{}",
pat2=0x7ded50 " in regex; marked by <-- HERE in m/%d%lu%4p <--
HERE %d%lu%4p/") at regcomp.c​:20050
#8 0x0000000000527f05 in S_grok_bslash_N
(pRExC_state=0x7fffffffe0a0,
node_p=0x7fffffffc4e8, code_point_p=0x0, cp_count=0x0,
flagp=0x7fffffffc7d0, strict=false, depth=9) at regcomp.c​:12059
#9 0x000000000052ccd1 in S_regatom (pRExC_state=0x7fffffffe0a0,
flagp=0x7fffffffc7d0, depth=9) at regcomp.c​:12971
#10 0x0000000000525238 in S_regpiece (pRExC_state=0x7fffffffe0a0,
flagp=0x7fffffffc8fc, depth=8) at regcomp.c​:11708
...

which looks embarassing similar to 131598.

Tony

[p5pRT]

From @atoomic

0001-Fixup-REPORT_LOCATION_ARGS-to-use-positive-length.patch

From 35b8492b237191408cd989bd19f2ea996e4b7ee3 Mon Sep 17 00:00:00 2001
From: Nicolas R <atoomic@cpan.org>
Date: Tue, 26 Sep 2017 12:26:15 -0500
Subject: [PATCH] Fixup REPORT_LOCATION_ARGS to use positive length

Use vFAIL instead of vFAIL2, and adjust the length
when the offset is negative.

RT-132163
---
 pod/perldiag.pod | 2 ++
 regcomp.c        | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/pod/perldiag.pod b/pod/perldiag.pod
index d417fb296e..dd03913fb7 100644
--- a/pod/perldiag.pod
+++ b/pod/perldiag.pod
@@ -3658,6 +3658,8 @@ L<perlfunc/require EXPR> and L<perlfunc/do EXPR>.
 
 =item Missing right brace on \%c{} in regex; marked by S<<-- HERE> in m/%s/
 
+=item Missing right brace on \N{} in regex; marked by S<<-- HERE> in m/%s/
+
 (F) Missing right brace in C<\x{...}>, C<\p{...}>, C<\P{...}>, or C<\N{...}>.
 
 =item Missing right brace on \N{}
diff --git a/regcomp.c b/regcomp.c
index cc0ff96064..b8233ac699 100644
--- a/regcomp.c
+++ b/regcomp.c
@@ -628,7 +628,7 @@ static const scan_data_t zero_scan_data = {
     UTF8fARG(UTF,                                                           \
              (xI(xC) > eC) /* Don't run off end */                          \
               ? eC - sC   /* Length before the <--HERE */                   \
-              : xI_offset(xC),                                              \
+              : ( xI_offset(xC) > 0 ? xI_offset(xC) : 0 ),                  \
              sC),         /* The input pattern printed up to the <--HERE */ \
     UTF8fARG(UTF,                                                           \
              (xI(xC) > eC) ? 0 : eC - xI(xC), /* Length after <--HERE */    \
@@ -12056,7 +12056,7 @@ S_grok_bslash_N(pTHX_ RExC_state_t *pRExC_state,
 
     endbrace = strchr(RExC_parse, '}');
     if (! endbrace) { /* no trailing brace */
-        vFAIL2("Missing right brace on \\%c{}", 'N');
+        vFAIL("Missing right brace on \\N{}");
     }
     else if(!(endbrace == RExC_parse		/* nothing between the {} */
               || (endbrace - RExC_parse >= 2	/* U+ (bad hex is checked... */
-- 
2.14.2

[p5pRT]

From @atoomic

Add one extra unit test for this case to t/re/re_tests

On Tue, 26 Sep 2017 10​:56​:13 -0700, atoomic wrote​:

sorry for the spam and extra message, took me some time to understood
how that porting/diag.t test was designed
this is now fixed, and the test suite is clean for me, with this
patch.

nicolas

On Mon, 25 Sep 2017 17​:44​:06 -0700, tonyc wrote​:

On Mon, 25 Sep 2017 16​:21​:55 -0700, brian.carpenter@​gmail.com wrote​:

Triggered while fuzzing v5.27.4-28-g60dfa51.

./perl -e '$p00="[\0\\N{U+.}";qr/$p00/'

perl​: sv.c​:12530​: void Perl_sv_vcatpvfn_flags(SV *const, const char
*const, const STRLEN, va_list *const, SV **const, const size_t,
_Bool
*const, const U32)​: Assertion `0' failed.
Aborted

Backtrace​:

...
#4 0x000000000061d537 in Perl_sv_vcatpvfn_flags (sv=0xacc3c8,
pat=0x7fffffffc050 "Missing right brace on \\%c{} in regex;
marked by <-- HERE in m/%d%lu%4p <-- HERE %d%lu%4p/\n", patlen=91,
args=0x7fffffffc038,
svargs=0x0, sv_count=0, maybe_tainted=0x0, flags=0) at sv.c​:12530
#5 0x00000000006190e0 in Perl_sv_vsetpvfn (sv=0xacc3c8,
pat=0x7fffffffc050 "Missing right brace on \\%c{} in regex;
marked by <-- HERE in m/%d%lu%4p <-- HERE %d%lu%4p/\n", patlen=91,
args=0x7fffffffc038,
svargs=0x0, sv_count=0, maybe_tainted=0x0) at sv.c​:11007
#6 0x000000000056647c in Perl_vmess (
pat=0x7fffffffc050 "Missing right brace on \\%c{} in regex;
marked
by <-- HERE in m/%d%lu%4p <-- HERE %d%lu%4p/\n", args=0x7fffffffc038)
at util.c​:1487
#7 0x000000000054afa1 in S_re_croak2 (utf8=false,
pat1=0x7e0488 "Missing right brace on \\%c{}",
pat2=0x7ded50 " in regex; marked by <-- HERE in m/%d%lu%4p <--
HERE %d%lu%4p/") at regcomp.c​:20050
#8 0x0000000000527f05 in S_grok_bslash_N
(pRExC_state=0x7fffffffe0a0,
node_p=0x7fffffffc4e8, code_point_p=0x0, cp_count=0x0,
flagp=0x7fffffffc7d0, strict=false, depth=9) at regcomp.c​:12059
#9 0x000000000052ccd1 in S_regatom (pRExC_state=0x7fffffffe0a0,
flagp=0x7fffffffc7d0, depth=9) at regcomp.c​:12971
#10 0x0000000000525238 in S_regpiece (pRExC_state=0x7fffffffe0a0,
flagp=0x7fffffffc8fc, depth=8) at regcomp.c​:11708
...

which looks embarassing similar to 131598.

Tony

[p5pRT]

From @atoomic

0001-Add-unit-test-for-RT-132163.patch

From ed25d8f69a6bb1e1bbcac4b452042ddbca7fdef4 Mon Sep 17 00:00:00 2001
From: Nicolas R <atoomic@cpan.org>
Date: Tue, 26 Sep 2017 13:27:57 -0500
Subject: [PATCH] Add unit test for RT #132163

---
 t/re/re_tests | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/re/re_tests b/t/re/re_tests
index 0bd9b5541f..7adc000658 100644
--- a/t/re/re_tests
+++ b/t/re/re_tests
@@ -1941,6 +1941,7 @@ A+(*PRUNE)BC(?{})	AAABC	y	$&	AAABC
 /w\z\R\z/i	\x{100}a\x{80}a	n	-	-
 
 /(a+){1}+a/	aaa	n	-	-		# [perl #125825]
+[\0\N{U+.}	aaa	c	-	Unmatched [ in regex; marked by <-- HERE in m/\[ <-- HERE \\0\\N{U+.}/	# [perl #132163]
 
 ^((?(?=x)xb|ya)z)	xbz	y	$1	xbz
 ^((?(?=x)xb|ya)z)	yaz	y	$1	yaz
-- 
2.14.2

[p5pRT]

From @khwilliamson

On Tue, 26 Sep 2017 10​:31​:43 -0700, atoomic wrote​:

I agree with Tony Cook this is a problem in REPORT_LOCATION_ARGS,
when calling vFAIL2 from S_grok_bslash_N

1/ minor, we should not use vFAIL2 as the second arg is a 'N', the
vFAIL is probably more appropriate there.

2/ I confirm that using FAIL or FAIL2 instead of the vFAIL family
fixes the issue

I think the problem is to use a negative offset for the length in
REPORT_LOCATION_ARGS (view attached patch)

note, as it a few tests are failing from re/regexp.t & co, can adjust
them, but want to confirm that the suggested patch is correct first

I finally had a good look at this problem. It turns out that the code patch is not the correct fix. It causes valgrind errors when I run it, and there is instead a deeper problem. The patch substitutes 0 when a number is negative. But it turns out that if the number is negative, it means something is terribly wrong. I will submit a patch that asserts against that.

I thought I understood why this number is getting negative, but now I realize that I still don't understand that. It turns out that I have been overhauling this area of the code for other reasons, and the most productive path forward is to start with the overhaul that I've done so far.

I'll look at the .t patch after that is done.

The reason for the vFAIL2 instead of the more obvious vFAIL is so that the existing entry in perldiag would work. Extra entries slow down the process for someone trying to figure out what's going on, and when I wrote the code I thought the tradeoff was worthwhile, to improve the user experience. Since we are about to croak, there's no performance impetus. The other option would have been to add a 'diag listed as' comment there. I try to avoid those. But I could have added an explanation why the non-obvious thing was done.
--
Karl Williamson

[p5pRT]

From @atoomic

using v5.27.6 (v5.27.5-349-gb9a5a78fe9), I cannot reproduce this SEGV.
The fix is probably coming from Karl's recent changes

v5.27.5-349-gb9a5a78fe9> ./perl -e '$p00="[\0\\N{U+.}";qr/$p00/'
Unmatched [ in regex; marked by <-- HERE in m/[ <-- HERE \N{U+.}/ at -e line 1.

At this point, I think we should close this ticket.

On Sat, 21 Oct 2017 16​:23​:26 -0700, khw wrote​:

On Tue, 26 Sep 2017 10​:31​:43 -0700, atoomic wrote​:

I agree with Tony Cook this is a problem in REPORT_LOCATION_ARGS,
when calling vFAIL2 from S_grok_bslash_N

1/ minor, we should not use vFAIL2 as the second arg is a 'N', the
vFAIL is probably more appropriate there.

2/ I confirm that using FAIL or FAIL2 instead of the vFAIL family
fixes the issue

I think the problem is to use a negative offset for the length in
REPORT_LOCATION_ARGS (view attached patch)

note, as it a few tests are failing from re/regexp.t & co, can adjust
them, but want to confirm that the suggested patch is correct first

I finally had a good look at this problem. It turns out that the code
patch is not the correct fix. It causes valgrind errors when I run
it, and there is instead a deeper problem. The patch substitutes 0
when a number is negative. But it turns out that if the number is
negative, it means something is terribly wrong. I will submit a patch
that asserts against that.

I thought I understood why this number is getting negative, but now I
realize that I still don't understand that. It turns out that I have
been overhauling this area of the code for other reasons, and the most
productive path forward is to start with the overhaul that I've done
so far.

I'll look at the .t patch after that is done.

The reason for the vFAIL2 instead of the more obvious vFAIL is so that
the existing entry in perldiag would work. Extra entries slow down
the process for someone trying to figure out what's going on, and when
I wrote the code I thought the tradeoff was worthwhile, to improve the
user experience. Since we are about to croak, there's no performance
impetus. The other option would have been to add a 'diag listed as'
comment there. I try to avoid those. But I could have added an
explanation why the non-obvious thing was done.

[p5pRT]

From @khwilliamson

On Mon, 13 Nov 2017 14​:01​:02 -0800, atoomic@​cpan.org wrote​:

using v5.27.6 (v5.27.5-349-gb9a5a78fe9), I cannot reproduce this SEGV.
The fix is probably coming from Karl's recent changes

v5.27.5-349-gb9a5a78fe9> ./perl -e '$p00="[\0\\N{U+.}";qr/$p00/'
Unmatched [ in regex; marked by <-- HERE in m/[ <-- HERE \N{U+.}/ at
-e line 1.

At this point, I think we should close this ticket.

The problem with closing the ticket is that just adding the missing ']' causes it to be buggy in a different way.

Compiling REx "[%0\N{U+.}]"
Illegal hexadecimal digit '|' ignored at 132163.pl line 2.
Illegal hexadecimal digit '\' ignored at 132163.pl line 2.
rarest char at 0
Final program​:
  1​: EXACT <\0\0\0]> (5)
  5​: END (0)
anchored "%0%0%0]" at 0..0 (checking anchored isall) minlen 4
Freeing REx​: "[%0\N{U+.}]"

There is no '|' in this pattern; and it should fail instead of compiling into the wrong stuff.

--
Karl Williamson