branch master updated: Comments on potential overflows, change in associated comments

[Patrice Dumas]

This is an automated email from the git hooks/post-receive script.

pertusus pushed a commit to branch master
in repository texinfo.

The following commit(s) were added to refs/heads/master by this push:
     new c9e325090c Comments on potential overflows, change in associated 
comments
c9e325090c is described below

commit c9e325090c7c7a8f097cac4137717982de982c44
Author: Patrice Dumas <pertusus@free.fr>
AuthorDate: Sun Aug 18 23:06:23 2024 +0200

    Comments on potential overflows, change in associated comments
---
 ChangeLog                            |  4 +++
 tp/Texinfo/XS/convert/ConvertXS.xs   |  3 +-
 tp/Texinfo/XS/main/build_perl_info.c | 56 ++++++++++++++++--------------------
 tp/Texinfo/XS/main/get_perl_info.c   |  1 -
 tp/Texinfo/XS/main/tree.c            |  4 +--
 tp/Texinfo/XS/parsetexi/Parsetexi.xs |  3 +-
 tp/Texinfo/XS/parsetexi/end_line.c   |  5 ++--
 7 files changed, 34 insertions(+), 42 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index e5a1112f86..d388d7a4e7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2024-08-18  Patrice Dumas  <pertusus@free.fr>
+
+       Comments on potential overflows, change in associated comments
+
 2024-08-18  Patrice Dumas  <pertusus@free.fr>
 
        * tp/t/converters_tests.t (form_feeds), tp/Makefile.tres: add parts of
diff --git a/tp/Texinfo/XS/convert/ConvertXS.xs 
b/tp/Texinfo/XS/convert/ConvertXS.xs
index 7cd2d176a3..686a70dda8 100644
--- a/tp/Texinfo/XS/convert/ConvertXS.xs
+++ b/tp/Texinfo/XS/convert/ConvertXS.xs
@@ -158,8 +158,7 @@ converter_defaults (SV *converter_in, SV *conf_sv)
 
            /* the converter needs to be found, nothing else to pass to
               Perl */
-  /* FIXME converter->converter_descriptor is size_t, there will be an overflow
-           if > max of IV */
+           /* NOTE unlikely IV overflow if PERL_QUAD_MAX < SIZE_MAX */
             hv_store (converter_hv, key, strlen (key),
                       newSViv ((IV)self->converter_descriptor), 0);
 
diff --git a/tp/Texinfo/XS/main/build_perl_info.c 
b/tp/Texinfo/XS/main/build_perl_info.c
index 5518cc4961..a96100a9bb 100644
--- a/tp/Texinfo/XS/main/build_perl_info.c
+++ b/tp/Texinfo/XS/main/build_perl_info.c
@@ -13,13 +13,26 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>. */
 
-/* TODO there are many places where there is a theoretical possibility of 
overflow
-   if the size of a Perl array is not enough to accomodate the number of 
Texinfo
-   objects stored.  This is possible if max of SSize_t < max of size_t.  This 
is
-   only theoretical, though as these are big numbers, and according to internet
-   memory could be exhausted before reaching SSize_t.  It sould be checked
-   if the max of SSize_t was available, but it does not seems to be in Perl
-   documentation. */
+/* NOTE we store in AV lists indexed as size_t.  AV max size seems to be
+   the max of SSize_t, if it is < SIZE_MAX there could theoretically be
+   overflows.  However, Perl documentation says sizeof(SSize_t) == 
sizeof(Size_t)
+   and "Size_t ... is usually size_t".  In addition these are very big numbers.
+   In Perl documentation there is no description of a constant that would
+   give the max of SSize_t.
+
+   We build index numbers, document, output units and converter descriptors
+   indexed as size_t to Perl SV using newSViv ((IV)descriptor).  There
+   could theoretically be and overflow of IV if PERL_QUAD_MAX < SIZE_MAX.
+   (PERL_QUAD_MAX is the max size of IV in Perl).  On an Intel 64 bit
+   GNU Linux, PERL_QUAD_MAX is half of SIZE_MAX.  However those are
+   big numbers, while the Texinfo command numbers are quite small and other
+   descriptor numbers should be very small so this should not be an issue
+   in practice.
+
+   We also get IV and cast to size_t when getting info from Perl, like
+   descriptor = (size_t) SvIV (descriptor_sv).  In that case there is no
+   reason to overflow.
+ */
 
 #include <config.h>
 #include <stdlib.h>
