undefined behaviour findings

undefined behaviour findings

[Bruno Haible]

Following Jeffrey Walton's example, I built and tested a gnulib
testdir with CC="gcc -fsanitize=undefined". Here are the findings.

Of course, since this is based on runtime tests, not gcc warnings, it does not
produce findings for modules without unit tests.

lib/bitrotate.h:98:14: runtime error: left shift of 43981 by 16 places cannot be represented in type 'int'
lib/bitrotate.h:109:25: runtime error: left shift of 43981 by 16 places cannot be represented in type 'int'

lib/bitset/table.c:784:54: runtime error: shift exponent 87 is too large for 64-bit type 'long unsigned int'

lib/des.c:552:3: runtime error: left shift of 255 by 24 places cannot be represented in type 'int'
lib/des.c:437:3: runtime error: left shift of 242 by 24 places cannot be represented in type 'int'
lib/des.c:625:3: runtime error: left shift of 254 by 24 places cannot be represented in type 'int'

lib/xmemdup0.c:41:3: runtime error: null pointer passed as argument 2, which is declared to never be null

tests/test-c-stack.c:66:13: runtime error: load of null pointer of type 'char'

tests/test-canonicalize.c:74:15: runtime error: null pointer passed as argument 1, which is declared to never be null

tests/test-count-leading-zeros.c:56:3: runtime error: left shift of 1804289383 by 31 places cannot be represented in type 'int'
tests/test-count-leading-zeros.c:56:3: runtime error: left shift of negative value -2147483648

tests/test-count-one-bits.c:55:3: runtime error: left shift of 1804289383 by 31 places cannot be represented in type 'int'
tests/test-count-one-bits.c:55:3: runtime error: left shift of negative value -2147483648

tests/test-count-trailing-zeros.c:56:3: runtime error: left shift of 1804289383 by 31 places cannot be represented in type 'int'
tests/test-count-trailing-zeros.c:56:3: runtime error: left shift of negative value -2147483648

tests/test-memmem.c:84:26: runtime error: null pointer passed as argument 3, which is declared to never be null


Let me fix some of them here.


2019-03-09  Bruno Haible  <bruno@clisp.org>

	Fix undefined behaviour.
	* lib/bitrotate.h (rotl16, rotr16, rotl8, rotr8): Case x to
	'unsigned int', to avoid shift operations on 'int'.
	* lib/xmemdup0.c (xmemdup0): Don't invoke memcpy with a zero size.
	* tests/test-count-leading-zeros.c (main): Use a random number that has
	as many bits as TYPE, not only 2*15 or 2*31 bits.
	* tests/test-count-trailing-zeros.c (main): Likewise.
	* tests/test-count-one-bits.c (main): Likewise.
	* tests/test-memmem.c: Don't include "null-ptr.h".
	(main): Use zerosize_ptr() instead of null_ptr().
	* modules/memmem-tests (Files): Remove tests/null-ptr.h.

