| Message ID | alpine.DEB.2.00.1610150200220.31859@tp.orcam.me.uk |
|---|---|
| State | Committed |
| Headers |
Received: (qmail 7836 invoked by alias); 17 Oct 2016 15:18:20 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 7809 invoked by uid 89); 17 Oct 2016 15:18:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=4.4.7, *operands, auxiliary X-HELO: mailapp01.imgtec.com Received: from mailapp02.imgtec.com (HELO mailapp01.imgtec.com) (217.156.133.132) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 17 Oct 2016 15:18:18 +0000 Received: from HHMAIL03.hh.imgtec.org (unknown [10.44.0.21]) by Forcepoint Email with ESMTPS id 2F62EDE82B8BA for <gdb-patches@sourceware.org>; Mon, 17 Oct 2016 16:18:12 +0100 (IST) Received: from HHMAIL01.hh.imgtec.org (10.100.10.19) by HHMAIL03.hh.imgtec.org (10.44.0.21) with Microsoft SMTP Server (TLS) id 14.3.294.0; Mon, 17 Oct 2016 16:18:15 +0100 Received: from [10.20.78.147] (10.20.78.147) by HHMAIL01.hh.imgtec.org (10.100.10.21) with Microsoft SMTP Server id 14.3.294.0; Mon, 17 Oct 2016 16:18:14 +0100 Date: Mon, 17 Oct 2016 16:18:06 +0100 From: "Maciej W. Rozycki" <macro@imgtec.com> To: <gdb-patches@sourceware.org> Subject: [PATCH] tilegx-tdep: Correct aliasing errors in `tilegx_analyze_prologue' Message-ID: <alpine.DEB.2.00.1610150200220.31859@tp.orcam.me.uk> User-Agent: Alpine 2.00 (DEB 1167 2008-08-23) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" |
Commit Message
Maciej W. Rozycki
Oct. 17, 2016, 3:18 p.m. UTC
Fix a load of aliasing build errors: cc1plus: warnings being treated as errors .../gdb/tilegx-tdep.c: In function 'CORE_ADDR tilegx_analyze_prologue(gdbarch*, CORE_ADDR, CORE_ADDR, tilegx_frame_cache*, frame_info*)': .../gdb/tilegx-tdep.c:609: error: dereferencing pointer 'operands' does break strict-aliasing rules .../gdb/tilegx-tdep.c:592: error: dereferencing pointer 'operands' does break strict-aliasing rules .../gdb/tilegx-tdep.c:571: error: dereferencing pointer 'operands' does break strict-aliasing rules [...] .../gdb/tilegx-tdep.c:601: error: dereferencing pointer '<anonymous>' does break strict-aliasing rules .../gdb/tilegx-tdep.c:601: note: initialized from here cc1plus: error: dereferencing pointer 'operands' does break strict-aliasing rules cc1plus: error: dereferencing pointer 'operands' does break strict-aliasing rules .../gdb/tilegx-tdep.c:452: note: initialized from here cc1plus: error: dereferencing pointer 'pretmp.896' does break strict-aliasing rules cc1plus: note: initialized from here cc1plus: error: dereferencing pointer 'pretmp.896' does break strict-aliasing rules cc1plus: note: initialized from here make[1]: *** [tilegx-tdep.o] Error 1 from an attempt to cast a `long long' pointer to an `int64_t' pointer, which may not necessarily be compatible types. Use the `long long' type for the auxiliary variable then as this is the type of the structure member referred. gdb/ * tilegx-tdep.c (tilegx_analyze_prologue): Use the `long long' type for `operands'. --- Hi, This was discovered in an `--enable-targets=all' `mips-mti-linux-gnu' build and may be dependent on the host type and compiler version, which are `x86_64-linux' and 4.4.7 here, respectively. OK to apply? Maciej gdb-tilegx-prologue-operands-fix.diff
Comments
On 10/17/2016 04:18 PM, Maciej W. Rozycki wrote: > Fix a load of aliasing build errors: > > cc1plus: warnings being treated as errors > .../gdb/tilegx-tdep.c: In function 'CORE_ADDR tilegx_analyze_prologue(gdbarch*, CORE_ADDR, CORE_ADDR, tilegx_frame_cache*, frame_info*)': > .../gdb/tilegx-tdep.c:609: error: dereferencing pointer 'operands' does break strict-aliasing rules > .../gdb/tilegx-tdep.c:592: error: dereferencing pointer 'operands' does break strict-aliasing rules > .../gdb/tilegx-tdep.c:571: error: dereferencing pointer 'operands' does break strict-aliasing rules > [...] > .../gdb/tilegx-tdep.c:601: error: dereferencing pointer '<anonymous>' does break strict-aliasing rules > .../gdb/tilegx-tdep.c:601: note: initialized from here > cc1plus: error: dereferencing pointer 'operands' does break strict-aliasing rules > cc1plus: error: dereferencing pointer 'operands' does break strict-aliasing rules > .../gdb/tilegx-tdep.c:452: note: initialized from here > cc1plus: error: dereferencing pointer 'pretmp.896' does break strict-aliasing rules > cc1plus: note: initialized from here > cc1plus: error: dereferencing pointer 'pretmp.896' does break strict-aliasing rules > cc1plus: note: initialized from here > make[1]: *** [tilegx-tdep.o] Error 1 > > from an attempt to cast a `long long' pointer to an `int64_t' pointer, > which may not necessarily be compatible types. Use the `long long' type > for the auxiliary variable then as this is the type of the structure > member referred. > > gdb/ > * tilegx-tdep.c (tilegx_analyze_prologue): Use the `long long' > type for `operands'. > --- > Hi, > > This was discovered in an `--enable-targets=all' `mips-mti-linux-gnu' > build and may be dependent on the host type and compiler version, which > are `x86_64-linux' and 4.4.7 here, respectively. OK to apply? > Sure, please do ahead. I'd be better if tilegx.h used uint32_t/uint64_t, etc, but that can always be done separately by someone motivated. Thanks, Pedro Alves
On Mon, 17 Oct 2016, Pedro Alves wrote: > > Fix a load of aliasing build errors: > > > > cc1plus: warnings being treated as errors > > .../gdb/tilegx-tdep.c: In function 'CORE_ADDR tilegx_analyze_prologue(gdbarch*, CORE_ADDR, CORE_ADDR, tilegx_frame_cache*, frame_info*)': > > .../gdb/tilegx-tdep.c:609: error: dereferencing pointer 'operands' does break strict-aliasing rules > > .../gdb/tilegx-tdep.c:592: error: dereferencing pointer 'operands' does break strict-aliasing rules > > .../gdb/tilegx-tdep.c:571: error: dereferencing pointer 'operands' does break strict-aliasing rules > > [...] > > .../gdb/tilegx-tdep.c:601: error: dereferencing pointer '<anonymous>' does break strict-aliasing rules > > .../gdb/tilegx-tdep.c:601: note: initialized from here > > cc1plus: error: dereferencing pointer 'operands' does break strict-aliasing rules > > cc1plus: error: dereferencing pointer 'operands' does break strict-aliasing rules > > .../gdb/tilegx-tdep.c:452: note: initialized from here > > cc1plus: error: dereferencing pointer 'pretmp.896' does break strict-aliasing rules > > cc1plus: note: initialized from here > > cc1plus: error: dereferencing pointer 'pretmp.896' does break strict-aliasing rules > > cc1plus: note: initialized from here > > make[1]: *** [tilegx-tdep.o] Error 1 > > > > from an attempt to cast a `long long' pointer to an `int64_t' pointer, > > which may not necessarily be compatible types. Use the `long long' type > > for the auxiliary variable then as this is the type of the structure > > member referred. > > > > gdb/ > > * tilegx-tdep.c (tilegx_analyze_prologue): Use the `long long' > > type for `operands'. > > --- > > Hi, > > > > This was discovered in an `--enable-targets=all' `mips-mti-linux-gnu' > > build and may be dependent on the host type and compiler version, which > > are `x86_64-linux' and 4.4.7 here, respectively. OK to apply? > > > > Sure, please do ahead. > > I'd be better if tilegx.h used uint32_t/uint64_t, etc, > but that can always be done separately by someone motivated. The issue there is `tilegx.h' is shared with binutils which have not yet moved past C89 I believe. Of course the use of `long long' is already non-C89, however it's been there with some C89 compilers before `stdint.h' types have been introduced. We could use `bfd_uint64_t' instead, but that would make `tilegx.h' depend on a BFD header. So it's not an obvious call and therefore I agree it's best left to someone who has an actual interest with the target. Committed now, thanks for your review. Maciej
On 10/18/2016 04:50 AM, Maciej W. Rozycki wrote: >> > >> > Sure, please do ahead. >> > >> > I'd be better if tilegx.h used uint32_t/uint64_t, etc, >> > but that can always be done separately by someone motivated. > The issue there is `tilegx.h' is shared with binutils which have not yet > moved past C89 I believe. Of course the use of `long long' is already > non-C89, however it's been there with some C89 compilers before `stdint.h' > types have been introduced. We could use `bfd_uint64_t' instead, but that > would make `tilegx.h' depend on a BFD header. So it's not an obvious call > and therefore I agree it's best left to someone who has an actual interest > with the target. Yeah, C89 on paper. :-) It's certainly fine to use unadorned uint64_t etc. directly nowadays. Even if the host compiler is so old that it doesn't support those, we have "bfd_stdint.h" to fill the gap: opcodes/aarch64-dis.h:23:#include "bfd_stdint.h" opcodes/nds32-dis.c:30:#include "bfd_stdint.h" opcodes/aarch64-dis.c:22:#include "bfd_stdint.h" ld/elf-hints-local.h:28:#include "bfd_stdint.h" Something like 'grep "u\?int\(8\|16\|32\|64\)_t" -rn' will show many uses of uint32_t, etc. (not bfd_uint*_t) throughout opcodes, bfd, ld, etc. I won't be surprised at all if someone missed adding bfd_stdint.h for some of them and they've been working in practice for a long while. (better than bfd_stdint.h would be to use gnulib for the whole toolchain, not just for gdb, but that's yet another matter.) As I've been suggesting in recent C++11 discussions, IMO, what we claim to support in theory matters significantly less than what we actually support in practice. And in practice, all it matters is which compilers (and their versions), not standards, people care about actually using and supporting. Standards give us guidelines, not final answers. > Committed now, thanks for your review. Thanks. Thanks, Pedro Alves
Index: binutils/gdb/tilegx-tdep.c =================================================================== --- binutils.orig/gdb/tilegx-tdep.c 2016-10-05 00:58:09.000000000 +0100 +++ binutils/gdb/tilegx-tdep.c 2016-10-15 01:29:35.689998335 +0100 @@ -449,7 +449,7 @@ tilegx_analyze_prologue (struct gdbarch* for (i = 0; i < num_insns; i++) { struct tilegx_decoded_instruction *this_insn = &decoded[i]; - int64_t *operands = (int64_t *) this_insn->operand_values; + long long *operands = this_insn->operand_values; const struct tilegx_opcode *opcode = this_insn->opcode; switch (opcode->mnemonic)