[Bug c/94773] New: Unhelpful warning "right shift count >= width of type [-Wshift-count-overflow]" on unreachable code.

[nisse at lysator dot liu.se]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94773

            Bug ID: 94773
           Summary: Unhelpful warning "right shift count >= width of type
                    [-Wshift-count-overflow]" on unreachable code.
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: nisse at lysator dot liu.se
  Target Milestone: ---

Consider the following program:

---8<------
/* To avoid including limits.h */
#define CHAR_BIT 8 

unsigned 
shift(unsigned x)
{
  if (sizeof(unsigned) * CHAR_BIT > 32)
    return x >> 32;
  else
    return x >> 1;
}
---8<-------

I compile this on a debian gnu/linux machine, x86_64, where char is 8 bits and
int is 32 bits, and hence the if condition is false. I think the function shift
above is well defined C, even if it has a peculiar dependency on the
architecture's word size.

When compiled with -Wall, I get a warning message on the shift which is in an
unreachable branch of the code: 

$ gcc-10 --version
gcc-10 (Debian 10-20200418-1) 10.0.1 20200418 (experimental) [master revision
27c171775ab:4c277008be0:c5bac7d127f288fd2f8a1f15c3f30da5903141c6]
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ gcc-10 -O -Wall -c rshift-warning.c 
rshift-warning.c: In function ‘shift’:
rshift-warning.c:8:14: warning: right shift count >= width of type
[-Wshift-count-overflow]
    8 |     return x >> 32;
      |              ^~

I get the same warning (just different formatting) with gcc-8.3.0.

I consider the warning unhelpful, since the code is unreachable precisely
because the outer if statement determines that the shift isn't valid. I mean,
if I did 

  if (CHAR_BIT != 8) return x / (CHAR_BIT - 8);

I wouldn't want to get a warning that the division by the constant zero is
invalid. I agree the minimal example is a bit silly, so I can't argue strongly
that the warning is unhelpful based on just this example. 

The actual code where I encountered the problem was the umul_ppmm macro in
mini-gmp. It's defined at
https://gmplib.org/repo/gmp/file/f4c89b9840ba/mini-gmp/mini-gmp.c#l132 and the
first few lines look like this:

#define gmp_umul_ppmm(w1, w0, u, v)                                     \
  do {                                                                  \
    int LOCAL_GMP_LIMB_BITS = GMP_LIMB_BITS;                            \
    if (sizeof(unsigned int) * CHAR_BIT >= 2 * GMP_LIMB_BITS)           \
      {                                                                 \
        unsigned int __ww = (unsigned int) (u) * (v);                   \
        w0 = (mp_limb_t) __ww;                                          \
        w1 = (mp_limb_t) (__ww >> LOCAL_GMP_LIMB_BITS);                 \
      }                                                                 \

Gcc doesn't warn for this, but that's because there's a workaround, the
LOCAL_GMP_LIMB_BITS variable. For some reason that suppresses the warning. But
if I try to simplify this by deleting LOCAL_GMP_LIMB_BITS and instead use
GMP_LIMB_BITS in the shift expression, I get the same warning as in the minimal
example above, for every expansion of the macro. In my environment,
sizeof(unsigned int) == 4, CHAR_BIT == 8, GMP_LIMB_BITS == 64, and mp_limb_t is
an alias for unsigned long.

For context, it's possible to make this code reachable by changing the way
mini-gmp is compiled, e.g., making mp_limb_t an alias for unsigned char, and
defining GMP_LIMB_BITS == 8. The purpose of the if statement is to use portable
C to determine, at compile time, if unsigned int is large enough to be used to
temporarily hold a number of bit size 2*GMP_LIMB_BITS, in which case the
following code block is correct.

BTW, if the example is written using a conditional expression instead of a
conditional statement,

  unsigned 
  shift(unsigned x)
  {
    return (sizeof(unsigned) * CHAR_BIT > 32? x >> 32 : x >> 1);
  }

it does *not* trigger a warning (gcc-8.3 and gcc-10 behave the same). But that
kind of rewrite is not an option for the code I'm really interested in.