diff --git a/lib/bitrotate.h b/lib/bitrotate.h
index 862331e..04b9083 100644
--- a/lib/bitrotate.h
+++ b/lib/bitrotate.h
@@ -95,7 +95,8 @@ rotr_sz (size_t x, int n)
 BITROTATE_INLINE uint16_t
 rotl16 (uint16_t x, int n)
 {
-  return ((x << n) | (x >> (16 - n))) & UINT16_MAX;
+  return (((unsigned int) x << n) | ((unsigned int) x >> (16 - n)))
+         & UINT16_MAX;
 }
 
 /* Given an unsigned 16-bit argument X, return the value corresponding
@@ -106,7 +107,8 @@ rotl16 (uint16_t x, int n)
 BITROTATE_INLINE uint16_t
 rotr16 (uint16_t x, int n)
 {
-  return ((x >> n) | (x << (16 - n))) & UINT16_MAX;
+  return (((unsigned int) x >> n) | ((unsigned int) x << (16 - n)))
+         & UINT16_MAX;
 }
 
 /* Given an unsigned 8-bit argument X, return the value corresponding
@@ -117,7 +119,7 @@ rotr16 (uint16_t x, int n)
 BITROTATE_INLINE uint8_t
 rotl8 (uint8_t x, int n)
 {
-  return ((x << n) | (x >> (8 - n))) & UINT8_MAX;
+  return (((unsigned int) x << n) | ((unsigned int) x >> (8 - n))) & UINT8_MAX;
 }
 
 /* Given an unsigned 8-bit argument X, return the value corresponding
@@ -128,7 +130,7 @@ rotl8 (uint8_t x, int n)
 BITROTATE_INLINE uint8_t
 rotr8 (uint8_t x, int n)
 {
-  return ((x >> n) | (x << (8 - n))) & UINT8_MAX;
+  return (((unsigned int) x >> n) | ((unsigned int) x << (8 - n))) & UINT8_MAX;
 }
 
 _GL_INLINE_HEADER_END
diff --git a/lib/xmemdup0.c b/lib/xmemdup0.c
index 4f491be..57c1b59 100644
--- a/lib/xmemdup0.c
+++ b/lib/xmemdup0.c
@@ -38,7 +38,8 @@ char *
 xmemdup0 (void const *p, size_t s)
 {
   char *result = xcharalloc (s + 1);
-  memcpy (result, p, s);
+  if (s > 0)
+    memcpy (result, p, s);
   result[s] = 0;
   return result;
 }

diff --git a/tests/test-count-leading-zeros.c b/tests/test-count-leading-zeros.c
index 3caae7f..e94e37e 100644
--- a/tests/test-count-leading-zeros.c
+++ b/tests/test-count-leading-zeros.c
@@ -34,23 +34,34 @@ main (int argc, char *argv[])
 {
   int i, j;
 
-#define TEST_COUNT_LEADING_ZEROS(FUNC, TYPE, BITS, MAX, ONE)    \
-  ASSERT (FUNC (0) == BITS);                                    \
-  for (i = 0; i < BITS; i++)                                    \
-    {                                                           \
-      ASSERT (FUNC (ONE << i) == BITS - i - 1);                 \
-      for (j = 0; j < i; j++)                                   \
-        ASSERT (FUNC ((ONE << i) | (ONE << j)) == BITS - i - 1);\
-    }                                                           \
-  for (i = 0; i < 1000; i++)                                    \
-    {                                                           \
-      TYPE value = rand () ^ (rand () << 31 << 1);              \
-      int count = 0;                                            \
-      for (j = 0; j < BITS; j++)                                \
-        if (value & (ONE << j))                                 \
-          count = BITS - j - 1;                                 \
-      ASSERT (count == FUNC (value));                           \
-    }                                                           \
+#define TEST_COUNT_LEADING_ZEROS(FUNC, TYPE, BITS, MAX, ONE)     \
+  ASSERT (FUNC (0) == BITS);                                     \
+  for (i = 0; i < BITS; i++)                                     \
+    {                                                            \
+      ASSERT (FUNC (ONE << i) == BITS - i - 1);                  \
+      for (j = 0; j < i; j++)                                    \
+        ASSERT (FUNC ((ONE << i) | (ONE << j)) == BITS - i - 1); \
+    }                                                            \
+  for (i = 0; i < 1000; i++)                                     \
+    {                                                            \
+      /* RAND_MAX is most often 0x7fff or 0x7fffffff.  */        \
+      TYPE value =                                               \
+        (RAND_MAX <= 0xffff                                      \
+         ? ((TYPE) rand () >> 3)                                 \
+           ^ (((TYPE) rand () >> 3) << 12)                       \
+           ^ (((TYPE) rand () >> 3) << 24)                       \
+           ^ (((TYPE) rand () >> 3) << 30 << 6)                  \
+           ^ (((TYPE) rand () >> 3) << 30 << 18)                 \
+           ^ (((TYPE) rand () >> 3) << 30 << 30)                 \
+         : ((TYPE) rand () >> 3)                                 \
+           ^ (((TYPE) rand () >> 3) << 22)                       \
+           ^ (((TYPE) rand () >> 3) << 22 << 22));               \
+      int count = 0;                                             \
+      for (j = 0; j < BITS; j++)                                 \
+        if (value & (ONE << j))                                  \
+          count = BITS - j - 1;                                  \
+      ASSERT (count == FUNC (value));                            \
+    }                                                            \
   ASSERT (FUNC (MAX) == 0);
 
   TEST_COUNT_LEADING_ZEROS (count_leading_zeros, unsigned int,
diff --git a/tests/test-count-one-bits.c b/tests/test-count-one-bits.c
index 109f3ee..852e1d6 100644
--- a/tests/test-count-one-bits.c
+++ b/tests/test-count-one-bits.c
@@ -34,22 +34,33 @@ main (int argc, char *argv[])
 {
   int i, j;
 
-#define TEST_COUNT_ONE_BITS(FUNC, TYPE, BITS, MAX, ONE) \
-  ASSERT (FUNC (0) == 0);                               \
-  for (i = 0; i < BITS; i++)                            \
-    {                                                   \
-      ASSERT (FUNC (ONE << i) == 1);                    \
-      for (j = i + 1; j < BITS; j++)                    \
-        ASSERT (FUNC ((ONE << i) | (ONE << j)) == 2);   \
-    }                                                   \
-  for (i = 0; i < 1000; i++)                            \
-    {                                                   \
-      TYPE value = rand () ^ (rand () << 31 << 1);      \
-      int count = 0;                                    \
-      for (j = 0; j < BITS; j++)                        \
-        count += (value & (ONE << j)) != 0;             \
-      ASSERT (count == FUNC (value));                   \
-    }                                                   \
+#define TEST_COUNT_ONE_BITS(FUNC, TYPE, BITS, MAX, ONE)   \
+  ASSERT (FUNC (0) == 0);                                 \
+  for (i = 0; i < BITS; i++)                              \
+    {                                                     \
+      ASSERT (FUNC (ONE << i) == 1);                      \
+      for (j = i + 1; j < BITS; j++)                      \
+        ASSERT (FUNC ((ONE << i) | (ONE << j)) == 2);     \
+    }                                                     \
+  for (i = 0; i < 1000; i++)                              \
+    {                                                     \
+      /* RAND_MAX is most often 0x7fff or 0x7fffffff.  */ \
+      TYPE value =                                        \
+        (RAND_MAX <= 0xffff                               \
+         ? ((TYPE) rand () >> 3)                          \
+           ^ (((TYPE) rand () >> 3) << 12)                \
+           ^ (((TYPE) rand () >> 3) << 24)                \
+           ^ (((TYPE) rand () >> 3) << 30 << 6)           \
+           ^ (((TYPE) rand () >> 3) << 30 << 18)          \
+           ^ (((TYPE) rand () >> 3) << 30 << 30)          \
+         : ((TYPE) rand () >> 3)                          \
+           ^ (((TYPE) rand () >> 3) << 22)                \
+           ^ (((TYPE) rand () >> 3) << 22 << 22));        \
+      int count = 0;                                      \
+      for (j = 0; j < BITS; j++)                          \
+        count += (value & (ONE << j)) != 0;               \
+      ASSERT (count == FUNC (value));                     \
+    }                                                     \
   ASSERT (FUNC (MAX) == BITS);
 
   TEST_COUNT_ONE_BITS (count_one_bits, unsigned int, UINT_BIT, UINT_MAX, 1U);
diff --git a/tests/test-count-trailing-zeros.c b/tests/test-count-trailing-zeros.c
index 5a18599..04d9ddc 100644
--- a/tests/test-count-trailing-zeros.c
+++ b/tests/test-count-trailing-zeros.c
@@ -34,23 +34,34 @@ main (int argc, char *argv[])
 {
   int i, j;
 
-#define TEST_COUNT_TRAILING_ZEROS(FUNC, TYPE, BITS, MAX, ONE)   \
-  ASSERT (FUNC (0) == BITS);                                    \
-  for (i = 0; i < BITS; i++)                                    \
-    {                                                           \
-      ASSERT (FUNC (ONE << i) == i);                            \
-      for (j = 0; j < i; j++)                                   \
-        ASSERT (FUNC ((ONE << i) | (ONE << j)) == j);           \
-    }                                                           \
-  for (i = 0; i < 1000; i++)                                    \
-    {                                                           \
-      TYPE value = rand () ^ (rand () << 31 << 1);              \
-      int count = 0;                                            \
-      for (j = BITS - 1; 0 <= j; j--)                           \
-        if (value & (ONE << j))                                 \
-          count = j;                                            \
-      ASSERT (count == FUNC (value));                           \
-    }                                                           \
+#define TEST_COUNT_TRAILING_ZEROS(FUNC, TYPE, BITS, MAX, ONE) \
+  ASSERT (FUNC (0) == BITS);                                  \
+  for (i = 0; i < BITS; i++)                                  \
+    {                                                         \
+      ASSERT (FUNC (ONE << i) == i);                          \
+      for (j = 0; j < i; j++)                                 \
+        ASSERT (FUNC ((ONE << i) | (ONE << j)) == j);         \
+    }                                                         \
+  for (i = 0; i < 1000; i++)                                  \
+    {                                                         \
+      /* RAND_MAX is most often 0x7fff or 0x7fffffff.  */     \
+      TYPE value =                                            \
+        (RAND_MAX <= 0xffff                                   \
+         ? ((TYPE) rand () >> 3)                              \
+           ^ (((TYPE) rand () >> 3) << 12)                    \
+           ^ (((TYPE) rand () >> 3) << 24)                    \
+           ^ (((TYPE) rand () >> 3) << 30 << 6)               \
+           ^ (((TYPE) rand () >> 3) << 30 << 18)              \
+           ^ (((TYPE) rand () >> 3) << 30 << 30)              \
+         : ((TYPE) rand () >> 3)                              \
+           ^ (((TYPE) rand () >> 3) << 22)                    \
+           ^ (((TYPE) rand () >> 3) << 22 << 22));            \
+      int count = 0;                                          \
+      for (j = BITS - 1; 0 <= j; j--)                         \
+        if (value & (ONE << j))                               \
+          count = j;                                          \
+      ASSERT (count == FUNC (value));                         \
+    }                                                         \
   ASSERT (FUNC (MAX) == 0);
 
   TEST_COUNT_TRAILING_ZEROS (count_trailing_zeros, unsigned int,
diff --git a/tests/test-memmem.c b/tests/test-memmem.c
index ed327e6..17e2e41 100644
--- a/tests/test-memmem.c
+++ b/tests/test-memmem.c
@@ -26,7 +26,6 @@ SIGNATURE_CHECK (memmem, void *, (void const *, size_t, void const *, size_t));
 #include <stdlib.h>
 #include <unistd.h>
 
-#include "null-ptr.h"
 #include "zerosize-ptr.h"
 #include "macros.h"
 
@@ -81,7 +80,7 @@ main (int argc, char *argv[])
 
   {
     const char input[] = "foo";
-    const char *result = memmem (input, strlen (input), null_ptr (), 0);
+    const char *result = memmem (input, strlen (input), zerosize_ptr (), 0);
     ASSERT (result == input);
   }
 
diff --git a/modules/memmem-tests b/modules/memmem-tests
index 28c0091..250ccbf 100644
--- a/modules/memmem-tests
+++ b/modules/memmem-tests
@@ -1,7 +1,6 @@
 Files:
 tests/test-memmem.c
 tests/signature.h
-tests/null-ptr.h
 tests/zerosize-ptr.h
 tests/macros.h
 m4/mmap-anon.m4

Re: undefined behaviour findings in bitset

[Bruno Haible]

Hi Akim,

This one I have not fixed:

lib/bitset/table.c:784:54: runtime error: shift exponent 87 is too large for 64-bit type 'long unsigned int'

To reproduce: Use gcc 8.2.0 and CC="gcc -fsanitize=undefined".

Bruno

Re: undefined behaviour findings

[Bruno Haible]

> lib/des.c:552:3: runtime error: left shift of 255 by 24 places cannot be represented in type 'int'
> lib/des.c:437:3: runtime error: left shift of 242 by 24 places cannot be represented in type 'int'
> lib/des.c:625:3: runtime error: left shift of 254 by 24 places cannot be represented in type 'int'

This one is fixed by this commit:


2019-03-09  Bruno Haible  <bruno@clisp.org>

	crypto/des: Fix undefined behaviour.
	* lib/des.c (READ_64BIT_DATA): Cast bytes to 'unsigned int', to avoid
	shift operations on 'int'.

diff --git a/lib/des.c b/lib/des.c
index f357192..ba174f7 100644
--- a/lib/des.c
+++ b/lib/des.c
@@ -407,11 +407,17 @@ gl_des_is_weak_key (const char * key)
  * Macros to convert 8 bytes from/to 32bit words.
  */
 #define READ_64BIT_DATA(data, left, right)                                 \
-    left  = (data[0] << 24) | (data[1] << 16) | (data[2] << 8) | data[3];  \
-    right = (data[4] << 24) | (data[5] << 16) | (data[6] << 8) | data[7];
+    left  = ((uint32_t) data[0] << 24)                                     \
+            | ((uint32_t) data[1] << 16)                                   \
+            | ((uint32_t) data[2] << 8)                                    \
+            | (uint32_t) data[3];                                          \
+    right = ((uint32_t) data[4] << 24)                                     \
+            | ((uint32_t) data[5] << 16)                                   \
+            | ((uint32_t) data[6] << 8)                                    \
+            | (uint32_t) data[7];
 
 #define WRITE_64BIT_DATA(data, left, right)                                \
-    data[0] = (left >> 24) &0xff; data[1] = (left >> 16) &0xff;    \
+    data[0] = (left >> 24) &0xff; data[1] = (left >> 16) &0xff;            \
     data[2] = (left >> 8) &0xff; data[3] = left &0xff;                     \
     data[4] = (right >> 24) &0xff; data[5] = (right >> 16) &0xff;          \
     data[6] = (right >> 8) &0xff; data[7] = right &0xff;

Re: undefined behaviour findings in bitset

[Akim Demaille]

Hi Bruno,

> Le 9 mars 2019 à 20:37, Bruno Haible <bruno@clisp.org> a écrit :
> 
> Hi Akim,
> 
> This one I have not fixed:
> 
> lib/bitset/table.c:784:54: runtime error: shift exponent 87 is too large for 64-bit type 'long unsigned int'
> 
> To reproduce: Use gcc 8.2.0 and CC="gcc -fsanitize=undefined".

Thanks for the report!  Here is my proposal, in three patches.

commit 6f8b57537eb47418a432b672cbb240713f005a6a
Author: Akim Demaille <akim.demaille@gmail.com>
Date:   Thu Mar 14 08:31:54 2019 +0100

    bitset: style changes
    
    * lib/bitset/table.c: Use NULL, not 0, for pointers.
    Formatting changes.
    (tbitset_list): Reduce scopes.

diff --git a/ChangeLog b/ChangeLog
index 544d69d4b..2352520a5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2019-03-14  Akim Demaille  <akim@lrde.epita.fr>
+
+	bitset: style changes.
+	* lib/bitset/table.c: Use NULL, not 0, for pointers.
+	Formatting changes.
+	(tbitset_list): Reduce scopes.
+
 2019-03-14  Bruno Haible  <bruno@clisp.org>
 
 	isatty: Make it return true in Cygwin consoles on native Windows.
diff --git a/lib/bitset/table.c b/lib/bitset/table.c
index 8351cf784..a129c8712 100644
--- a/lib/bitset/table.c
+++ b/lib/bitset/table.c
@@ -300,7 +300,7 @@ tbitset_elt_find (bitset bset, bitset_bindex bindex,
       abort ();
 
     case EBITSET_FIND:
-      return 0;
+      return NULL;
 
     case EBITSET_CREATE:
       if (eindex >= size)
@@ -588,8 +588,8 @@ tbitset_list_reverse (bitset bset, bitset_bindex *list,
 
 
 /* Find list of up to NUM bits set in BSET starting from and including
- *NEXT and store in array LIST.  Return with actual number of bits
- found and with *NEXT indicating where search stopped.  */
+   *NEXT and store in array LIST.  Return with actual number of bits
+   found and with *NEXT indicating where search stopped.  */
 static bitset_bindex
 tbitset_list (bitset bset, bitset_bindex *list,
               bitset_bindex num, bitset_bindex *next)
@@ -607,17 +607,14 @@ tbitset_list (bitset bset, bitset_bindex *list,
   if (bitno % EBITSET_ELT_BITS)
     {
       /* We need to start within an element.  This is not very common.  */
-
       tbitset_elt *elt = elts[eindex];
       if (elt)
         {
-          bitset_windex woffset;
           bitset_word *srcp = EBITSET_WORDS (elt);
+          bitset_windex woffset = eindex * EBITSET_ELT_WORDS;
 
-          bitset_windex windex = bitno / BITSET_WORD_BITS;
-          woffset = eindex * EBITSET_ELT_WORDS;
-
-          for (; (windex - woffset) < EBITSET_ELT_WORDS; windex++)
+          for (bitset_windex windex = bitno / BITSET_WORD_BITS;
+               (windex - woffset) < EBITSET_ELT_WORDS; windex++)
             {
               bitset_word word = srcp[windex - woffset] >> (bitno % BITSET_WORD_BITS);
 



commit f4a85b4ffa59650c36227fc94e4c5973e6449d48
Author: Akim Demaille <akim.demaille@gmail.com>
Date:   Sat Mar 16 17:16:48 2019 +0100

    bitset: fix bitset overflows
    
    Reported by Bruno Haible.
    https://lists.gnu.org/archive/html/bug-gnulib/2019-03/msg00017.html
    
    * lib/bitset/table.c (tbitset_test): last_bit is the position of
    the bit in the array of bitset_word, so be sure to take its modulo
    number-of-bits-in-bitset-word (i.e., EBITSET_ELT_WORDS).
    * lib/bitset/list.c (lbitset_unused_clear): Likewise.

diff --git a/ChangeLog b/ChangeLog
index 2352520a5..0e7a74a10 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2019-03-16  Akim Demaille  <akim@lrde.epita.fr>
+
+	bitset: fix bitset overflows.
+	Reported by Bruno Haible.
+	https://lists.gnu.org/archive/html/bug-gnulib/2019-03/msg00017.html
+	* lib/bitset/table.c (tbitset_test): last_bit is the position of
+	the bit in the array of bitset_word, so be sure to take its modulo
+	number-of-bits-in-bitset-word (i.e., EBITSET_ELT_WORDS).
+	* lib/bitset/list.c (lbitset_unused_clear): Likewise.
+
 2019-03-14  Akim Demaille  <akim@lrde.epita.fr>
 
 	bitset: style changes.
diff --git a/lib/bitset/list.c b/lib/bitset/list.c
index f42edb8ea..fe1b52869 100644
--- a/lib/bitset/list.c
+++ b/lib/bitset/list.c
@@ -859,7 +859,8 @@ lbitset_unused_clear (bitset dst)
       bitset_word *srcp = elt->words;
       bitset_windex windex = n_bits / BITSET_WORD_BITS;
 
-      srcp[windex - elt->index] &= ((bitset_word) 1 << last_bit) - 1;
+      srcp[windex - elt->index]
+        &= ((bitset_word) 1 << (last_bit % BITSET_WORD_BITS)) - 1;
       windex++;
 
       for (; (windex - elt->index) < LBITSET_ELT_WORDS; windex++)
diff --git a/lib/bitset/table.c b/lib/bitset/table.c
index a129c8712..9cac96469 100644
--- a/lib/bitset/table.c
+++ b/lib/bitset/table.c
@@ -778,7 +778,8 @@ tbitset_unused_clear (bitset dst)
           bitset_windex windex = n_bits / BITSET_WORD_BITS;
           bitset_windex woffset = eindex * EBITSET_ELT_WORDS;
 
-          srcp[windex - woffset] &= ((bitset_word) 1 << last_bit) - 1;
+          srcp[windex - woffset]
+            &= ((bitset_word) 1 << (last_bit % BITSET_WORD_BITS)) - 1;
           windex++;
           for (; (windex - woffset) < EBITSET_ELT_WORDS; windex++)
             srcp[windex - woffset] = 0;



commit 4c8a35ed125e1b87a11d6aecd0b7b216384552bb
Author: Akim Demaille <akim.demaille@gmail.com>
Date:   Sat Mar 16 17:36:22 2019 +0100

    bitset: a bit (...) more tests
    
    * tests/test-bitset.c (check_attributes): Check zero and ones.

diff --git a/ChangeLog b/ChangeLog
index 0e7a74a10..db23ee898 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2019-03-16  Akim Demaille  <akim@lrde.epita.fr>
+
+	bitset: a bit (...) more tests
+	* tests/test-bitset.c (check_attributes): Check zero and ones.
+
 2019-03-16  Akim Demaille  <akim@lrde.epita.fr>
 
 	bitset: fix bitset overflows.
diff --git a/tests/test-bitset.c b/tests/test-bitset.c
index 032865095..f4502d1d6 100644
--- a/tests/test-bitset.c
+++ b/tests/test-bitset.c
@@ -32,12 +32,18 @@ void assert_bitset_equal (bitset bs1, bitset bs2)
     ASSERT (bitset_test (bs1, i) == bitset_test (bs2, i));
 }
 
+static
 void bitset_random (bitset bs)
 {
   for (bitset_bindex i = 0; i < bitset_size (bs); ++i)
     bitset_set (bs, RANDOM (2));
 }
 
+
+/* Check various operations on random bitsets with two different
+   implementations.  */
+
+static
 void compare (enum bitset_attr a, enum bitset_attr b)
 {
   const int nbits = RANDOM (256);
@@ -62,10 +68,17 @@ void compare (enum bitset_attr a, enum bitset_attr b)
   bitset_copy (bsrc3, asrc3);
   bitset bdst = bitset_create (nbits, b);
 
+  /* not */
+  bitset_not (adst, asrc0);
+  bitset_not (bdst, bsrc0);
+  assert_bitset_equal (adst, bdst);
+
+  /* not */
   bitset_not (adst, asrc0);
   bitset_not (bdst, bsrc0);
   assert_bitset_equal (adst, bdst);
 
+  /* and */
   bitset_and (adst, asrc0, asrc1);
   bitset_and (bdst, bsrc0, bsrc1);
   assert_bitset_equal (adst, bdst);
@@ -73,6 +86,7 @@ void compare (enum bitset_attr a, enum bitset_attr b)
           == bitset_and_cmp (bdst, bsrc0, bsrc1));
   assert_bitset_equal (adst, bdst);
 
+  /* andn */
   bitset_andn (adst, asrc0, asrc1);
   bitset_andn (bdst, bsrc0, bsrc1);
   assert_bitset_equal (adst, bdst);
@@ -80,6 +94,7 @@ void compare (enum bitset_attr a, enum bitset_attr b)
           == bitset_andn_cmp (bdst, bsrc0, bsrc1));
   assert_bitset_equal (adst, bdst);
 
+  /* or */
   bitset_or (adst, asrc0, asrc1);
   bitset_or (bdst, bsrc0, bsrc1);
   assert_bitset_equal (adst, bdst);
@@ -87,6 +102,7 @@ void compare (enum bitset_attr a, enum bitset_attr b)
           == bitset_or_cmp (bdst, bsrc0, bsrc1));
   assert_bitset_equal (adst, bdst);
 
+  /* xor */
   bitset_xor (adst, asrc0, asrc1);
   bitset_xor (bdst, bsrc0, bsrc1);
   assert_bitset_equal (adst, bdst);
@@ -94,6 +110,7 @@ void compare (enum bitset_attr a, enum bitset_attr b)
           == bitset_xor_cmp (bdst, bsrc0, bsrc1));
   assert_bitset_equal (adst, bdst);
 
+  /* and_or */
   bitset_and_or (adst, asrc0, asrc1, asrc2);
   bitset_and_or (bdst, bsrc0, bsrc1, bsrc2);
   assert_bitset_equal (adst, bdst);
@@ -101,6 +118,7 @@ void compare (enum bitset_attr a, enum bitset_attr b)
           == bitset_and_or_cmp (bdst, bsrc0, bsrc1, bsrc2));
   assert_bitset_equal (adst, bdst);
 
+  /* andn_or */
   bitset_andn_or (adst, asrc0, asrc1, asrc2);
   bitset_andn_or (bdst, bsrc0, bsrc1, bsrc2);
   assert_bitset_equal (adst, bdst);
@@ -108,6 +126,7 @@ void compare (enum bitset_attr a, enum bitset_attr b)
           == bitset_andn_or_cmp (bdst, bsrc0, bsrc1, bsrc2));
   assert_bitset_equal (adst, bdst);
 
+  /* or_and */
   bitset_or_and (adst, asrc0, asrc1, asrc2);
   bitset_or_and (bdst, bsrc0, bsrc1, bsrc2);
   assert_bitset_equal (adst, bdst);
@@ -115,6 +134,16 @@ void compare (enum bitset_attr a, enum bitset_attr b)
           == bitset_or_and_cmp (bdst, bsrc0, bsrc1, bsrc2));
   assert_bitset_equal (adst, bdst);
 
+  /* ones */
+  bitset_ones (adst);
+  bitset_ones (bdst);
+  assert_bitset_equal (adst, bdst);
+
+  /* zero */
+  bitset_zero (adst);
+  bitset_zero (bdst);
+  assert_bitset_equal (adst, bdst);
+  
   bitset_free (bdst);
   bitset_free (bsrc3);
   bitset_free (bsrc2);
@@ -127,6 +156,11 @@ void compare (enum bitset_attr a, enum bitset_attr b)
   bitset_free (asrc0);
 }
 
+
+/* Check various operations against expected values for a bitset
+   having attributes ATTR.  */
+
+static
 void check_attributes (enum bitset_attr attr)
 {
   enum { nbits = 32 };

Re: undefined behaviour findings in bitset

[Akim Demaille]

> commit 4c8a35ed125e1b87a11d6aecd0b7b216384552bb
> Author: Akim Demaille <akim.demaille@gmail.com>
> Date:   Sat Mar 16 17:36:22 2019 +0100
> 
>    bitset: a bit (...) more tests
> 
>    * tests/test-bitset.c (check_attributes): Check zero and ones.
> 
> diff --git a/tests/test-bitset.c b/tests/test-bitset.c
> index 032865095..f4502d1d6 100644
> --- a/tests/test-bitset.c
> +++ b/tests/test-bitset.c
> @@ -62,10 +68,17 @@ void compare (enum bitset_attr a, enum bitset_attr b)
>   bitset_copy (bsrc3, asrc3);
>   bitset bdst = bitset_create (nbits, b);
> 
> +  /* not */
> +  bitset_not (adst, asrc0);
> +  bitset_not (bdst, bsrc0);
> +  assert_bitset_equal (adst, bdst);
> +
> +  /* not */
>   bitset_not (adst, asrc0);
>   bitset_not (bdst, bsrc0);
>   assert_bitset_equal (adst, bdst);

I have removed that spurious duplicate.

Re: undefined behaviour findings in bitset

[Bruno Haible]

Hi Akim,

In the first patch:
> +          for (bitset_windex windex = bitno / BITSET_WORD_BITS;
> +               (windex - woffset) < EBITSET_ELT_WORDS; windex++)

This is C99 syntax. If you don't add 'c99' as a dependency of your
module, the module will not compile with GCC versions < 5.1.0 with
their default settings.

Bruno

Re: undefined behaviour findings in bitset

[Akim Demaille]

> Le 16 mars 2019 à 20:53, Bruno Haible <bruno@clisp.org> a écrit :
> 
> Hi Akim,
> 
> In the first patch:
>> +          for (bitset_windex windex = bitno / BITSET_WORD_BITS;
>> +               (windex - woffset) < EBITSET_ELT_WORDS; windex++)
> 
> This is C99 syntax. If you don't add 'c99' as a dependency of your
> module, the module will not compile with GCC versions < 5.1.0 with
> their default settings.

hi Bruno,

bitset is already implemented in C99, it's not a new requirement.
I was unaware of the c99 module.  I installed the following commit
in addition to the ones I submitted.

Cheers!

commit 4d9813bf6bbf6489c1de4ad9d48943b961976bce
Author: Akim Demaille <akim.demaille@gmail.com>
Date:   Sun Mar 17 08:34:22 2019 +0100

    bitset, timevar: Depend on c99
    
    Reported by Bruno Haible.
    * modules/bitset, modules/timevar (Depends-on): Add c99.

diff --git a/ChangeLog b/ChangeLog
index 97b20b909..9ce4bf24c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2019-03-17  Akim Demaille  <akim@lrde.epita.fr>
+
+	bitset, timevar: Depend on c99.
+	Reported by Bruno Haible.
+	* modules/bitset, modules/timevar (Depends-on): Add c99.
+
 2019-03-16  Akim Demaille  <akim@lrde.epita.fr>
 
 	bitset: a bit (...) more tests
diff --git a/modules/bitset b/modules/bitset
index 76d1a2cac..81b71c0c7 100644
--- a/modules/bitset
+++ b/modules/bitset
@@ -17,6 +17,7 @@ lib/bitset/vector.c
 lib/bitset/vector.h
 
 Depends-on:
+c99
 gettext-h
 obstack
 xalloc
diff --git a/modules/timevar b/modules/timevar
index 3e1eb48f7..ef86fedb2 100644
--- a/modules/timevar
+++ b/modules/timevar
@@ -6,6 +6,7 @@ lib/timevar.h
 lib/timevar.c
 
 Depends-on:
+c99
 gethrxtime
 getrusage
 gettext-h

Re: undefined behaviour findings in bitset

[Akim Demaille]

Le 17 mars 2019 à 08:39, Akim Demaille <akim@lrde.epita.fr> a écrit :
> 
> commit 4d9813bf6bbf6489c1de4ad9d48943b961976bce
> Author: Akim Demaille <akim.demaille@gmail.com>
> Date:   Sun Mar 17 08:34:22 2019 +0100
> 
>    bitset, timevar: Depend on c99
> 
>    Reported by Bruno Haible.
>    * modules/bitset, modules/timevar (Depends-on): Add c99.

Since that change, Bison's CI fails on GCC 4.7.  That's because the c99 module also plays tricks with the C++ compiler, which is now set to use -std=gnu++11 if supported.  But GCC 4.7 does not support [[noreturn]] yet, even in gnu++11 mode.

I need something like the following changes.

Wouldn't it be useful to have a file that defines macros such as GL_GCC_VERSION (that is defined on GCC only, not clang, nor icc), GL_ICC_VERSION, etc.?  Defined as integer versions easy to compare (say major * 1e6 + minor * 1e3 + patchlevel, or just major&minor if deemed sufficient).

commit 896548de03f6490b57e9aca1f3d4ac8efba0f085
Author: Akim Demaille <akim.demaille@gmail.com>
Date:   Sun Mar 17 19:27:20 2019 +0100

    _Noreturn: GCC 4.7 does not support [[noreturn]] in C++11 mode
    
    * lib/_Noreturn.h, m4/gnulib-common.m4: Don't use [[noreturn]] before
    GCC 4.8.

diff --git a/ChangeLog b/ChangeLog
index 9ce4bf24c..4735bf65b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2019-03-17  Akim Demaille  <akim@lrde.epita.fr>
+
+	_Noreturn: GCC 4.7 does not support [[noreturn]] in C++11 mode
+	* lib/_Noreturn.h, m4/gnulib-common.m4: Don't use [[noreturn]] before
+	GCC 4.8.
+
 2019-03-17  Akim Demaille  <akim@lrde.epita.fr>
 
 	bitset, timevar: Depend on c99.
diff --git a/lib/_Noreturn.h b/lib/_Noreturn.h
index 94fdfaf02..ed8f6dbbf 100644
--- a/lib/_Noreturn.h
+++ b/lib/_Noreturn.h
@@ -1,5 +1,6 @@
 #ifndef _Noreturn
-# if 201103 <= (defined __cplusplus ? __cplusplus : 0)
+# if (201103 <= (defined __cplusplus ? __cplusplus : 0) \
+      && 4 < __GNUC__ + (8 <= __GNUC_MINOR__))
 #  define _Noreturn [[noreturn]]
 # elif (201112 <= (defined __STDC_VERSION__ ? __STDC_VERSION__ : 0) \
         || 4 < __GNUC__ + (7 <= __GNUC_MINOR__))
diff --git a/m4/gnulib-common.m4 b/m4/gnulib-common.m4
index 7c0e3e8fa..378470a36 100644
--- a/m4/gnulib-common.m4
+++ b/m4/gnulib-common.m4
@@ -1,4 +1,4 @@
-# gnulib-common.m4 serial 41
+# gnulib-common.m4 serial 42
 dnl Copyright (C) 2007-2019 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -17,7 +17,8 @@ AC_DEFUN([gl_COMMON_BODY], [
   AH_VERBATIM([_Noreturn],
 [/* The _Noreturn keyword of C11.  */
 #ifndef _Noreturn
-# if 201103 <= (defined __cplusplus ? __cplusplus : 0)
+# if (201103 <= (defined __cplusplus ? __cplusplus : 0) \
+      && 4 < __GNUC__ + (8 <= __GNUC_MINOR__))
 #  define _Noreturn [[noreturn]]
 # elif (201112 <= (defined __STDC_VERSION__ ? __STDC_VERSION__ : 0) \
         || 4 < __GNUC__ + (7 <= __GNUC_MINOR__))

Re: undefined behaviour findings in bitset

[Paul Eggert]

Akim Demaille wrote:
> Wouldn't it be useful to have a file that defines macros such as GL_GCC_VERSION

libc-config.h defines __GNUC_PREREQ, which is the most common spelling for such 
a macro. It's in the libc-config module. This was originally designed for code 
shared with glibc but can be used elsewhere.

Re: undefined behaviour findings in bitset

[Bruno Haible]

Hi Akim,

> But GCC 4.7 does not support [[noreturn]] yet, even in gnu++11 mode.
> 
> I need something like the following changes.

> -# if 201103 <= (defined __cplusplus ? __cplusplus : 0)
> +# if (201103 <= (defined __cplusplus ? __cplusplus : 0) \
> +      && 4 < __GNUC__ + (8 <= __GNUC_MINOR__))

Something like this is OK. But this change drops support for [[noreturn]]
in compilers other than GCC and clang. Shouldn't it be like this:

   # if (201103 <= (defined __cplusplus ? __cplusplus : 0) \
         && (!defined __GNUC__ || 4 < __GNUC__ + (8 <= __GNUC_MINOR__)))

> Wouldn't it be useful to have a file that defines macros such as
> GL_GCC_VERSION (that is defined on GCC only, not clang, nor icc),
> GL_ICC_VERSION, etc.?

The effort to distribute (in a module) and include this file may well
be larger than the gain, no?

Also, in this case the code duplication is between a file in m4/ and
a file in lib/. However, we don't have a good way to share code between
m4/ and lib/, because an include statement in a .m4 file
  #include "$srcdir/lib/foo.h"
would need to reference the gnulib-tool --source-base value instead of 'lib'.

Bruno

Re: undefined behaviour findings in bitset

[Akim Demaille]

[-- Attachment #1: Type: text/html, Size: 3131 bytes --]

Re: undefined behaviour findings in bitset

[Bruno Haible]

Hi Akim,

> Also, the relationship with noreturn.h is not completely clear for
> me either.

There are a couple of comment in noreturn.h lines 33..41. But I agree,
some text in the documentation would be better.

> Clang 7 pretends to be GCC 4.2 (__GNUC__ __GNUC_MINOR__).

> For instance I see it already has the above fix for
> GCC 4.7, but in a different way.
> 
>  /* Use ISO C++11 syntax when the compiler supports it. */
> # if (__cplusplus >= 201103 && !(__GNUC__ == 4 && __GNUC_MINOR__ == 7)) \
>   || (_MSC_VER >= 1900)
> # define _GL_NORETURN_FUNC [[noreturn]]

Right, this snippet gets clang++ and MSVC++ support right. How about this
patch?


2019-03-18  Bruno Haible  <bruno@clisp.org>

	_Noreturn: clang and MSVC do support [[noreturn]] in C++11 mode.
	* lib/_Noreturn.h: Use [[noreturn]] if __GNUC__ and __GNUC_MINOR__
	indicate clang, or if _MSC_VER indicates MSVC++ 14.0 or newer.

diff --git a/lib/_Noreturn.h b/lib/_Noreturn.h
index 19597e0..1629cef 100644
--- a/lib/_Noreturn.h
+++ b/lib/_Noreturn.h
@@ -1,6 +1,7 @@
 #ifndef _Noreturn
-# if (201103 <= (defined __cplusplus ? __cplusplus : 0) \
-      && (!defined __GNUC__ || 4 < __GNUC__ + (8 <= __GNUC_MINOR__)))
+# if (defined __cplusplus \
+      && ((201103 <= __cplusplus && !(__GNUC__ == 4 && __GNUC_MINOR__ == 7)) \
+          || (defined _MSC_VER && 1900 <= _MSC_VER)))
 #  define _Noreturn [[noreturn]]
 # elif (201112 <= (defined __STDC_VERSION__ ? __STDC_VERSION__ : 0) \
         || 4 < __GNUC__ + (7 <= __GNUC_MINOR__))
