diff mbox

Fix regcomp.c build for 32-bit with GCC mainline

Message ID alpine.DEB.2.20.1711201903170.13739@digraph.polyomino.org.uk
State New
Headers show

Commit Message

Joseph Myers Nov. 20, 2017, 7:03 p.m. UTC
posix/regcomp.c fails to build for 32-bit configurations with GCC
mainline:

In file included from regex.c:66:
regcomp.c: In function 'init_word_char':
regcomp.c:934:24: error: conversion from 'long long unsigned int' to 'bitset_word_t {aka long unsigned int}' changes value from '287948901175001088' to '0' [-Werror=overflow]
    dfa->word_char[0] = wc0;
                        ^~~
regcomp.c:935:24: error: conversion from 'long long unsigned int' to 'bitset_word_t {aka long unsigned int}' changes value from '576460745995190270' to '2281701374' [-Werror=overflow]
    dfa->word_char[1] = wc1;
                        ^~~

The code has a workaround using const temporary variables instead of
having constants directly on the RHS of the assignments; this
workaround no longer suffices to avoid the warning.  This patch
replaces the workaround by a different one, using conditional
expressions that evaluate to 0, in the case where this code is inside
an "if" whose condition evaluates to 0.

Tested compilation for arm with GCC mainline with build-many-glibcs.py
(where it allows the compilation to get further, next running into
-Werror=format-overflow= failures in res_debug.c, for which I've filed
bug 22463 as there seems to be a potentially user-visible bug there);
also tested for 32-bit x86 with an older compiler to make sure no
execution failures result from this patch.

2017-11-20  Joseph Myers  <joseph@codesourcery.com>

	* posix/regcomp.c (init_word_char): Use conditional expressions
	not intermediate const variables to avoid -Werror=overflow errors
	for 32-bit systems in 64-bit case.

Comments

Adhemerval Zanella Nov. 20, 2017, 8:05 p.m. UTC | #1
On 20/11/2017 17:03, Joseph Myers wrote:
> posix/regcomp.c fails to build for 32-bit configurations with GCC
> mainline:
> 
> In file included from regex.c:66:
> regcomp.c: In function 'init_word_char':
> regcomp.c:934:24: error: conversion from 'long long unsigned int' to 'bitset_word_t {aka long unsigned int}' changes value from '287948901175001088' to '0' [-Werror=overflow]
>     dfa->word_char[0] = wc0;
>                         ^~~
> regcomp.c:935:24: error: conversion from 'long long unsigned int' to 'bitset_word_t {aka long unsigned int}' changes value from '576460745995190270' to '2281701374' [-Werror=overflow]
>     dfa->word_char[1] = wc1;
>                         ^~~
> 
> The code has a workaround using const temporary variables instead of
> having constants directly on the RHS of the assignments; this
> workaround no longer suffices to avoid the warning.  This patch
> replaces the workaround by a different one, using conditional
> expressions that evaluate to 0, in the case where this code is inside
> an "if" whose condition evaluates to 0.
> 
> Tested compilation for arm with GCC mainline with build-many-glibcs.py
> (where it allows the compilation to get further, next running into
> -Werror=format-overflow= failures in res_debug.c, for which I've filed
> bug 22463 as there seems to be a potentially user-visible bug there);
> also tested for 32-bit x86 with an older compiler to make sure no
> execution failures result from this patch.
> 
> 2017-11-20  Joseph Myers  <joseph@codesourcery.com>
> 
> 	* posix/regcomp.c (init_word_char): Use conditional expressions
> 	not intermediate const variables to avoid -Werror=overflow errors
> 	for 32-bit systems in 64-bit case.
> 
> diff --git a/posix/regcomp.c b/posix/regcomp.c
> index 871ae2f..54573f7 100644
> --- a/posix/regcomp.c
> +++ b/posix/regcomp.c
> @@ -927,12 +927,14 @@ init_word_char (re_dfa_t *dfa)
>      {
>        if (sizeof (dfa->word_char[0]) == 8)
>  	{
> -          /* The extra temporaries here avoid "implicitly truncated"
> +          /* The conditionals here avoid "implicitly truncated"
>               warnings in the case when this is dead code, i.e. 32-bit.  */
> -          const uint64_t wc0 = UINT64_C (0x03ff000000000000);
> -          const uint64_t wc1 = UINT64_C (0x07fffffe87fffffe);
> -	  dfa->word_char[0] = wc0;
> -	  dfa->word_char[1] = wc1;
> +	  dfa->word_char[0] = (sizeof (dfa->word_char[0]) == 8
> +			       ? UINT64_C (0x03ff000000000000)
> +			       : 0);
> +	  dfa->word_char[1] = (sizeof (dfa->word_char[0]) == 8
> +			       ? UINT64_C (0x07fffffe87fffffe)
> +			       : 0);
>  	  i = 2;
>  	}
>        else if (sizeof (dfa->word_char[0]) == 4)
> 

Why not use the already upstream fix from gnulib 
(252b52457da7887667c036d18cc5169777615bb0) ?
Joseph Myers Nov. 20, 2017, 11:18 p.m. UTC | #2
On Mon, 20 Nov 2017, Adhemerval Zanella wrote:

> Why not use the already upstream fix from gnulib 
> (252b52457da7887667c036d18cc5169777615bb0) ?

That change would not apply cleanly to current sources, and if we have 
over five years of regex changes out of sync with gnulib, it doesn't seem 
a good idea to try to tie any possible resync with gnulib (in both 
directions) to fixing build failures to make it possible to detect build 
regressions when they occur (rather than piling one failure on top of 
another that isn't yet fixed, so making it hard to determine the 
problematic patch, which is the current state).
diff mbox

Patch

diff --git a/posix/regcomp.c b/posix/regcomp.c
index 871ae2f..54573f7 100644
--- a/posix/regcomp.c
+++ b/posix/regcomp.c
@@ -927,12 +927,14 @@  init_word_char (re_dfa_t *dfa)
     {
       if (sizeof (dfa->word_char[0]) == 8)
 	{
-          /* The extra temporaries here avoid "implicitly truncated"
+          /* The conditionals here avoid "implicitly truncated"
              warnings in the case when this is dead code, i.e. 32-bit.  */
-          const uint64_t wc0 = UINT64_C (0x03ff000000000000);
-          const uint64_t wc1 = UINT64_C (0x07fffffe87fffffe);
-	  dfa->word_char[0] = wc0;
-	  dfa->word_char[1] = wc1;
+	  dfa->word_char[0] = (sizeof (dfa->word_char[0]) == 8
+			       ? UINT64_C (0x03ff000000000000)
+			       : 0);
+	  dfa->word_char[1] = (sizeof (dfa->word_char[0]) == 8
+			       ? UINT64_C (0x07fffffe87fffffe)
+			       : 0);
 	  i = 2;
 	}
       else if (sizeof (dfa->word_char[0]) == 4)