Re: [PATCH] Avoid invoking undefined behavior when initializing CRC table

[Pedro Alves <palves@redhat.com>]

On 12/28/2015 04:15 AM, Patrick Palka wrote:
> On Sun, 27 Dec 2015, Patrick Palka wrote:
> 
>> When I built GDB with (an older snapshot of) GCC 6 I get the following
>> error:
>>
>> .../binutils-gdb/gdb/gdbserver/server.c: In function ‘crc32’:
>> .../binutils-gdb/gdb/gdbserver/server.c:1895:15: error: iteration 128 invokes undefined behavior [-Werror=aggressive-loop-optimizations]
>>    for (c = i << 24, j = 8; j > 0; --j)
>>               ^
>> .../binutils-gdb/gdb/gdbserver/server.c:1893:7: note: within this loop
>>       for (i = 0; i < 256; i++)
>>       ^
>> This error seems to be correct.  When the variable "int i" is >= 128,
>> the computation "i << 24" overflows for 32-bit signed int.
>>
>> To avoid shifting into the sign bit, this patch makes the variables i
>> (and j, because why not) have type unsigned int instead.
>>
>> (Alternatively, I can just define this local crc32 function in terms of
>> libiberty's xcrc32.  Any reason not to?

Just history.  gdbserver only started linking with libiberty in
2014 (0b04e52316) and the gdbserver crc32 code predates that.

> xcrc32 seems to be
>> based off of GDB's crc32 implementation.  Its documentation even
>> refers to it!)
> 
> And here's a rough diff that defines crc32 in terms of xcrc32:
> 

That looks good.  Tom did the same on the GDB side in 85ec6ce7d5 (2013).

Just in case, please make sure testing against gdbserver doesn't regress:

  $ make check -j8 RUNTESTFLAGS="--target_board=native-gdbserver" FORCE_PARALLEL=1

particularly, the compare-sections command.  We have a test which covers it:

  $ make check RUNTESTFLAGS="--target_board=native-gdbserver compare-sections.exp"

Thanks,
Pedro Alves