@@ -224,7 +237,6 @@ build_perl_array (const ELEMENT_LIST *e_l, int 
avoid_recursion)
               element_to_perl_hash (e_l->list[i], avoid_recursion);
             }
         }
-      /* NOTE theoretical overflow if max(SSize_t) < i */
       av_store (av, (SSize_t) i, newRV_inc ((SV *) e_l->list[i]->hv));
     }
   return sv;
@@ -269,7 +281,6 @@ build_perl_const_element_array (const CONST_ELEMENT_LIST 
*e_l, int avoid_recursi
               element_to_perl_hash (f, avoid_recursion);
             }
         }
-      /* NOTE theoretical overflow if max(SSize_t) < i */
       av_store (av, (SSize_t) i, newRV_inc ((SV *) e_l->list[i]->hv));
     }
   return sv;
@@ -614,7 +625,6 @@ store_source_mark_list (const ELEMENT *e)
                 break;
             }
 
-          /* NOTE theoretical overflow if max(SSize_t) < i */
           av_push (av, newRV_noinc ((SV *)source_mark));
 #undef STORE
         }
@@ -958,7 +968,6 @@ element_to_perl_hash (ELEMENT *e, int avoid_recursion)
          released when the element is destroyed, by calling
          unregister_perl_tree_element */
           sv = newRV_inc ((SV *) child->hv);
-         /* NOTE theoretical overflow if max(SSize_t) < i */
           av_store (av, i, sv);
         }
     }
@@ -979,7 +988,6 @@ element_to_perl_hash (ELEMENT *e, int avoid_recursion)
           if (!child->hv || !avoid_recursion)
             element_to_perl_hash (child, avoid_recursion);
           sv = newRV_inc ((SV *) child->hv);
-          /* NOTE theoretical overflow if max(SSize_t) < i */
           av_store (av, i, sv);
         }
     }
@@ -1077,7 +1085,6 @@ build_string_list (const STRING_LIST *strings_list, enum 
sv_string_type type)
   for (i = 0; i < strings_list->number; i++)
     {
       const char *value = strings_list->list[i];
-      /* NOTE theoretical overflow if max(SSize_t) < i */
       if (!value)
         av_push (av, newSV (0));
       else if (type == svt_char)
@@ -1104,7 +1111,6 @@ build_elements_list (const CONST_ELEMENT_LIST *list)
   for (i = 0; i < list->number; i++)
     {
       sv = newRV_inc (list->list[i]->hv);
-      /* NOTE theoretical overflow if max(SSize_t) < i */
       av_store (list_av, i, sv);
     }
 
@@ -1206,7 +1212,6 @@ build_errors (const ERROR_MESSAGE *error_list, size_t 
error_number)
   for (i = 0; i < error_number; i++)
     {
       SV *sv = convert_error (error_list[i]);
-      /* NOTE theoretical overflow if max(SSize_t) < i */
       av_push (av, sv);
     }
 
@@ -1272,7 +1277,6 @@ add_formatted_error_messages (const ERROR_MESSAGE_LIST 
*error_messages,
               const ERROR_MESSAGE error_msg = error_messages->list[i];
               SV *sv = convert_error (error_msg);
 
-              /* NOTE theoretical overflow if max(SSize_t) < i */
               av_push (av, sv);
             }
 
@@ -1384,7 +1388,6 @@ build_target_elements_list (const LABEL_LIST *labels_list)
   for (i = 0; i < labels_list->number; i++)
     {
       sv = newRV_inc (labels_list->list[i].element->hv);
-      /* NOTE theoretical overflow if max(SSize_t) < i */
       av_store (target_array, i, sv);
     }
 
@@ -1429,7 +1432,6 @@ build_internal_xref_list (const ELEMENT_LIST 
*internal_xref_list)
   for (i = 0; i < internal_xref_list->number; i++)
     {
       sv = newRV_inc (internal_xref_list->list[i]->hv);
-      /* NOTE theoretical overflow if max(SSize_t) < i */
       av_store (list_av, i, sv);
     }
 
@@ -1474,7 +1476,6 @@ build_float_types_list (const FLOAT_RECORD_LIST *floats)
                         newRV_noinc ((SV *)av), 0);
         }
       sv = newRV_inc ((SV *)floats->list[i].element->hv);