diff --git a/m4/gnulib-common.m4 b/m4/gnulib-common.m4
index 688a1e5..20666a5 100644
--- a/m4/gnulib-common.m4
+++ b/m4/gnulib-common.m4
@@ -1,4 +1,4 @@
-# gnulib-common.m4 serial 42
+# gnulib-common.m4 serial 43
 dnl Copyright (C) 2007-2019 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -17,8 +17,9 @@ AC_DEFUN([gl_COMMON_BODY], [
   AH_VERBATIM([_Noreturn],
 [/* The _Noreturn keyword of C11.  */
 #ifndef _Noreturn
-# if (201103 <= (defined __cplusplus ? __cplusplus : 0) \
-      && (!defined __GNUC__ || 4 < __GNUC__ + (8 <= __GNUC_MINOR__)))
+# if (defined __cplusplus \
+      && ((201103 <= __cplusplus && !(__GNUC__ == 4 && __GNUC_MINOR__ == 7)) \
+          || (defined _MSC_VER && 1900 <= _MSC_VER)))
 #  define _Noreturn [[noreturn]]
 # elif (201112 <= (defined __STDC_VERSION__ ? __STDC_VERSION__ : 0) \
         || 4 < __GNUC__ + (7 <= __GNUC_MINOR__))

Re: undefined behaviour findings in bitset

[Akim Demaille]

Hi Bruno,

> Le 18 mars 2019 à 22:03, Bruno Haible <bruno@clisp.org> a écrit :
> 
> Hi Akim,
> 
>> Also, the relationship with noreturn.h is not completely clear for
>> me either.
> 
> There are a couple of comment in noreturn.h lines 33..41. But I agree,
> some text in the documentation would be better.
> 
>> Clang 7 pretends to be GCC 4.2 (__GNUC__ __GNUC_MINOR__).
> 
>> For instance I see it already has the above fix for
>> GCC 4.7, but in a different way.
>> 
>> /* Use ISO C++11 syntax when the compiler supports it. */
>> # if (__cplusplus >= 201103 && !(__GNUC__ == 4 && __GNUC_MINOR__ == 7)) \
>>  || (_MSC_VER >= 1900)
>> # define _GL_NORETURN_FUNC [[noreturn]]
> 
> Right, this snippet gets clang++ and MSVC++ support right. How about this
> patch?

Yes, I agree this is the best option so far, thanks!

Re: undefined behaviour findings in bitset

[Bruno Haible]

Akim Demaille wrote:
> Also, the relationship with noreturn.h is not completely clear for
> me either.

I'm adding this documentation:


2019-03-19  Bruno Haible  <bruno@clisp.org>

	doc: Document the 'stdnoreturn' and 'noreturn' modules.
	Reported by Akim Demaille.
	* doc/noreturn.texi: New file.
	* doc/gnulib.texi: Include it.

diff --git a/doc/gnulib.texi b/doc/gnulib.texi
index ac3d570..838895c 100644
--- a/doc/gnulib.texi
+++ b/doc/gnulib.texi
@@ -6372,6 +6372,7 @@ to POSIX that it can be treated like any other Unix-like platform.
 * alloca-opt::
 * Safe Allocation Macros::
 * Compile-time Assertions::
+* Non-returning Functions::
 * Integer Properties::
 * static inline::
 * extern inline::
@@ -6402,6 +6403,8 @@ to POSIX that it can be treated like any other Unix-like platform.
 
 @include verify.texi
 
+@include noreturn.texi
+
 @include intprops.texi
 
 @include static-inline.texi
diff --git a/doc/noreturn.texi b/doc/noreturn.texi
new file mode 100644
index 0000000..862a409
--- /dev/null
+++ b/doc/noreturn.texi
@@ -0,0 +1,60 @@
+@c GNU noreturn, stdnoreturn modules documentation
+
+@c Copyright (C) 2019 Free Software Foundation, Inc.
+
+@c Permission is granted to copy, distribute and/or modify this document
+@c under the terms of the GNU Free Documentation License, Version 1.3
+@c or any later version published by the Free Software Foundation;
+@c with no Invariant Sections, no Front-Cover Texts, and no Back-Cover
+@c Texts.  A copy of the license is included in the ``GNU Free
+@c Documentation License'' file as part of this distribution.
+
+@node Non-returning Functions
+@section Non-returning Functions
+
+@cindex @code{_Noreturn}
+@cindex @code{noreturn}
+@cindex @code{stdnoreturn}
+A "non-returning" function is a function which cannot return normally.
+It can transfer control only through @code{longjmp()}, @code{throw}
+(in C++), or similar mechanisms.  The most prominent function of this
+class is the @code{abort} function.  Non-returning functions are
+declared with a @code{void} return type.
+
+It helps the compiler's ability to emit sensible warnings, following
+data-flow analysis, to declare which functions are non-returning.
+
+Gnulib has two modules that support such a declaration:
+
+@itemize @bullet
+@item
+The @samp{stdnoreturn} module.  It provides a way to put this
+declaration at function declarations and function definitions, but not
+in function pointer types.  The identifier to use is @code{_Noreturn}
+or @code{noreturn}; @code{_Noreturn} is to be preferred because
+@code{noreturn} is a no-op on some platforms.  The include file is
+@code{<stdnoreturn.h>}.
+
+@item
+The @samp{noreturn} module.  It provides a way to put this declaration
+at function declarations, at function definitions, and in function
+pointer types.  The identifiers to use are:
+@itemize -
+@item
+@code{_GL_NORETURN_FUNC} for use in function declarations and function
+definitions.
+@item
+@code{_GL_NORETURN_FUNCPTR} for use on function pointers.
+@end itemize
+@noindent
+The include file is @code{<noreturn.h>}.
+@end itemize
+
+Which of the two modules to use?  If the non-returning functions you
+have to declare are unlikely to be accessed through function pointers,
+you should use module @code{stdnoreturn}; otherwise the module
+@code{noreturn} provides for better data-flow analysis and thus for
+better warnings.
+
+For a detailed description of the @code{stdnoreturn} module, see
+@ref{stdnoreturn.h}.

Re: undefined behaviour findings in bitset

[Akim Demaille]

Hi Bruno,

> Le 20 mars 2019 à 04:39, Bruno Haible <bruno@clisp.org> a écrit :
> 
> Akim Demaille wrote:
>> Also, the relationship with noreturn.h is not completely clear for
>> me either.
> 
> I'm adding this documentation:
> 
> 2019-03-19  Bruno Haible  <bruno@clisp.org>
> 
> 	doc: Document the 'stdnoreturn' and 'noreturn' modules.
> 	Reported by Akim Demaille.
> 	* doc/noreturn.texi: New file.
> 	* doc/gnulib.texi: Include it.

Thanks!

Re: undefined behaviour findings in bitset

[Akim Demaille]

Hi Bruno,

> Le 19 mars 2019 à 07:07, Akim Demaille <akim@lrde.epita.fr> a écrit :
> 
> Hi Bruno,
> 
>> Le 18 mars 2019 à 22:03, Bruno Haible <bruno@clisp.org> a écrit :
>> 
>> Hi Akim,
>> 
>>> Also, the relationship with noreturn.h is not completely clear for
>>> me either.
>> 
>> There are a couple of comment in noreturn.h lines 33..41. But I agree,
>> some text in the documentation would be better.
>> 
>>> Clang 7 pretends to be GCC 4.2 (__GNUC__ __GNUC_MINOR__).
>> 
>>> For instance I see it already has the above fix for
>>> GCC 4.7, but in a different way.
>>> 
>>> /* Use ISO C++11 syntax when the compiler supports it. */
>>> # if (__cplusplus >= 201103 && !(__GNUC__ == 4 && __GNUC_MINOR__ == 7)) \
>>> || (_MSC_VER >= 1900)
>>> # define _GL_NORETURN_FUNC [[noreturn]]
>> 
>> Right, this snippet gets clang++ and MSVC++ support right. How about this
>> patch?
> 
> Yes, I agree this is the best option so far, thanks!

Well, it does not work with G++ in C++98 mode: _Noreturn is not supported there.

This works with Bison's CI:

commit b2a7dec46c0b702b5f7618994641e7381c8ff86f
Author: Akim Demaille <akim.demaille@gmail.com>
Date:   Fri Mar 22 08:25:53 2019 +0100

  _Noreturn: beware of C's _Noreturn in C++ pre C++11

  * lib/_Noreturn.h, m4/gnulib-common.m4: Using C's _Noreturn in
  C++98 appears to be supported by Clang, but not by GCC nor ICC.

diff --git a/ChangeLog b/ChangeLog
index 62c522e65..b694c3512 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2019-03-22  Akim Demaille  <akim@lrde.epita.fr>
+
+	_Noreturn: beware of C's _Noreturn in C++ pre C++11.
+	* lib/_Noreturn.h, m4/gnulib-common.m4: Using C's _Noreturn in
+	C++98 appears to be supported by Clang, but not by GCC nor ICC.
+
2019-03-19  Akim Demaille  <akim@lrde.epita.fr>

	bitset: fix memory leaks
diff --git a/lib/_Noreturn.h b/lib/_Noreturn.h
index 1629cef39..7594e4b0c 100644
--- a/lib/_Noreturn.h
+++ b/lib/_Noreturn.h
@@ -3,8 +3,9 @@
     && ((201103 <= __cplusplus && !(__GNUC__ == 4 && __GNUC_MINOR__ == 7)) \
         || (defined _MSC_VER && 1900 <= _MSC_VER)))
