regcomp.c: Change error reporting mechanism slightly
authorKarl Williamson <khw@cpan.org>
Fri, 2 Mar 2018 11:47:45 +0000 (04:47 -0700)
committerKarl Williamson <khw@cpan.org>
Wed, 10 Oct 2018 16:42:40 +0000 (10:42 -0600)
There are (rare) constructs which cause regcomp.c to modify the user
input stream (stashing the original), and that is parsed instead before
returning to the continue with the original.  A problem arises if an
error occurs during the parsing of this modified version.  We want to
report the location of the error and context based on the original.
This led to 285b5ca0145796a915dec03e87e0176fd4681041 (fixing #126261).

This new commit simplifies the mechanism so that it is easier to
understand.

regcomp.c

index 1e8266b..3c09bc7 100644 (file)
--- a/regcomp.c
+++ b/regcomp.c
@@ -133,8 +133,10 @@ struct RExC_state_t {
     char       *start;                 /* Start of input for compile */
     char       *end;                   /* End of input for compile */
     char       *parse;                 /* Input-scan pointer. */
-    char        *adjusted_start;        /* 'start', adjusted.  See code use */
-    STRLEN      precomp_adj;            /* an offset beyond precomp.  See code use */
+    char        *copy_start;            /* start of copy of input within
+                                           constructed parse string */
+    char        *copy_start_in_input;   /* Position in input string
+                                           corresponding to copy_start */
     SSize_t    whilem_seen;            /* number of WHILEM in this expr */
     regnode    *emit_start;            /* Start of emitted-code area */
     regnode    *emit_bound;            /* First regnode outside of the
@@ -218,8 +220,8 @@ struct RExC_state_t {
 #define RExC_flags     (pRExC_state->flags)
 #define RExC_pm_flags  (pRExC_state->pm_flags)
 #define RExC_precomp   (pRExC_state->precomp)
-#define RExC_precomp_adj (pRExC_state->precomp_adj)
-#define RExC_adjusted_start  (pRExC_state->adjusted_start)
+#define RExC_copy_start_in_input (pRExC_state->copy_start_in_input)
+#define RExC_copy_start_in_constructed  (pRExC_state->copy_start)
 #define RExC_precomp_end (pRExC_state->precomp_end)
 #define RExC_rx_sv     (pRExC_state->rx_sv)
 #define RExC_rx                (pRExC_state->rx)
@@ -606,11 +608,11 @@ static const scan_data_t zero_scan_data = {
  * the form of something that is completely different from the input, or
  * something that uses the input as part of the alternate.  In the first case,
  * there should be no possibility of an error, as we are in complete control of
- * the alternate string.  But in the second case we don't control the input
- * portion, so there may be errors in that.  Here's an example:
+ * the alternate string.  But in the second case we don't completely control
+ * the input portion, so there may be errors in that.  Here's an example:
  *      /[abc\x{DF}def]/ui
  * is handled specially because \x{df} folds to a sequence of more than one
- * character, 'ss'.  What is done is to create and parse an alternate string,
+ * character: 'ss'.  What is done is to create and parse an alternate string,
  * which looks like this:
  *      /(?:\x{DF}|[abc\x{DF}def])/ui
  * where it uses the input unchanged in the middle of something it constructs,
@@ -619,55 +621,65 @@ static const scan_data_t zero_scan_data = {
  * class while in this substitute parse.) 'abc' and 'def' may have errors that
  * need to be reported.  The general situation looks like this:
  *
+ *                                       |<------- identical ------>|
  *              sI                       tI               xI       eI
- * Input:       ----------------------------------------------------
+ * Input:       ---------------------------------------------------------------
  * Constructed:         ---------------------------------------------------
  *                      sC               tC               xC       eC     EC
+ *                                       |<------- identical ------>|
  *
- * The input string sI..eI is the input pattern.  The string sC..EC is the
- * constructed substitute parse string.  The portions sC..tC and eC..EC are
- * constructed by us.  The portion tC..eC is an exact duplicate of the input
- * pattern tI..eI.  In the diagram, these are vertically aligned.  Suppose that
- * while parsing, we find an error at xC.  We want to display a message showing
- * the real input string.  Thus we need to find the point xI in it which
- * corresponds to xC.  xC >= tC, since the portion of the string sC..tC has
- * been constructed by us, and so shouldn't have errors.  We get:
+ * sI..eI   is the portion of the input pattern we are concerned with here.
+ * sC..EC   is the constructed substitute parse string.
+ *  sC..tC  is constructed by us
+ *  tC..eC  is an exact duplicate of the portion of the input pattern tI..eI.
+ *          In the diagram, these are vertically aligned.
+ *  eC..EC  is also constructed by us.
+ * xC       is the position in the substitute parse string where we found a
+ *          problem.
+ * xI       is the position in the original pattern corresponding to xC.
  *
- *      xI = sI + (tI - sI) + (xC - tC)
+ * We want to display a message showing the real input string.  Thus we need to
+ * translate from xC to xI.  We know that xC >= tC, since the portion of the
+ * string sC..tC has been constructed by us, and so shouldn't have errors.  We
+ * get:
+ *      xI = tI + (xC - tC)
  *
- * and, the offset into sI is:
+ * When the substitute parse is constructed, the code needs to set:
+ *      RExC_start (sC)
+ *      RExC_end (eC)
+ *      RExC_copy_start_in_input  (tI)
+ *      RExC_copy_start_in_constructed (tC)
+ * and restore them when done.
  *
- *      (xI - sI) = (tI - sI) + (xC - tC)
- *
- * When the substitute is constructed, we save (tI -sI) as RExC_precomp_adj,
- * and we save tC as RExC_adjusted_start.
- *
- * During normal processing of the input pattern, everything points to that,
- * with RExC_precomp_adj set to 0, and RExC_adjusted_start set to sI.
+ * During normal processing of the input pattern, both
+ * 'RExC_copy_start_in_input' and 'RExC_copy_start_in_constructed' are set to
+ * sI, so that xC equals xI.
  */
 
-#define tI_sI           RExC_precomp_adj
-#define tC              RExC_adjusted_start
-#define sC              RExC_precomp
-#define xI_offset(xC)   ((IV) (tI_sI + (xC - tC)))
-#define xI(xC)          (sC + xI_offset(xC))
-#define eC              RExC_precomp_end
+#define sI              RExC_precomp
+#define eI              RExC_precomp_end
+#define sC              RExC_start
+#define eC              RExC_end
+#define tI              RExC_copy_start_in_input
+#define tC              RExC_copy_start_in_constructed
+#define xI(xC)          (tI + (xC - tC))
+#define xI_offset(xC)   (xI(xC) - sI)
 
 #define REPORT_LOCATION_ARGS(xC)                                            \
     UTF8fARG(UTF,                                                           \
-             (xI(xC) > eC) /* Don't run off end */                          \
+             (xI(xC) > eI) /* Don't run off end */                          \
               ? eC - sC   /* Length before the <--HERE */                   \
               : ((xI_offset(xC) >= 0)                                       \
                  ? xI_offset(xC)                                            \
                  : (Perl_croak(aTHX_ "panic: %s: %d: negative offset: %"    \
                                     IVdf " trying to output message for "   \
                                     " pattern %.*s",                        \
-                                    __FILE__, __LINE__, xI_offset(xC),      \
+                                    __FILE__, __LINE__, (IV) xI_offset(xC), \
                                     ((int) (eC - sC)), sC), 0)),            \
-             sC),         /* The input pattern printed up to the <--HERE */ \
+             sI),         /* The input pattern printed up to the <--HERE */ \
     UTF8fARG(UTF,                                                           \
-             (xI(xC) > eC) ? 0 : eC - xI(xC), /* Length after <--HERE */    \
-             (xI(xC) > eC) ? eC : xI(xC))     /* pattern after <--HERE */
+             (xI(xC) > eI) ? 0 : eI - xI(xC), /* Length after <--HERE */    \
+             (xI(xC) > eI) ? eI : xI(xC))     /* pattern after <--HERE */
 
 /* Used to point after bad bytes for an error message, but avoid skipping
  * past a nul byte. */
@@ -7150,8 +7162,7 @@ Perl_re_op_compile(pTHX_ SV ** const patternp, int pat_count,
        set_regex_charset(&rx_flags, REGEX_UNICODE_CHARSET);
     }
 
-    RExC_precomp = exp;
-    RExC_precomp_adj = 0;
+    RExC_copy_start_in_constructed = RExC_copy_start_in_input = RExC_precomp = exp;
     RExC_flags = rx_flags;
     RExC_pm_flags = pm_flags;
 
@@ -7184,7 +7195,7 @@ Perl_re_op_compile(pTHX_ SV ** const patternp, int pat_count,
 
     /* First pass: determine size, legality. */
     RExC_parse = exp;
-    RExC_start = RExC_adjusted_start = exp;
+    RExC_start = RExC_copy_start_in_constructed = exp;
     RExC_end = exp + plen;
     RExC_precomp_end = RExC_end;
     RExC_naughty = 0;
@@ -12538,7 +12549,9 @@ S_grok_bslash_N(pTHX_ RExC_state_t *pRExC_state,
      * constructs.  This can be called from within a substitute parse already.
      * The error reporting mechanism doesn't work for 2 levels of this, but the
      * code above has validated this new construct, so there should be no
-     * errors generated by the below.*/
+     * errors generated by the below.  And this isn' an exact copy, so the
+     * mechanism to seamlessly deal with this won't work.  XXX Maybe should
+     * turn off all warnings for safety? */
     save_start = RExC_start;
     orig_end = RExC_end;
 
@@ -17626,15 +17639,16 @@ S_regclass(pTHX_ RExC_state_t *pRExC_state, I32 *flagp, U32 depth,
        char *save_end = RExC_end;
        char *save_parse = RExC_parse;
        char *save_start = RExC_start;
-        STRLEN prefix_end = 0;      /* We copy the character class after a
-                                       prefix supplied here.  This is the size
-                                       + 1 of that prefix */
+        Size_t constructed_prefix_len = 0; /* This gives the length of the
+                                              constructed portion of the
+                                              substitute parse. */
         bool first_time = TRUE;     /* First multi-char occurrence doesn't get
                                        a "|" */
         I32 reg_flags;
 
         assert(! invert);
-        assert(RExC_precomp_adj == 0); /* Only one level of recursion allowed */
+        /* Only one level of recursion allowed */
+        assert(RExC_copy_start_in_constructed == RExC_precomp);
 
 #if 0   /* Have decided not to deal with multi-char folds in inverted classes,
            because too confusing */
@@ -17672,7 +17686,7 @@ S_regclass(pTHX_ RExC_state_t *pRExC_state, I32 *flagp, U32 depth,
          * multi-character folds, have to include it in recursive parsing */
         if (element_count) {
             sv_catpvs(substitute_parse, "|[");
-            prefix_end = SvCUR(substitute_parse);
+            constructed_prefix_len = SvCUR(substitute_parse);
             sv_catpvn(substitute_parse, orig_parse, RExC_parse - orig_parse);
 
             /* Put in a closing ']' only if not going off the end, as otherwise
@@ -17695,9 +17709,9 @@ S_regclass(pTHX_ RExC_state_t *pRExC_state, I32 *flagp, U32 depth,
         /* Set up the data structure so that any errors will be properly
          * reported.  See the comments at the definition of
          * REPORT_LOCATION_ARGS for details */
-        RExC_precomp_adj = orig_parse - RExC_precomp;
-       RExC_start =  RExC_parse = SvPV(substitute_parse, len);
-        RExC_adjusted_start = RExC_start + prefix_end;
+        RExC_copy_start_in_input = (char *) orig_parse;
+       RExC_start = RExC_parse = SvPV(substitute_parse, len);
+        RExC_copy_start_in_constructed = RExC_start + constructed_prefix_len;
        RExC_end = RExC_parse + len;
         RExC_in_multi_char_class = 1;
         RExC_emit = (regnode *)orig_emit;
@@ -17708,8 +17722,7 @@ S_regclass(pTHX_ RExC_state_t *pRExC_state, I32 *flagp, U32 depth,
 
         /* And restore so can parse the rest of the pattern */
         RExC_parse = save_parse;
-       RExC_start = RExC_adjusted_start = save_start;
-        RExC_precomp_adj = 0;
+       RExC_start = RExC_copy_start_in_constructed = RExC_copy_start_in_input = save_start;
        RExC_end = save_end;
        RExC_in_multi_char_class = 0;
         SvREFCNT_dec_NN(multi_char_matches);