-      /* NOTE theoretical overflow if max(SSize_t) < i */
       av_push (av, sv);
     }
 
@@ -1528,10 +1529,9 @@ build_single_index_data (const INDEX *index)
           if (e->entry_associated_element)
             STORE2("entry_associated_element",
                    newRV_inc ((SV *)e->entry_associated_element->hv));
-          /* FIXME if there is some overflow here there is gonna be trouble */
+          /* NOTE theoretical IV overflow if PERL_QUAD_MAX < SIZE_MAX */
           STORE2("entry_number", newSViv ((IV) entry_number));
 
-          /* NOTE theoretical overflow if max(SSize_t) < j */
           av_store (entries, j, newRV_noinc ((SV *)entry));
 
           entry_number++;
@@ -1672,7 +1672,6 @@ build_global_commands (const GLOBAL_COMMANDS 
*global_commands_ref)
       for (i = 0; i < global_commands.dircategory_direntry.number; i++)
         {
           const ELEMENT *e = global_commands.dircategory_direntry.list[i];
-          /* NOTE theoretical overflow if max(SSize_t) < i */
           if (e->hv)
             av_push (av, newRV_inc ((SV *) e->hv));
         }
@@ -1688,7 +1687,6 @@ build_global_commands (const GLOBAL_COMMANDS 
*global_commands_ref)
       for (i = 0; i < global_commands.footnotes.number; i++)
         {
           const ELEMENT *e = global_commands.footnotes.list[i];
-          /* NOTE theoretical overflow if max(SSize_t) < i */
           if (e->hv)
             av_push (av, newRV_inc ((SV *) e->hv));
         }
@@ -1703,7 +1701,6 @@ build_global_commands (const GLOBAL_COMMANDS 
*global_commands_ref)
       for (i = 0; i < global_commands.floats.number; i++)
         {
           const ELEMENT *e = global_commands.floats.list[i];
-          /* NOTE theoretical overflow if max(SSize_t) < i */
           if (e->hv)
             av_push (av, newRV_inc ((SV *) e->hv));
         }
@@ -1718,7 +1715,6 @@ build_global_commands (const GLOBAL_COMMANDS 
*global_commands_ref)
       for (i = 0; i < global_commands.cmd.number; i++)             \
         {                                                               \
           const ELEMENT *e = global_commands.cmd.list[i];            \
-          /* NOTE theoretical overflow if max(SSize_t) < i */          \
           if (e->hv)                                                    \
             av_push (av, newRV_inc ((SV *) e->hv));                     \
         }                                                               \
@@ -2467,8 +2463,7 @@ output_unit_to_perl_hash (OUTPUT_UNIT *output_unit)
         return;
     }
 
-  /* FIXME output_unit->index is size_t, there will be an overflow if
-     output_unit->index > max of IV */
+  /* NOTE theoretical IV overflow if PERL_QUAD_MAX < SIZE_MAX */
   sv = newSViv ((IV) output_unit->index);
   STORE("unit_index");
 
@@ -2537,7 +2532,6 @@ output_unit_to_perl_hash (OUTPUT_UNIT *output_unit)
 
           sv = newRV_inc ((SV *) element_hv);
 
-          /* NOTE theoretical overflow if max(SSize_t) < i */
           av_push (av, sv);
 
           unit_sv = newRV_inc ((SV *) output_unit->hv);
@@ -2618,8 +2612,7 @@ fill_output_units_descriptor_av (const DOCUMENT *document,
     }
 
   /* store in the first perl output unit of the list */
-  /* FIXME output_units_descriptor is size_t, there will be an overflow
-           if > max of IV */
+  /* NOTE theoretical IV overflow if PERL_QUAD_MAX < SIZE_MAX */
   hv_store (output_units->list[0]->hv, "output_units_descriptor",
             strlen ("output_units_descriptor"),
             newSViv ((IV)output_units_descriptor), 0);