#  define _Noreturn [[noreturn]]
-# elif (201112 <= (defined __STDC_VERSION__ ? __STDC_VERSION__ : 0) \
-        || 4 < __GNUC__ + (7 <= __GNUC_MINOR__))
+# elif ((!defined __cplusplus || defined __clang__)                     \
+        && (201112 <= (defined __STDC_VERSION__ ? __STDC_VERSION__ : 0)  \
+            || 4 < __GNUC__ + (7 <= __GNUC_MINOR__)))
  /* _Noreturn works as-is.  */
# elif 2 < __GNUC__ + (8 <= __GNUC_MINOR__) || 0x5110 <= __SUNPRO_C
#  define _Noreturn __attribute__ ((__noreturn__))
diff --git a/m4/gnulib-common.m4 b/m4/gnulib-common.m4
index 8ad963e35..57b94ed53 100644
--- a/m4/gnulib-common.m4
+++ b/m4/gnulib-common.m4
@@ -21,8 +21,9 @@ AC_DEFUN([gl_COMMON_BODY], [
     && ((201103 <= __cplusplus && !(__GNUC__ == 4 && __GNUC_MINOR__ == 7)) \
         || (defined _MSC_VER && 1900 <= _MSC_VER)))
#  define _Noreturn [[noreturn]]
-# elif (201112 <= (defined __STDC_VERSION__ ? __STDC_VERSION__ : 0) \
-        || 4 < __GNUC__ + (7 <= __GNUC_MINOR__))
+# elif ((!defined __cplusplus || defined __clang__) \
+        && (201112 <= (defined __STDC_VERSION__ ? __STDC_VERSION__ : 0)  \
+            || 4 < __GNUC__ + (7 <= __GNUC_MINOR__)))
  /* _Noreturn works as-is.  */
