Fix off-by-one error in inversion lists.

The first commit of this topic branch added a dummy 0 element to the end
of certain inversion lists to work around an off-by-one error.  This
commit makes the necessary changes to stop that error, and to remove
the dummy element.  SvCUR() and invlist_len() now are kept in sync.

diff --git a/charclass_invlists.h b/charclass_invlists.h
index b1a6772..f3de65a 100644 (file)
--- a/charclass_invlists.h
+++ b/charclass_invlists.h
@@ -11,12 +11,11 @@
 
 static const UV Latin1_invlist[] = {
        2,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       148565664, /* Version and data structure type */
        0,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
-       256,
-       0
+       256
 };
 
 #endif
@@ -24,8 +23,8 @@ static const UV Latin1_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV AboveLatin1_invlist[] = {
-       1,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       2,      /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -38,12 +37,11 @@ static const UV AboveLatin1_invlist[] = {
 
 static const UV ASCII_invlist[] = {
        2,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       148565664, /* Version and data structure type */
        0,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
-       128,
-       0
+       128
 };
 
 #endif
@@ -51,8 +49,8 @@ static const UV ASCII_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV L1Cased_invlist[] = {
-       16,     /* Number of elements */
-       1826693541, /* Version and data structure type */
+       17,     /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -79,8 +77,8 @@ static const UV L1Cased_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV VertSpace_invlist[] = {
-       6,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       7,      /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -97,8 +95,8 @@ static const UV VertSpace_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV PerlSpace_invlist[] = {
-       4,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       5,      /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -113,8 +111,8 @@ static const UV PerlSpace_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV XPerlSpace_invlist[] = {
-       22,     /* Number of elements */
-       1826693541, /* Version and data structure type */
+       23,     /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -147,8 +145,8 @@ static const UV XPerlSpace_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV PosixAlnum_invlist[] = {
-       6,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       7,      /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -165,8 +163,8 @@ static const UV PosixAlnum_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV L1PosixAlnum_invlist[] = {
-       18,     /* Number of elements */
-       1826693541, /* Version and data structure type */
+       19,     /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -195,8 +193,8 @@ static const UV L1PosixAlnum_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV PosixAlpha_invlist[] = {
-       4,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       5,      /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -211,8 +209,8 @@ static const UV PosixAlpha_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV L1PosixAlpha_invlist[] = {
-       16,     /* Number of elements */
-       1826693541, /* Version and data structure type */
+       17,     /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -239,8 +237,8 @@ static const UV L1PosixAlpha_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV PosixBlank_invlist[] = {
-       4,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       5,      /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -255,8 +253,8 @@ static const UV PosixBlank_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV XPosixBlank_invlist[] = {
-       18,     /* Number of elements */
-       1826693541, /* Version and data structure type */
+       19,     /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -286,14 +284,13 @@ static const UV XPosixBlank_invlist[] = {
 
 static const UV PosixCntrl_invlist[] = {
        4,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       148565664, /* Version and data structure type */
        0,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
        32,
        127,
-       128,
-       0
+       128
 };
 
 #endif
@@ -302,14 +299,13 @@ static const UV PosixCntrl_invlist[] = {
 
 static const UV XPosixCntrl_invlist[] = {
        4,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       148565664, /* Version and data structure type */
        0,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
        32,
        127,
-       160,
-       0
+       160
 };
 
 #endif
@@ -317,8 +313,8 @@ static const UV XPosixCntrl_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV PosixDigit_invlist[] = {
-       2,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       3,      /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -331,8 +327,8 @@ static const UV PosixDigit_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV PosixGraph_invlist[] = {
-       2,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       3,      /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -345,8 +341,8 @@ static const UV PosixGraph_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV L1PosixGraph_invlist[] = {
-       4,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       5,      /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -361,8 +357,8 @@ static const UV L1PosixGraph_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV PosixLower_invlist[] = {
-       2,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       3,      /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -375,8 +371,8 @@ static const UV PosixLower_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV L1PosixLower_invlist[] = {
-       12,     /* Number of elements */
-       1826693541, /* Version and data structure type */
+       13,     /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -399,8 +395,8 @@ static const UV L1PosixLower_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV PosixPrint_invlist[] = {
-       2,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       3,      /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -413,8 +409,8 @@ static const UV PosixPrint_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV L1PosixPrint_invlist[] = {
-       4,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       5,      /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -429,8 +425,8 @@ static const UV L1PosixPrint_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV PosixPunct_invlist[] = {
-       8,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       9,      /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -449,8 +445,8 @@ static const UV PosixPunct_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV L1PosixPunct_invlist[] = {
-       20,     /* Number of elements */
-       1826693541, /* Version and data structure type */
+       21,     /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -481,8 +477,8 @@ static const UV L1PosixPunct_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV PosixSpace_invlist[] = {
-       4,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       5,      /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -497,8 +493,8 @@ static const UV PosixSpace_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV XPosixSpace_invlist[] = {
-       22,     /* Number of elements */
-       1826693541, /* Version and data structure type */
+       23,     /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -531,8 +527,8 @@ static const UV XPosixSpace_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV PosixUpper_invlist[] = {
-       2,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       3,      /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -545,8 +541,8 @@ static const UV PosixUpper_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV L1PosixUpper_invlist[] = {
-       6,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       7,      /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -563,8 +559,8 @@ static const UV L1PosixUpper_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV PosixWord_invlist[] = {
-       8,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       9,      /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -583,8 +579,8 @@ static const UV PosixWord_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV L1PosixWord_invlist[] = {
-       20,     /* Number of elements */
-       1826693541, /* Version and data structure type */
+       21,     /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -615,8 +611,8 @@ static const UV L1PosixWord_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV PosixXDigit_invlist[] = {
-       6,      /* Number of elements */
-       1826693541, /* Version and data structure type */
+       7,      /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -633,8 +629,8 @@ static const UV PosixXDigit_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV XPosixXDigit_invlist[] = {
-       12,     /* Number of elements */
-       1826693541, /* Version and data structure type */
+       13,     /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -655,8 +651,8 @@ static const UV XPosixXDigit_invlist[] = {
 #endif
 
 static const UV NonL1_Perl_Non_Final_Folds_invlist[] = {
-       44,     /* Number of elements */
-       1826693541, /* Version and data structure type */
+       45,     /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,
@@ -709,8 +705,8 @@ static const UV NonL1_Perl_Non_Final_Folds_invlist[] = {
 #ifndef PERL_IN_XSUB_RE
 
 static const UV _Perl_Multi_Char_Folds_invlist[] = {
-       58,     /* Number of elements */
-       1826693541, /* Version and data structure type */
+       59,     /* Number of elements */
+       148565664, /* Version and data structure type */
        1,      /* 0 if the list starts at 0;
                   1 if it starts at the element beyond 0 */
        0,

diff --git a/regcomp.c b/regcomp.c
index f109681..61639ce 100644 (file)
--- a/regcomp.c
+++ b/regcomp.c
@@ -7048,10 +7048,10 @@ S_reg_scan_name(pTHX_ RExC_state_t *pRExC_state, U32 flags)
  * list.)
  * Taking the complement (inverting) an inversion list is quite simple, if the
  * first element is 0, remove it; otherwise add a 0 element at the beginning.
- * This implementation reserves an element (considered to be the final element
- * of the header) at the beginning of each inversion list to always contain 0;
- * there is an additional flag in the header which indicates if the list begins
- * at the 0, or is offset to begin at the next element.
+ * This implementation reserves an element at the beginning of each inversion
+ * list to always contain 0; there is an additional flag in the header which
+ * indicates if the list begins at the 0, or is offset to begin at the next
+ * element.
  *
  * More about inversion lists can be found in "Unicode Demystified"
  * Chapter 13 by Richard Gillam, published by Addison-Wesley.
@@ -7067,12 +7067,9 @@ S_reg_scan_name(pTHX_ RExC_state_t *pRExC_state, U32 flags)
 
 /* The header definitions are in F<inline_invlist.c> */
 
-/* This converts to/from our UVs to what the SV code is expecting: bytes.  The
- * first element is always a 0, which may or may not currently be in the list.
- * Just assume the worst case, that it isn't, and so the length of the list
- * passed in has to add 1 to account for it */
-#define TO_INTERNAL_SIZE(x) (((x) + 1) * sizeof(UV))
-#define FROM_INTERNAL_SIZE(x) (((x)/ sizeof(UV)) - 1)
+/* This converts to/from our UVs to what the SV code is expecting: bytes. */
+#define TO_INTERNAL_SIZE(x) ((x) * sizeof(UV))
+#define FROM_INTERNAL_SIZE(x) ((x)/ sizeof(UV))
 
 
 PERL_STATIC_INLINE UV*
@@ -7125,16 +7122,18 @@ S_invlist_array(pTHX_ SV* const invlist)
 PERL_STATIC_INLINE void
 S_invlist_set_len(pTHX_ SV* const invlist, const UV len)
 {
-    /* Sets the current number of elements stored in the inversion list */
+    /* Sets the current number of elements stored in the inversion list.
+     * Updates SvCUR correspondingly */
 
     PERL_ARGS_ASSERT_INVLIST_SET_LEN;
 
     *_get_invlist_len_addr(invlist) = len;
 
-    assert(SvLEN(invlist) == 0 || len <= SvLEN(invlist));
-
-    SvCUR_set(invlist, TO_INTERNAL_SIZE(len));
-    /* Note that when inverting, SvCUR shouldn't change */
+    SvCUR_set(invlist,
+              (len == 0)
+               ? 0
+               : TO_INTERNAL_SIZE(len + *get_invlist_offset_addr(invlist)));
+    assert(SvLEN(invlist) == 0 || SvCUR(invlist) <= SvLEN(invlist));
 }
 
 PERL_STATIC_INLINE IV*
@@ -7178,9 +7177,11 @@ S_invlist_max(pTHX_ SV* const invlist)
 
     PERL_ARGS_ASSERT_INVLIST_MAX;
 
+    /* Assumes worst case, in which the 0 element is not counted in the
+     * inversion list, so subtracts 1 for that */
     return SvLEN(invlist) == 0  /* This happens under _new_invlist_C_array */
-           ? _invlist_len(invlist)
-           : FROM_INTERNAL_SIZE(SvLEN(invlist));
+           ? FROM_INTERNAL_SIZE(SvCUR(invlist)) - 1
+           : FROM_INTERNAL_SIZE(SvLEN(invlist)) - 1;
 }
 
 PERL_STATIC_INLINE bool*
@@ -7211,8 +7212,10 @@ Perl__new_invlist(pTHX_ IV initial_size)
 
     /* Allocate the initial space */
     new_list = newSV_type(SVt_INVLIST);
-    SvGROW(new_list, TO_INTERNAL_SIZE(initial_size) + 1); /* 1 is for trailing
-                                                             NUL */
+
+    /* First 1 is in case the zero element isn't in the list; second 1 is for
+     * trailing NUL */
+    SvGROW(new_list, TO_INTERNAL_SIZE(initial_size + 1) + 1);
     invlist_set_len(new_list, 0);
 
     /* Force iterinit() to be used to get iteration to work */
@@ -7242,7 +7245,7 @@ S__new_invlist_C_array(pTHX_ const UV* const list)
      * (if appropriate) and regenerate INVLIST_VERSION_ID by running
      *      perl -E 'say int(rand 2**31-1)'
      */
-#define INVLIST_VERSION_ID 1826693541/* This is a combination of a version and
+#define INVLIST_VERSION_ID 148565664 /* This is a combination of a version and
                                         data structure type, so that one being
                                         passed in can be validated to be an
                                         inversion list of the correct vintage.
@@ -7262,17 +7265,14 @@ S__new_invlist_C_array(pTHX_ const UV* const list)
 
     SvLEN_set(invlist, 0);  /* Means we own the contents, and the system
                               shouldn't touch it */
-    /* The 'length' passed to us is the number of elements in the inversion
-     * list.  If the list doesn't include the always present 0 element, the
-     * length should be adjusted upwards to include it.  The TO_INTERNAL_SIZE
-     * macro returns a worst case scenario, always making that adjustment, even
-     * if not needed.  To get the precise size, then, we have to subtract 1
-     * when that adjustment wasn't needed.  That happens when the offset was 0;
-     * the exclusive-or flips the 0 to 1, and vice versa */
-    SvCUR_set(invlist, TO_INTERNAL_SIZE(length - (offset ^ 1)));
+    /* The 'length' passed to us is the physical number of elements in the
+     * inversion list. */
+    SvCUR_set(invlist, TO_INTERNAL_SIZE(length));
+
     *(get_invlist_offset_addr(invlist)) = offset;
 
-    invlist_set_len(invlist, length);
+    /* But if there is an offset the logical number is one less than that */
+    invlist_set_len(invlist, length  - offset);
 
     invlist_set_previous_index(invlist, 0);
 
@@ -7289,7 +7289,9 @@ S_invlist_extend(pTHX_ SV* const invlist, const UV new_max)
 
     PERL_ARGS_ASSERT_INVLIST_EXTEND;
 
-    SvGROW((SV *)invlist, TO_INTERNAL_SIZE(new_max));
+    /* Add one to account for the zero element at the beginning which may not
+     * be counted by the calling parameters */
+    SvGROW((SV *)invlist, TO_INTERNAL_SIZE(new_max + 1));
 }
 
 PERL_STATIC_INLINE void
@@ -8140,7 +8142,6 @@ S_invlist_clone(pTHX_ SV* const invlist)
 
     PERL_ARGS_ASSERT_INVLIST_CLONE;
 
-    SvCUR_set(new_invlist, physical_length); /* This isn't done automatically */
     *(get_invlist_offset_addr(new_invlist))
                                 = *(get_invlist_offset_addr(invlist));
     invlist_set_len(new_invlist, _invlist_len(invlist));

diff --git a/regen/mk_invlists.pl b/regen/mk_invlists.pl
index c1caf4f..b857b10 100644 (file)
--- a/regen/mk_invlists.pl
+++ b/regen/mk_invlists.pl
@@ -15,7 +15,7 @@ require 'regen/regen_lib.pl';
 # in the headers is used to minimize the possibility of things getting
 # out-of-sync, or the wrong data structure being passed.  Currently that
 # random number is:
-my $VERSION_DATA_STRUCTURE_TYPE = 1826693541;
+my $VERSION_DATA_STRUCTURE_TYPE = 148565664;
 
 my $out_fh = open_new('charclass_invlists.h', '>',
                      {style => '*', by => $0,
@@ -38,16 +38,11 @@ sub output_invlist ($$) {
 
     # Is the last element of the header 0, or 1 ?
     my $zero_or_one = 0;
-    my $count = @$invlist;
     if ($invlist->[0] != 0) {
         unshift @$invlist, 0;
         $zero_or_one = 1;
     }
-    else {  # Temporary until we disentangle the header from the body.  In the
-            # meantime, Adding this 0 at the end of lists that begin with 0
-            # will keep the code from reading beyond the end of the array.
-        push @$invlist, 0;
-    }
+    my $count = @$invlist;
 
     print $out_fh "\n#ifndef PERL_IN_XSUB_RE\n" unless exists $include_in_ext_re{$name};
     print $out_fh "\nstatic const UV ${name}_invlist[] = {\n";