From patchwork Sun Jan 12 20:22:18 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 37336 Received: (qmail 30237 invoked by alias); 12 Jan 2020 20:22:23 -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 30219 invoked by uid 89); 12 Jan 2020 20:22:22 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-17.9 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.1 spammy=resources, forgotten, H*M:cc68 X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 12 Jan 2020 20:22:21 +0000 Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 826CE1E4C2; Sun, 12 Jan 2020 15:22:19 -0500 (EST) Subject: Re: [PATCH v2 0/7] Enable -Wmissing-declarations diagnostic To: Simon Marchi , gdb-patches@sourceware.org, Tom Tromey , Joel Brobecker References: <20200110220027.26450-1-simon.marchi@efficios.com> From: Simon Marchi Message-ID: Date: Sun, 12 Jan 2020 15:22:18 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.3.1 MIME-Version: 1.0 In-Reply-To: <20200110220027.26450-1-simon.marchi@efficios.com> On 2020-01-10 5:00 p.m., Simon Marchi wrote: > This is v2 of: > > https://sourceware.org/ml/gdb-patches/2019-11/msg00805.html > > Although a few patches of the original series were merged, since they > were valid fixes in any case. > > I have re-generated the configure script for gdbserver, which I had > forgotten in v1, and it of course unearthed some more issues. > > I have build-tested the series with gcc 7.4.0 and clang 9. As I tried building gdbserver with a few cross compilers, I found another build issue, with code generated by regdat.sh. The following patch addresses it. It should not be too controversial, but I wanted to run it by you first. From ff6633d14065273791b5b70266f4afceba22aeec Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Sun, 12 Jan 2020 13:09:46 -0500 Subject: [PATCH] gdb: make regformats output a declaration for the init function When compiling gdbserver for an architecture that uses the regdat.sh script (such as m68k) and the -Wmissing-declarations compiler flag, I get: REGDAT reg-m68k-generated.c CXX reg-m68k.o reg-m68k-generated.c:30:1: error: no previous declaration for 'void init_registers_m68k()' [-Werror=missing-declarations] 30 | init_registers_m68k (void) | ^~~~~~~~~~~~~~~~~~~ The same happens with other architectures, such as s390, but I'll be using 68k as an example. The init_registers_m68k function is defined in reg-m68k-generated.c, which is produced by the regformats/regdat.sh script. This script reads the regformats/reg-m68k.dat file, containing a register description, and produces C code that creates a corresponding target description at runtime. The init_registers_m68k function is invoked at initialization time in linux-m68k-low.c. The function must therefore be non-static, but does not have a declaration at the moment. The real clean way of fixing this would be to make regdat.sh generate a .h file (in addition to the .c file) with declarations for whatever is in the .c file. The generated .c file would include the .h file, and therefore the definition would have a corresponding declaration. The linux-m68k-low.c file would also include this .h file, instead of having its own declaration of init_registers_m68k, like it does now. However, this would be a quite big change for not much gain. As far as I understand, some common architectures (i386, x86-64, ARM, AArch64) have been moved to dynamically building target descriptions based on features (the linux-*-tdesc.c files in gdbserver) and don't use regdat.sh anymore. Logically (and given infinite development resources), the other architectures would be migrated to this system too and the regdat.sh script would be dropped. A new architecture would probably not use regdat.sh either. So I therefore propose this simpler patch instead, which just adds a local declaration in the generated file. gdb/ChangeLog: * regformats/regdat.sh: Generate declaration for init function. --- gdb/regformats/regdat.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh index 1839a881213c..a40f2336484f 100755 --- a/gdb/regformats/regdat.sh +++ b/gdb/regformats/regdat.sh @@ -127,6 +127,10 @@ do echo "const struct target_desc *tdesc_${name};" echo "" + + # This is necessary for -Wmissing-declarations. + echo "void init_registers_${name} (void);" + echo "void" echo "init_registers_${name} (void)" echo "{"