# elif 2 < __GNUC__ + (8 <= __GNUC_MINOR__) || 0x5110 <= __SUNPRO_C
#  define _Noreturn __attribute__ ((__noreturn__))

Re: undefined behaviour findings in bitset

[Bruno Haible]

Hi Akim,

> it does not work with G++ in C++98 mode: _Noreturn is not supported there.

I confirm: No g++ version I tested supported _Noreturn, neither with
-std=c++98 nor with -std=c++11.

Your patch looks good.

> Using C's _Noreturn in C++98 appears to be supported by Clang

Good point! Let me adjust the 'noreturn' module accordingly:


2019-03-23  Bruno Haible  <bruno@clisp.org>

	noreturn: In C++ mode with clang, use _Noreturn as fallback.
	Reported by Akim Demaille.
	* lib/noreturn.h (_GL_NORETURN_FUNC): In C++ mode with clang, when
	[[noreturn]] would not work, use _Noreturn instead.

diff --git a/lib/noreturn.h b/lib/noreturn.h
index 3ceeaf5..57e9a88 100644
--- a/lib/noreturn.h
+++ b/lib/noreturn.h
@@ -74,6 +74,9 @@
 # if (__cplusplus >= 201103 && !(__GNUC__ == 4 && __GNUC_MINOR__ == 7)) \
      || (_MSC_VER >= 1900)
 #  define _GL_NORETURN_FUNC [[noreturn]]
+  /* clang++ supports the _Noreturn keyword, but g++ doesn't.  */
+# elif defined __clang__
+#  define _GL_NORETURN_FUNC _Noreturn
 # else
 #  define _GL_NORETURN_FUNC /* empty */
 # endif