Skip to content

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

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

Description

@p5pRT
p5pRT
opened on Sep 25, 2017

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

Searchable as RT132163$

Activity

  1. p5pRT commented on Sep 25, 2017

    @p5pRT
    p5pRT
    on Sep 25, 2017
    Author

    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

  2. p5pRT commented on Sep 26, 2017

    @p5pRT
    p5pRT
    on Sep 26, 2017
    Author

    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

  3. p5pRT commented on Sep 26, 2017

    @p5pRT
    p5pRT
    on Sep 26, 2017
    Author

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

  4. p5pRT commented on Sep 26, 2017

    @p5pRT
    p5pRT
    on Sep 26, 2017
    Author

    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()

  5. p5pRT commented on Sep 26, 2017

    @p5pRT
    p5pRT
    on Sep 26, 2017
    Author

    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()

  6. p5pRT commented on Sep 26, 2017

    @p5pRT
    p5pRT
    on Sep 26, 2017
    Author

    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

  7. p5pRT commented on Sep 26, 2017

    @p5pRT
    p5pRT
    on Sep 26, 2017
    Author

    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()

  8. p5pRT commented on Sep 26, 2017

    @p5pRT
    p5pRT
    on Sep 26, 2017
    Author

    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

  9. p5pRT commented on Sep 26, 2017

    @p5pRT
    p5pRT
    on Sep 26, 2017
    Author

    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

  10. p5pRT commented on Sep 26, 2017

    @p5pRT
    p5pRT
    on Sep 26, 2017
    Author

    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

  11. p5pRT commented on Sep 26, 2017

    @p5pRT
    p5pRT
    on Sep 26, 2017
    Author

    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

  12. p5pRT commented on Sep 26, 2017

    @p5pRT
    p5pRT
    on Sep 26, 2017
    Author

    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

  13. p5pRT commented on Oct 21, 2017

    @p5pRT
    p5pRT
    on Oct 21, 2017
    Author

    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

  14. p5pRT commented on Nov 13, 2017

    @p5pRT
    p5pRT
    on Nov 13, 2017
    Author

    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.

  15. p5pRT commented on Mar 5, 2018

    @p5pRT
    p5pRT
    on Mar 5, 2018
    Author

    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

  16. p5pRT commented on Mar 6, 2018

    @p5pRT
    p5pRT
    on Mar 6, 2018
    Author

    From @khwilliamson

    Thanks for finding this; fixed by
    fb7e725
    --
    Karl Williamson

  17. p5pRT commented on Mar 6, 2018

    @p5pRT
    p5pRT
    on Mar 6, 2018
    Author

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

  18. p5pRT commented on Jun 23, 2018

    @p5pRT
    p5pRT
    on Jun 23, 2018
    Author

    From @khwilliamson

    Thank 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
    resolved.

    Perl 5.28.0 may be downloaded via​:
    https://metacpan.org/release/XSAWYERX/perl-5.28.0

    If you find that the problem persists, feel free to reopen this ticket.

  19. p5pRT commented on Jun 23, 2018

    @p5pRT
    p5pRT
    on Jun 23, 2018
    Author

    @khwilliamson - Status changed from 'pending release' to 'resolved'

  20. p5pRT
    closed this as completedon Jun 23, 2018
  21. p5pRT
    added
    Severity Low
    on Oct 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions