regcomp_trie: make octet trie lookups always valid

Fix intermittent segfaults from the trie logic, as reported in #24816,
and discussed in #24814.

The list compiler previously recorded a slot after a sparse frame in
lasttrans without always allocating or clearing it.  A lookup could
therefore treat uninitialized transition data as belonging to its state
and use an arbitrary next state.

This issue affected all platforms but was most prominently visible on
Windows and Cygwin.  (Historically Windows realloc is not as good as
*nix realloc.)

As part of fixing this, I reworked some of the logic to be more robust
and ensure we never see this kind of issue again.

We give the root state the complete octet frame at slots 1 through 256.
Other states can reuse unoccupied root slots.  Each state base maps octet
zero to an offset not less than 1, and we ensure there are enough valid
null transitions at the end, so we never read a bogus transition and do
not need to do bounds checks either.

While working on this I noticed we had some logic left over from the old
implementation that still used the concept of "charid", which confused
matters.  This was removed, and we now have simpler runtime logic to do
transition lookups.

I also added code to validate the state and transition indexes under
DEBUG_r.  Move that block outside DEBUG_r when validation is wanted in
every full DEBUGGING build.  Thus, compilation under C<use re 'debug'>
also exercises the transition validation, so if you suspect a related
bug try the pattern under that pragma.

Added support to regexp.t to allow the developer to select an inclusive
range of re_tests input lines while retaining its original TAP numbering.
This makes the triggering cases practical to stress independently.

The re_tests lines 2221 through 2222 also pass with 100,000 iterations,
including under ASAN with a refcounted stack (PERL_RC_STACK).

Author: demerphq

regcomp.h

