From patchwork Mon Dec 28 04:15:22 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Patrick Palka X-Patchwork-Id: 10151 Received: (qmail 35640 invoked by alias); 28 Dec 2015 04:15:34 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 35598 invoked by uid 89); 28 Dec 2015 04:15:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy=Compute X-HELO: mail-qk0-f181.google.com Received: from mail-qk0-f181.google.com (HELO mail-qk0-f181.google.com) (209.85.220.181) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 28 Dec 2015 04:15:29 +0000 Received: by mail-qk0-f181.google.com with SMTP id q19so16791612qke.3 for ; Sun, 27 Dec 2015 20:15:29 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:date:to:cc:subject:in-reply-to:message-id :references:user-agent:mime-version:content-type; bh=ck6EJGtJG0pQsObAzIaC85p/uhrWuh3K1TnTMb2Tduc=; b=JhJvTAE5oRbPmbyPwdrR29d/RDo96Ngn7F5cKj9Y3J1blZr4xu13PyZQeUfnoT8Di+ kUG4Kv6xBxn2Tw41q+NcyCodkJu0g6MzFXGH19P99cVP06JhoRkrnJuyVVrMnpiQr58I /OPHpLgpLePbt6DgB78hSLvjaUFtiK4bCyq7M1W6ixiRblLnsjZP2eyODm2n4Xj/d2p5 y6uH28ZwF8xrb0jpTM+03PLawBkVSICPLUYd5fkYQyaugIdKldxGvhUmpyZknkj1XfFf CGHPhxUdm95Pxh17ERoea8kfoAdbrJ3UWA+EONBoPWUinV02LPABbXn4hQjYD/aGcqYI xDRQ== X-Gm-Message-State: ALoCoQlBOkv4/jpffTHn3f3AFyl2BfaDOwtfb/Nj0v/Yb4YylOa/lw4pIX7CdTVkCD2vCdcu+KNmXGgcA4HccEvVpJdq1tCN1g== X-Received: by 10.55.51.203 with SMTP id z194mr68486961qkz.21.1451276127350; Sun, 27 Dec 2015 20:15:27 -0800 (PST) Received: from [192.168.1.130] (ool-4353a8be.dyn.optonline.net. [67.83.168.190]) by smtp.gmail.com with ESMTPSA id m184sm2740326qhc.45.2015.12.27.20.15.26 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 27 Dec 2015 20:15:26 -0800 (PST) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Sun, 27 Dec 2015 23:15:22 -0500 (EST) To: Patrick Palka cc: gdb-patches@sourceware.org Subject: Re: [PATCH] Avoid invoking undefined behavior when initializing CRC table In-Reply-To: <1451275797-648-1-git-send-email-patrick@parcs.ath.cx> Message-ID: References: <1451275797-648-1-git-send-email-patrick@parcs.ath.cx> User-Agent: Alpine 2.20.9 (DEB 106 2015-09-22) MIME-Version: 1.0 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;