@@ -3185,8 +3178,7 @@ pass_generic_converter_to_converter_sv (SV *converter_sv,
     STORE("converted_format", newSVpv_utf8 (converter->converted_format, 0));
 
   /* store converter_descriptor in perl converter */
-  /* FIXME converter->converter_descriptor is size_t, there will be an overflow
-           if > max of IV */
+  /* NOTE unlikely IV overflow if PERL_QUAD_MAX < SIZE_MAX */
   STORE("converter_descriptor", newSViv ((IV)converter->converter_descriptor));
 
 #undef STORE
diff --git a/tp/Texinfo/XS/main/get_perl_info.c 
b/tp/Texinfo/XS/main/get_perl_info.c
index 81734b8e41..6cd46fa228 100644
--- a/tp/Texinfo/XS/main/get_perl_info.c
+++ b/tp/Texinfo/XS/main/get_perl_info.c
@@ -127,7 +127,6 @@ get_document_or_warn (SV *sv_in, char *key, char 
*warn_string)
   document_descriptor_sv = hv_fetch (hv_in, key, strlen (key), 0);
   if (document_descriptor_sv && SvOK (*document_descriptor_sv))
     {
-      /* NOTE if size_t size is more than IV we could have overflow here */
       document_descriptor = (size_t) SvIV (*document_descriptor_sv);
       document = retrieve_document (document_descriptor);
     }
diff --git a/tp/Texinfo/XS/main/tree.c b/tp/Texinfo/XS/main/tree.c
index 443e385943..660639e900 100644
--- a/tp/Texinfo/XS/main/tree.c
+++ b/tp/Texinfo/XS/main/tree.c
@@ -373,7 +373,7 @@ void
 add_to_element_list (ELEMENT_LIST *list, ELEMENT *e)
 {
   /* NOTE there could be theoretically an overflow if
-     list->number + 1 > max (size_t).  The numbers are big, this is unlikely
+     list->number + 1 > SIZE_MAX.  The numbers are big, this is unlikely
      to happen */
   reallocate_list (list);
 
@@ -445,7 +445,7 @@ insert_list_slice_into_list (ELEMENT_LIST *to, size_t where,
                              const ELEMENT_LIST *from, size_t start, size_t 
end)
 {
   /* NOTE there could be theoretically an overflow if
-     list->number + num > max (size_t).  The numbers are big, this is unlikely
+     list->number + num > SIZE_MAX.  The numbers are big, this is unlikely
      to happen */
   size_t num = end - start;
   reallocate_list_for (num, to);
diff --git a/tp/Texinfo/XS/parsetexi/Parsetexi.xs 
b/tp/Texinfo/XS/parsetexi/Parsetexi.xs
index 19dd313e88..37d91a230c 100644
--- a/tp/Texinfo/XS/parsetexi/Parsetexi.xs
+++ b/tp/Texinfo/XS/parsetexi/Parsetexi.xs
@@ -71,8 +71,7 @@ register_parser_conf (SV *parser)
     CODE:
       hv_in = (HV *)SvRV (parser);
       parser_conf = register_conf ();
-      /* FIXME parser_conf->descriptor is size_t, there will be an overflow
-         if parser_conf->descriptor > max of IV, which seems to be 
PERL_QUAD_MAX */
+      /* NOTE unlikely IV overflow if PERL_QUAD_MAX < SIZE_MAX */
       hv_store (hv_in, key, strlen (key),
                 newSViv ((IV) parser_conf->descriptor), 0);
 
diff --git a/tp/Texinfo/XS/parsetexi/end_line.c 
b/tp/Texinfo/XS/parsetexi/end_line.c
index d3905c0241..c78f25ddc9 100644
--- a/tp/Texinfo/XS/parsetexi/end_line.c
+++ b/tp/Texinfo/XS/parsetexi/end_line.c
@@ -828,9 +828,8 @@ end_line_starting_block (ELEMENT *current)
   else if (command == CM_multitable)
     {
       size_t i;
-      /* NOTE max_columns could overflow, as in general max (int) < max 
(size_t).
-         We do not do anything special as this would be for unrealistically big
-         numbers for columns */
+      /* NOTE max_columns could overflow, as in general INT_MAX < SIZE_MAX.
+         We ignore as this would be for unrealistic column numbers */
       int max_columns = 0;
 
       for (i = 0; i < current->e.c->contents.number; i++)