* undefined behaviour findings @ 2019-03-09 19:33 Bruno Haible 2019-03-09 19:37 ` undefined behaviour findings in bitset Bruno Haible 2019-03-09 21:22 ` undefined behaviour findings Bruno Haible 0 siblings, 2 replies; 17+ messages in thread From: Bruno Haible @ 2019-03-09 19:33 UTC (permalink / raw) To: bug-gnulib; +Cc: Jeffrey Walton 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: undefined behaviour findings in bitset 2019-03-09 19:33 undefined behaviour findings Bruno Haible @ 2019-03-09 19:37 ` Bruno Haible 2019-03-16 16:38 ` Akim Demaille 2019-03-09 21:22 ` undefined behaviour findings Bruno Haible 1 sibling, 1 reply; 17+ messages in thread From: Bruno Haible @ 2019-03-09 19:37 UTC (permalink / raw) To: Akim Demaille; +Cc: bug-gnulib 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: undefined behaviour findings in bitset 2019-03-09 19:37 ` undefined behaviour findings in bitset Bruno Haible @ 2019-03-16 16:38 ` Akim Demaille 2019-03-16 16:42 ` Akim Demaille 2019-03-16 19:53 ` Bruno Haible 0 siblings, 2 replies; 17+ messages in thread From: Akim Demaille @ 2019-03-16 16:38 UTC (permalink / raw) To: Bruno Haible; +Cc: bug-gnulib 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 }; ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: undefined behaviour findings in bitset 2019-03-16 16:38 ` Akim Demaille @ 2019-03-16 16:42 ` Akim Demaille 2019-03-16 19:53 ` Bruno Haible 1 sibling, 0 replies; 17+ messages in thread From: Akim Demaille @ 2019-03-16 16:42 UTC (permalink / raw) To: Bruno Haible; +Cc: bug-gnulib > 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: undefined behaviour findings in bitset 2019-03-16 16:38 ` Akim Demaille 2019-03-16 16:42 ` Akim Demaille @ 2019-03-16 19:53 ` Bruno Haible 2019-03-17 7:39 ` Akim Demaille 1 sibling, 1 reply; 17+ messages in thread From: Bruno Haible @ 2019-03-16 19:53 UTC (permalink / raw) To: Akim Demaille; +Cc: bug-gnulib 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: undefined behaviour findings in bitset 2019-03-16 19:53 ` Bruno Haible @ 2019-03-17 7:39 ` Akim Demaille 2019-03-17 18:36 ` Akim Demaille 0 siblings, 1 reply; 17+ messages in thread From: Akim Demaille @ 2019-03-17 7:39 UTC (permalink / raw) To: Bruno Haible; +Cc: bug-gnulib > 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: undefined behaviour findings in bitset 2019-03-17 7:39 ` Akim Demaille @ 2019-03-17 18:36 ` Akim Demaille 2019-03-17 18:56 ` Paul Eggert 2019-03-17 19:27 ` Bruno Haible 0 siblings, 2 replies; 17+ messages in thread From: Akim Demaille @ 2019-03-17 18:36 UTC (permalink / raw) To: Bruno Haible; +Cc: bug-gnulib 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__)) ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: undefined behaviour findings in bitset 2019-03-17 18:36 ` Akim Demaille @ 2019-03-17 18:56 ` Paul Eggert 2019-03-17 19:27 ` Bruno Haible 1 sibling, 0 replies; 17+ messages in thread From: Paul Eggert @ 2019-03-17 18:56 UTC (permalink / raw) To: Akim Demaille, Bruno Haible; +Cc: bug-gnulib 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: undefined behaviour findings in bitset 2019-03-17 18:36 ` Akim Demaille 2019-03-17 18:56 ` Paul Eggert @ 2019-03-17 19:27 ` Bruno Haible 2019-03-18 8:16 ` Akim Demaille 1 sibling, 1 reply; 17+ messages in thread From: Bruno Haible @ 2019-03-17 19:27 UTC (permalink / raw) To: Akim Demaille; +Cc: bug-gnulib 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: undefined behaviour findings in bitset 2019-03-17 19:27 ` Bruno Haible @ 2019-03-18 8:16 ` Akim Demaille 2019-03-18 21:03 ` Bruno Haible 2019-03-20 3:39 ` Bruno Haible 0 siblings, 2 replies; 17+ messages in thread From: Akim Demaille @ 2019-03-18 8:16 UTC (permalink / raw) To: Bruno Haible; +Cc: bug-gnulib [-- Attachment #1: Type: text/html, Size: 3131 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: undefined behaviour findings in bitset 2019-03-18 8:16 ` Akim Demaille @ 2019-03-18 21:03 ` Bruno Haible 2019-03-19 6:07 ` Akim Demaille 2019-03-20 3:39 ` Bruno Haible 1 sibling, 1 reply; 17+ messages in thread From: Bruno Haible @ 2019-03-18 21:03 UTC (permalink / raw) To: Akim Demaille; +Cc: bug-gnulib 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__)) ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: undefined behaviour findings in bitset 2019-03-18 21:03 ` Bruno Haible @ 2019-03-19 6:07 ` Akim Demaille 2019-03-22 16:00 ` Akim Demaille 0 siblings, 1 reply; 17+ messages in thread From: Akim Demaille @ 2019-03-19 6:07 UTC (permalink / raw) To: Bruno Haible; +Cc: bug-gnulib 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! ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: undefined behaviour findings in bitset 2019-03-19 6:07 ` Akim Demaille @ 2019-03-22 16:00 ` Akim Demaille 2019-03-23 18:54 ` Bruno Haible 0 siblings, 1 reply; 17+ messages in thread From: Akim Demaille @ 2019-03-22 16:00 UTC (permalink / raw) To: Bruno Haible; +Cc: bug-gnulib 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__)) ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: undefined behaviour findings in bitset 2019-03-22 16:00 ` Akim Demaille @ 2019-03-23 18:54 ` Bruno Haible 0 siblings, 0 replies; 17+ messages in thread From: Bruno Haible @ 2019-03-23 18:54 UTC (permalink / raw) To: Akim Demaille; +Cc: bug-gnulib 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: undefined behaviour findings in bitset 2019-03-18 8:16 ` Akim Demaille 2019-03-18 21:03 ` Bruno Haible @ 2019-03-20 3:39 ` Bruno Haible 2019-03-20 17:04 ` Akim Demaille 1 sibling, 1 reply; 17+ messages in thread From: Bruno Haible @ 2019-03-20 3:39 UTC (permalink / raw) To: Akim Demaille; +Cc: bug-gnulib 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}. ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: undefined behaviour findings in bitset 2019-03-20 3:39 ` Bruno Haible @ 2019-03-20 17:04 ` Akim Demaille 0 siblings, 0 replies; 17+ messages in thread From: Akim Demaille @ 2019-03-20 17:04 UTC (permalink / raw) To: Bruno Haible; +Cc: bug-gnulib 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! ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: undefined behaviour findings 2019-03-09 19:33 undefined behaviour findings Bruno Haible 2019-03-09 19:37 ` undefined behaviour findings in bitset Bruno Haible @ 2019-03-09 21:22 ` Bruno Haible 1 sibling, 0 replies; 17+ messages in thread From: Bruno Haible @ 2019-03-09 21:22 UTC (permalink / raw) To: bug-gnulib; +Cc: Jeffrey Walton > 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; ^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2019-03-23 18:54 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-09 19:33 undefined behaviour findings Bruno Haible 2019-03-09 19:37 ` undefined behaviour findings in bitset Bruno Haible 2019-03-16 16:38 ` Akim Demaille 2019-03-16 16:42 ` Akim Demaille 2019-03-16 19:53 ` Bruno Haible 2019-03-17 7:39 ` Akim Demaille 2019-03-17 18:36 ` Akim Demaille 2019-03-17 18:56 ` Paul Eggert 2019-03-17 19:27 ` Bruno Haible 2019-03-18 8:16 ` Akim Demaille 2019-03-18 21:03 ` Bruno Haible 2019-03-19 6:07 ` Akim Demaille 2019-03-22 16:00 ` Akim Demaille 2019-03-23 18:54 ` Bruno Haible 2019-03-20 3:39 ` Bruno Haible 2019-03-20 17:04 ` Akim Demaille 2019-03-09 21:22 ` undefined behaviour findings Bruno Haible
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).