From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 39051 invoked by alias); 12 Jan 2017 14:35:38 -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 39029 invoked by uid 89); 12 Jan 2017 14:35:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-5.1 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=sk:compoun X-Spam-User: qpsmtpd, 2 recipients 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 ESMTP; Thu, 12 Jan 2017 14:35:26 +0000 Received: from smtp.corp.redhat.com (int-mx16.intmail.prod.int.phx2.redhat.com [10.5.11.28]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D38936DD95; Thu, 12 Jan 2017 14:35:26 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0C8FE6D24D; Thu, 12 Jan 2017 14:35:25 +0000 (UTC) Subject: Re: [PATCH 3/8] Disassembly unit test: disassemble one instruction To: Yao Qi , binutils@sourceware.org, gdb-patches@sourceware.org References: <1484051178-16013-1-git-send-email-yao.qi@linaro.org> <1484051178-16013-4-git-send-email-yao.qi@linaro.org> From: Pedro Alves Message-ID: <1be65603-63b3-5273-973a-84b6e6006798@redhat.com> Date: Thu, 12 Jan 2017 14:35:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1484051178-16013-4-git-send-email-yao.qi@linaro.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-01/txt/msg00226.txt.bz2 On 01/10/2017 12:26 PM, Yao Qi wrote: > +static void > +gdb_disassembler_print_one_insn_test (struct gdbarch *gdbarch) > +{ > + int len = -1; > + const gdb_byte *insn = NULL; > + > + switch (gdbarch_bfd_arch_info (gdbarch)->arch) > + { > + case bfd_arch_bfin: > + /* M3.L = 0xe117 */ > + insn = (const gdb_byte[]) {0x17, 0xe1, 0xff, 0xff}; > + len = 4; > + break; This is construct is problematic. Unfortunately, compound literals aren't valid C++. They're valid in C, but not in C++. G++ accepts them as an extension. Maybe all C++ compilers we care about support them too, so that's not the real problem I'm pointing at. The problem is that compound literal above may have automatic storage duration, and thus die at the end of the switch scope. >From http://en.cppreference.com/w/c/language/compound_literal "The unnamed object to which the compound literal evaluates has static storage duration if the compound literal occurs at file scope and automatic storage duration if the compound literal occurs at block scope (in which case the object's lifetime ends at the end of the enclosing block)." I didn't check the C standard, but I assume that's correct. At , we see: "As an optimization, G++ sometimes gives array compound literals longer lifetimes: when the array either appears outside a function or has a const-qualified type. If foo and its initializer had elements of type char *const rather than char *, or if foo were a global variable, the array would have static storage duration. But it is probably safest just to avoid the use of array compound literals in C++ code. " Given all the warnings, I'd think it best to avoid the construct. We can just name the arrays: static void gdb_disassembler_print_one_insn_test (struct gdbarch *gdbarch) { int len = -1; const gdb_byte *insn = NULL; switch (gdbarch_bfd_arch_info (gdbarch)->arch) { case bfd_arch_bfin: /* M3.L = 0xe117 */ static const gdb_byte bfin_insn[] = { 0x17, 0xe1, 0xff, 0xff }; insn = bfin_insn; len = 4; break; case bfd_arch_bfin: /* mov r0, #0 */ static const gdb_byte arm_insn[] = { 0x0, 0x0, 0xa0, 0xe3 }; insn = arm_insn; len = 4; Or maybe make the compiler do the "sizeof" for us, maybe eliminating copy/paste mistakes: struct test_insn { template explicit test_insn (const gdb_byte (&insn_)[Len]) : insn (insn_), len_ (Len) {} gdb_byte *insn; int len; }; static void gdb_disassembler_print_one_insn_test (struct gdbarch *gdbarch) { test_insn insn = { NULL, -1 }; switch (gdbarch_bfd_arch_info (gdbarch)->arch) { case bfd_arch_bfin: /* M3.L = 0xe117 */ static const gdb_byte bfin_insn[] = { 0x17, 0xe1, 0xff, 0xff }; insn = test_insn (bfin_insn); break; case bfd_arch_bfin: /* mov r0, #0 */ static const gdb_byte arm_insn[] = { 0x0, 0x0, 0xa0, 0xe3 }; insn = test_insn (arm_insn); [....] Maybe split the insns out of the switch: /* M3.L = 0xe117 */ static const gdb_byte bfin_test_insn[] = { 0x17, 0xe1, 0xff, 0xff }; /* mov r0, #0 */ static const gdb_byte arm_test_insn[] = { 0x0, 0x0, 0xa0, 0xe3 }; static void gdb_disassembler_print_one_insn_test (struct gdbarch *gdbarch) { test_insn insn = { NULL, -1 }; switch (gdbarch_bfd_arch_info (gdbarch)->arch) { case bfd_arch_bfin: insn = test_insn (bfin_test_insn); break; case bfd_arch_bfin: insn = test_insn (arm_test_insn); break; [....] Just some ideas. Thanks, Pedro Alves