This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Avoid invoking undefined behavior when initializing CRC table
- From: Patrick Palka <patrick at parcs dot ath dot cx>
- To: Patrick Palka <patrick at parcs dot ath dot cx>
- Cc: gdb-patches at sourceware dot org
- Date: Sun, 27 Dec 2015 23:15:22 -0500 (EST)
- Subject: Re: [PATCH] Avoid invoking undefined behavior when initializing CRC table
- Authentication-results: sourceware.org; auth=none
- References: <1451275797-648-1-git-send-email-patrick at parcs dot ath dot cx>
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? 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:
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 70acafc..0e3ac4e 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1911,11 +1911,6 @@ handle_qxfer (char *own_buf, int packet_len, int *new_packet_len_p)
return 0;
}
-/* Table used by the crc32 function to calcuate the checksum. */
-
-static unsigned int crc32_table[256] =
-{0, 0};
-
/* Compute 32 bit CRC from inferior memory.
On success, return 32 bit CRC.
@@ -1924,20 +1919,6 @@ static unsigned int crc32_table[256] =
static unsigned long long
crc32 (CORE_ADDR base, int len, unsigned int crc)
{
- if (!crc32_table[1])
- {
- /* Initialize the CRC table and the decoding table. */
- unsigned int i, j;
- unsigned int c;
-
- for (i = 0; i < 256; i++)
- {
- for (c = i << 24, j = 8; j > 0; --j)
- c = c & 0x80000000 ? (c << 1) ^ 0x04c11db7 : (c << 1);
- crc32_table[i] = c;
- }
- }
-
while (len--)
{
unsigned char byte = 0;
@@ -1946,7 +1927,7 @@ crc32 (CORE_ADDR base, int len, unsigned int crc)
if (read_inferior_memory (base, &byte, 1) != 0)
return (unsigned long long) -1;
- crc = (crc << 8) ^ crc32_table[((crc >> 24) ^ byte) & 255];
+ crc = xcrc32 (&byte, 1, crc);
base++;
}
return (unsigned long long) crc;