Re: address warnings from GCC's UB sanitizer

[Akim Demaille]

> Le 16 mars 2019 à 17:21, Paul Eggert <address@hidden> a écrit :
> 
> Akim Demaille wrote:
>> Are you aware of any way to use this macro and avoid these spurious warnings?
> 
> Sure, don't use -Wtype-limits and don't use 
> -Wtautological-constant-out-of-range-compare.

:) :) :)

Sure.  Except that I don't explicitly ask for it.  So I'd have to check for the 
-Wno- options, and make sure not to use it in the test suite (people do compile 
the generated parsers with all sorts of crazy options).  So eventually it 
becomes more difficult to use INT_ADD_WRAPV that directly the computation.

> That's standard practice in other GNU projects. These warnings generate too 
> many false alarms in code that checks for integer overflow properly.

That's the first case I have so far.  Maybe I'll change my mind if I get more 
of them, which is however unlikely since:

> I should also mention that recent GCC versions do better in this area, due to 
> the fact that they have builtins that avoid the warnings. So one possibility 
> is to enable those particular warnings only if GCC is new enough. However, I 
> haven't bother to do that myself, as I haven't found the warnings useful (too 
> many false alarms anyway).

Yes, I've seen that on the CI and in the code.  I'm quite impressed with 
intprops.h, I had no idea there were builtins for safer arithmetics.

So I think I'll stay with the following version, unless you object to it.

Thanks Paul!


commit 7f066065c32bf173313d4839dc4ff62ec2f254b2
Author: Akim Demaille <address@hidden>
Date:   Tue Mar 12 19:09:10 2019 +0100

    address warnings from GCC's UB sanitizer
    
    Running with CC='gcc-mp-8 -fsanitize=undefined' revealed Undefined
    Behaviors.
    https://lists.gnu.org/archive/html/bison-patches/2019-03/msg00008.html
    
    * src/state.c (errs_new): Don't call memcpy with NULL as source.
    * src/location.c (add_column_width): Don't assume that the column
    argument is nonnegative: the scanner sometimes "backtracks" (e.g., see
    ROLLBACK_CURRENT_TOKEN and DEPRECATED) in which case we can have
    negative column numbers (temporarily).
    Found in test 3 (Invalid inputs).

diff --git a/src/location.c b/src/location.c
index ab478107..2597e79f 100644
--- a/src/location.c
+++ b/src/location.c
@@ -31,25 +31,16 @@ location const empty_location = EMPTY_LOCATION_INIT;
 
 /* If BUF is null, add BUFSIZE (which in this case must be less than
    INT_MAX) to COLUMN; otherwise, add mbsnwidth (BUF, BUFSIZE, 0) to
-   COLUMN.  If an overflow occurs, or might occur but is undetectable,
-   return INT_MAX.  Assume COLUMN is nonnegative.  */
+   COLUMN.  If an overflow occurs, return INT_MAX.  */
 
 static inline int
 add_column_width (int column, char const *buf, size_t bufsize)
 {
-  size_t width;
-  unsigned remaining_columns = INT_MAX - column;
-
-  if (buf)
-    {
-      if (INT_MAX / 2 <= bufsize)
-        return INT_MAX;
-      width = mbsnwidth (buf, bufsize, 0);
-    }
-  else
-    width = bufsize;
-
-  return width <= remaining_columns ? column + width : INT_MAX;
+  int width
+    = buf ? mbsnwidth (buf, bufsize, 0)
+    : INT_MAX <= bufsize ? INT_MAX
+    : bufsize;
+  return column <= INT_MAX - width ? column + width : INT_MAX;
 }
 
 /* Set *LOC and adjust scanner cursor to account for token TOKEN of
@@ -66,7 +57,7 @@ location_compute (location *loc, boundary *cur, char const 
*token, size_t size)
 
   loc->start = *cur;
 
-  for (p = token; p < lim; p++)
+  for (p = token; p < lim; ++p)
     switch (*p)
       {
       case '\n':
diff --git a/src/state.c b/src/state.c
index b8b8e2ff..58980954 100644
--- a/src/state.c
+++ b/src/state.c
@@ -77,7 +77,8 @@ errs_new (int num, symbol **tokens)
   size_t symbols_size = num * sizeof *tokens;
   errs *res = xmalloc (offsetof (errs, symbols) + symbols_size);
   res->num = num;
-  memcpy (res->symbols, tokens, symbols_size);
+  if (tokens)
+    memcpy (res->symbols, tokens, symbols_size);
   return res;
 }