From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21054 invoked by alias); 28 Dec 2015 11:53:28 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 21038 invoked by uid 89); 28 Dec 2015 11:53:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=sk:compare, sk:native-, sk:native, HContent-Transfer-Encoding:8bit X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 28 Dec 2015 11:53:26 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 6EC54A04B3; Mon, 28 Dec 2015 11:53:25 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id tBSBrOe1006652; Mon, 28 Dec 2015 06:53:24 -0500 Message-ID: <568122B4.7030502@redhat.com> Date: Mon, 28 Dec 2015 11:53:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Patrick Palka CC: gdb-patches@sourceware.org Subject: Re: [PATCH] Avoid invoking undefined behavior when initializing CRC table References: <1451275797-648-1-git-send-email-patrick@parcs.ath.cx> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-SW-Source: 2015-12/txt/msg00518.txt.bz2 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