@@ -1265,7 +1265,7 @@ enum trie_flags {
 
 struct reg_trie_data_ {
     U32             refcount;        /* number of times this trie is referenced */
-    U32             lasttrans;       /* last valid transition element */
+    U32             lasttrans;       /* one past the transition allocation */
     reg_trie_state  *states;         /* state data */
     reg_trie_trans  *trans;          /* array of transition elements */
     TRIE_JUMP_TYPE  *jump;           /* optional 1 indexed array of offsets before tail

regcomp_trie.c

@@ -538,18 +538,17 @@ S_trie_list_transition(pTHX_ reg_trie_data *trie, U32 *state, const U32 octet,
 
 PERL_STATIC_INLINE U32
 S_trie_trans_state(const reg_trie_data *trie, const U32 state,
-                   const U32 base, const U32 ucharcount, const U32 octet,
-                   const U32 special, const U32 ubound)
+                   const U32 base, const U32 octet, const U32 special)
 {
-    const U32 index = base - ucharcount + octet;
-
     /* The packed table stores a transition at base - alphabet_size + octet.
      * The check field confirms that the physical slot belongs to this state.
      * During Aho-Corasick construction, special is the fallback transition
      * used when the lookup is performed from the root state. */
-    return base + octet >= ucharcount
-        && base + octet < ubound
-        && state == trie->trans[index].check
+    if (!base)
+        return state == 1 ? special : 0;
+
+    const U32 index = base - TRIE_ALPHABET_SIZE + octet;
+    return state == trie->trans[index].check
         && trie->trans[index].next
         ? trie->trans[index].next
         : state == 1 ? special : 0;
@@ -638,17 +637,20 @@ Perl_make_trie(pTHX_ RExC_state_t *pRExC_state, regnode *startbranch,
     trie->wordinfo = (reg_trie_wordinfo *) PerlMemShared_calloc(
                        trie->wordcount+1, sizeof(reg_trie_wordinfo));
 
+#ifdef DEBUGGING
+    /* Allocate the optional word list only while regex debugging is enabled. */
     DEBUG_r({
         trie_words = newAV();
     });
 
     DEBUG_TRIE_COMPILE_r({
         re_indentf(
           "make_trie start == %d, first == %d, last == %d, tail == %d depth = %d\n",
-          depth+1,
+        depth+1,
           REG_NODE_NUM(startbranch), REG_NODE_NUM(first),
           REG_NODE_NUM(last), REG_NODE_NUM(tail), (int)depth);
     });
+#endif
 
    /* Find the node we are going to overwrite */
     if ( first == startbranch && OP( last ) != BRANCH ) {
@@ -997,21 +999,43 @@ Perl_make_trie(pTHX_ RExC_state_t *pRExC_state, regnode *startbranch,
 #ifdef DEBUGGING
         const STRLEN ideal_transition_count = transition_count - 1;
 #endif
-        STRLEN transition_capacity = transition_count;
+        /* Allow roughly 20% slack.  The root state needs one complete octet
+         * frame, and sparse frames can still expand the table. */
+        STRLEN transition_capacity = transition_count + transition_count / 5;
+        if (transition_capacity < TRIE_ALPHABET_SIZE + 1)
+            transition_capacity = TRIE_ALPHABET_SIZE + 1;
         trie->trans = (reg_trie_trans *)
             PerlMemShared_calloc( transition_capacity,
                                   sizeof(reg_trie_trans) );
         {
             U32 state;
-            /* table_highwater is one past the packed portion of trie->trans;
-             * next_hole is the first unused slot within that portion.  A
-             * non-zero next field marks an occupied slot, so holes can be
-             * reused without a separate occupancy map. */
-            U32 table_highwater = 0;
-            U32 next_hole = 0;
-
+            /* Slot zero is reserved.  The root state starts at slot one, so
+             * its complete octet frame occupies slots 1 through 256.  Other
+             * states can reuse its empty slots. */
+            U32 table_highwater = TRIE_ALPHABET_SIZE + 1;
+            U32 safe_highwater = TRIE_ALPHABET_SIZE + 1;
+            U32 next_hole = 1;
+
+            if (trie->states[1].trans.list) {
+                const U32 used = TRIE_LIST_USED(1);
+                const U32 base = TRIE_ALPHABET_SIZE + 1;
+                U32 transition_index;
+
+                for (transition_index = 1;
+                     transition_index <= used;
+                     transition_index++) {
+                    const U32 tid = base - TRIE_ALPHABET_SIZE
+                                   + TRIE_LIST_ITEM(1,
+                                                    transition_index).octet;
+                    trie->trans[tid].next = TRIE_LIST_ITEM(1,
+                                                    transition_index).newstate;
+                    trie->trans[tid].check = 1;
+                }
+                Safefree(trie->states[1].trans.list);
+                trie->states[1].trans.base = base;
+            }
 
-            for( state = 1; state < next_alloc; state ++ ) {
+            for( state = 2; state < next_alloc; state ++ ) {
                 U32 base = 0;
 
                 /*
@@ -1030,15 +1054,14 @@ Perl_make_trie(pTHX_ RExC_state_t *pRExC_state, regnode *startbranch,
                     bool placed = false;
                     U32 transition_index;
 
-                    /* A state's transitions form a frame from min_octet
-                     * through max_octet.  Sparse frames may fit into holes
-                     * left by earlier frames.  Try the first available
-                     * position.  The frame may extend beyond table_highwater,
-                     * provided it
-                     * fits in the allocation and its occupied slots do not
-                     * clash. */
+                    /* next_hole is the physical slot for min_octet.  Its
+                     * octet-zero slot is next_hole - min_octet, which must
+                     * not be slot zero.  Sparse frames may fit into holes
+                     * left by earlier frames when their occupied slots do
+                     * not clash. */
                     while (next_hole < table_highwater
-                            && trie->trans[next_hole].next)
+                            && (next_hole <= min_octet
+                                || trie->trans[next_hole].next))
                         next_hole++;
                     /* Only sparse frames benefit from hole searching.  Small
                      * frames are cheap to try, while larger frames are tried
@@ -1047,9 +1070,11 @@ Perl_make_trie(pTHX_ RExC_state_t *pRExC_state, regnode *startbranch,
                             && next_hole < table_highwater
                             && next_hole + frame_width >= next_hole) {
                         const U32 candidate_end = next_hole + frame_width;
-                        if (transition_capacity < candidate_end) {
+                        const U32 candidate_base = TRIE_ALPHABET_SIZE
+                                                   + next_hole - min_octet;
+                        const U32 needed = MAX(candidate_end, candidate_base);
+                        if (transition_capacity < needed) {
                             const U32 old_capacity = transition_capacity;
-                            const U32 needed = candidate_end;
 
                             while (transition_capacity < needed)
                                 transition_capacity *= 2;
@@ -1073,7 +1098,7 @@ Perl_make_trie(pTHX_ RExC_state_t *pRExC_state, regnode *startbranch,
                                 break;
                         }
                         if (transition_index > used) {
-                            base = TRIE_ALPHABET_SIZE + next_hole - min_octet;
+                            base = candidate_base;
                             for (transition_index = 1;
                                  transition_index <= used;
                                  transition_index++) {
@@ -1087,29 +1112,36 @@ Perl_make_trie(pTHX_ RExC_state_t *pRExC_state, regnode *startbranch,
                             }
                             if (candidate_end > table_highwater)
                                 table_highwater = candidate_end;
+                            if (candidate_base > safe_highwater)
+                                safe_highwater = candidate_base;
                             while (next_hole < table_highwater
-                                    && trie->trans[next_hole].next)
+                                    && (next_hole <= min_octet
+                                        || trie->trans[next_hole].next))
                                 next_hole++;
                             placed = true;
                         }
                     }
 
                     /* Grow the physical table when the frame cannot fit at
                      * the current high-water mark. */
-                    if (!placed
-                            && transition_capacity < table_highwater + frame_width) {
-                        const U32 old_capacity = transition_capacity;
-                        const U32 needed = table_highwater + frame_width;
-
-                        while (transition_capacity < needed)
-                            transition_capacity *= 2;
-                        trie->trans = (reg_trie_trans *)
-                            PerlMemShared_realloc( trie->trans,
+                    if (!placed) {
+                        const U32 base_at_highwater = TRIE_ALPHABET_SIZE
+                                                    + table_highwater - min_octet;
+                        const U32 needed = MAX(table_highwater + frame_width,
+                                               base_at_highwater);
+                        if (transition_capacity < needed) {
+                            const U32 old_capacity = transition_capacity;
+
+                            while (transition_capacity < needed)
+                                transition_capacity *= 2;
+                            trie->trans = (reg_trie_trans *)
+                                PerlMemShared_realloc( trie->trans,
                                                      transition_capacity
                                                      * sizeof(reg_trie_trans) );
-                        Zero( trie->trans + old_capacity,
-                              transition_capacity - old_capacity,
-                              reg_trie_trans );
+                            Zero( trie->trans + old_capacity,
+                                  transition_capacity - old_capacity,
+                                  reg_trie_trans );
+                        }
                     }
                     if (!placed) {
                         base = TRIE_ALPHABET_SIZE + table_highwater - min_octet;
@@ -1119,7 +1151,8 @@ Perl_make_trie(pTHX_ RExC_state_t *pRExC_state, regnode *startbranch,
                     if ( !placed && max_octet == min_octet ) {
                         U32 set = 0;
                         for ( ; next_hole < table_highwater ; next_hole++ ) {
-                            if ( ! trie->trans[ next_hole ].next ) {
+                            if (next_hole > min_octet
+                                && !trie->trans[next_hole].next) {
                                 base = TRIE_ALPHABET_SIZE + next_hole - min_octet;
                                 trie->trans[ next_hole ].next = TRIE_LIST_ITEM( state,
                                                                    1).newstate;
@@ -1136,7 +1169,8 @@ Perl_make_trie(pTHX_ RExC_state_t *pRExC_state, regnode *startbranch,
                             next_hole = table_highwater;
                         } else {
                             while (next_hole < table_highwater
-                                    && trie->trans[next_hole].next)
+                                    && (next_hole <= min_octet
+                                        || trie->trans[next_hole].next))
                                 next_hole++;
                         }
                     } else if (!placed) {
@@ -1153,9 +1187,12 @@ Perl_make_trie(pTHX_ RExC_state_t *pRExC_state, regnode *startbranch,
                         }
                         table_highwater += frame_width;
                         while (next_hole < table_highwater
-                                && trie->trans[next_hole].next)
+                                && (next_hole <= min_octet
+                                    || trie->trans[next_hole].next))
                             next_hole++;
                     }
+                    if (base > safe_highwater)
+                        safe_highwater = base;
                     Safefree(trie->states[ state ].trans.list);
                 }
                 /*
@@ -1165,9 +1202,20 @@ Perl_make_trie(pTHX_ RExC_state_t *pRExC_state, regnode *startbranch,
                 */
                 trie->states[ state ].trans.base = base;
             }
-            /* Slot zero is reserved by the packed-table representation; keep
-             * it in the recorded range even when table_highwater is zero. */
-            trie->lasttrans = table_highwater + 1;
+            /* Every state's base is one past its highest possible lookup.
+             * Keep sufficient zeroed tail space for the complete frame. */
+            trie->lasttrans = safe_highwater;
+            if (transition_capacity < trie->lasttrans) {
+                const U32 old_capacity = transition_capacity;
+
+                trie->trans = (reg_trie_trans *)
+                    PerlMemShared_realloc(trie->trans,
+                                          trie->lasttrans
+                                            * sizeof(reg_trie_trans));
+                Zero(trie->trans + old_capacity,
+                     trie->lasttrans - old_capacity, reg_trie_trans);
+                transition_capacity = trie->lasttrans;
+            }
             assert(next_hole <= table_highwater);
             assert(table_highwater <= transition_capacity);
             DEBUG_TRIE_COMPILE_MORE_r(
@@ -1194,6 +1242,24 @@ Perl_make_trie(pTHX_ RExC_state_t *pRExC_state, regnode *startbranch,
         PerlMemShared_realloc( trie->trans, trie->lasttrans
                                * sizeof(reg_trie_trans) );
 
+#ifdef DEBUGGING
+    /* Validate the packed trie indexes while regex debugging is enabled.
+     * Move this outside DEBUG_r() to validate every full DEBUGGING build. */
+    DEBUG_r({
+    for (U32 state = 1; state < trie->statecount; state++) {
+        assert(!trie->states[state].trans.base
+               || (trie->states[state].trans.base > TRIE_ALPHABET_SIZE
+                   && trie->states[state].trans.base <= trie->lasttrans));
+    }
+    for (U32 transition = 1; transition < trie->lasttrans; transition++) {
+        assert(!trie->trans[transition].next
+               || (trie->trans[transition].check
+                       && trie->trans[transition].check < trie->statecount
+                   && trie->trans[transition].next < trie->statecount));
+    }
+    });
+#endif
+
     {   /* Modify the program and insert the new TRIE node */
         U8 nodetype =(U8) flags;
         U8 trie_op = TRIE;
@@ -1661,9 +1727,7 @@ Perl_construct_ahocorasick_from_trie(pTHX_ RExC_state_t *pRExC_state, regnode *s
     const U32 trie_offset = TRIE_DATA_SLOT(source);
     reg_trie_data *trie = (reg_trie_data *)RExC_rxi->data->data[trie_offset];
     U32 *q;
-    const U32 ucharcount = TRIE_ALPHABET_SIZE;
     const U32 numstates = trie->statecount;
-    const U32 ubound = trie->lasttrans + ucharcount;
     U32 q_read = 0;
     U32 q_write = 0;
     U32 octet;
@@ -1716,8 +1780,7 @@ Perl_construct_ahocorasick_from_trie(pTHX_ RExC_state_t *pRExC_state, regnode *s
     if (base) {
         for ( octet = trie->states[1].min_octet;
               octet <= trie->states[1].max_octet; octet++ ) {
-            const U32 newstate = S_trie_trans_state(trie, 1, base, ucharcount,
-                                                    octet, 0, ubound);
+            const U32 newstate = S_trie_trans_state(trie, 1, base, octet, 0);
             if ( newstate ) {
                 q[ q_write ] = newstate;
                 /* set to point at the root */
@@ -1733,19 +1796,18 @@ Perl_construct_ahocorasick_from_trie(pTHX_ RExC_state_t *pRExC_state, regnode *s
             for ( octet = aho->states[cur].min_octet;
                   octet <= aho->states[cur].max_octet; octet++ ) {
                 const U32 ch_state = S_trie_trans_state(trie, cur, base,
-                                                        ucharcount, octet, 1,
-                                                        ubound);
+                                                        octet, 1);
                 if (ch_state) {
                     U32 fail_state = cur;
                     U32 fail_base;
                     do {
                         fail_state = fail[ fail_state ];
                         fail_base = aho->states[ fail_state ].trans.base;
                     } while ( !S_trie_trans_state(trie, fail_state, fail_base,
-                                                  ucharcount, octet, 1, ubound) );
+                                                  octet, 1) );
 
                     fail_state = S_trie_trans_state(trie, fail_state, fail_base,
-                                                ucharcount, octet, 1, ubound);
+                                                octet, 1);
                     fail[ ch_state ] = fail_state;
                     if ( !aho->states[ ch_state ].wordnum && aho->states[ fail_state ].wordnum )
                     {