Message ID | 1429287116-20216-1-git-send-email-jcmvbkbc@gmail.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 58478 invoked by alias); 17 Apr 2015 16:12:30 -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 58409 invoked by uid 89); 17 Apr 2015 16:12:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.2 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, FROM_LOCAL_NOVOWEL, HK_RANDOM_ENVFROM, HK_RANDOM_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-lb0-f174.google.com Received: from mail-lb0-f174.google.com (HELO mail-lb0-f174.google.com) (209.85.217.174) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 17 Apr 2015 16:12:28 +0000 Received: by lbbuc2 with SMTP id uc2so86879587lbb.2 for <gdb-patches@sourceware.org>; Fri, 17 Apr 2015 09:12:24 -0700 (PDT) X-Received: by 10.112.61.136 with SMTP id p8mr4875345lbr.107.1429287144744; Fri, 17 Apr 2015 09:12:24 -0700 (PDT) Received: from octofox.metropolis ([5.19.183.212]) by mx.google.com with ESMTPSA id uj9sm2500088lbb.38.2015.04.17.09.12.22 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 17 Apr 2015 09:12:23 -0700 (PDT) From: Max Filippov <jcmvbkbc@gmail.com> To: gdb-patches@sourceware.org Cc: Maxim Grigoriev <maxim2405@gmail.com>, Woody LaRue <larue@cadence.com>, Marc Gauthier <marc@cadence.com>, linux-xtensa@linux-xtensa.org, Pedro Alves <palves@redhat.com>, Max Filippov <jcmvbkbc@gmail.com> Subject: [PATCH] gdb: fix xtensa build with custom overlay Date: Fri, 17 Apr 2015 19:11:56 +0300 Message-Id: <1429287116-20216-1-git-send-email-jcmvbkbc@gmail.com> |
Commit Message
Max Filippov
April 17, 2015, 4:11 p.m. UTC
The commit 14e361d7aa3bbd8601b0457ee8558344e444c651 ("xtensa-config.c: missing defs.h include") fixed the build of default xtensa configuration by including defs.h in the beginning of xtensa-config.c. Unfortunately this fix doesn't work when gdb is configured for another xtensa core, as the file xtensa-config.c is a part of configuration overlay and it gets overwritten. To fix the build for all existing configurations include defs.h into gdb/xtensa-tdep.h, where the issue (reference to undeclared uint32_t) actually is. 2015-04-17 Max Filippov <jcmvbkbc@gmail.com> gdb/ * xtensa-tdep.h: Include defs.h --- gdb/xtensa-tdep.h | 2 ++ 1 file changed, 2 insertions(+)
Comments
On 04/17/2015 05:11 PM, Max Filippov wrote: > The commit 14e361d7aa3bbd8601b0457ee8558344e444c651 ("xtensa-config.c: > missing defs.h include") fixed the build of default xtensa configuration > by including defs.h in the beginning of xtensa-config.c. Unfortunately > this fix doesn't work when gdb is configured for another xtensa core, as > the file xtensa-config.c is a part of configuration overlay and it gets > overwritten. To fix the build for all existing configurations include > defs.h into gdb/xtensa-tdep.h, where the issue (reference to undeclared > uint32_t) actually is. Hmm, I think that's news to me. Can you please expand a bit on why is this mechanism necessary? In any case, that mechanism must already by outputting: #include "xtensa-config.h" #include "xtensa-tdep.h" Just make it output #include "defs.h" as well? That .c files must include defs.h is part of the standards: https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards " (...) All other .c files under gdb/ must include defs.h as their first non-comment line. (...) No .h file should include defs.h, server.h or common-defs.h. (...) " > > 2015-04-17 Max Filippov <jcmvbkbc@gmail.com> > gdb/ > * xtensa-tdep.h: Include defs.h Note missing period. Thanks, Pedro Alves
On Fri, Apr 17, 2015 at 7:21 PM, Pedro Alves <palves@redhat.com> wrote: > On 04/17/2015 05:11 PM, Max Filippov wrote: >> The commit 14e361d7aa3bbd8601b0457ee8558344e444c651 ("xtensa-config.c: >> missing defs.h include") fixed the build of default xtensa configuration >> by including defs.h in the beginning of xtensa-config.c. Unfortunately >> this fix doesn't work when gdb is configured for another xtensa core, as >> the file xtensa-config.c is a part of configuration overlay and it gets >> overwritten. To fix the build for all existing configurations include >> defs.h into gdb/xtensa-tdep.h, where the issue (reference to undeclared >> uint32_t) actually is. > > Hmm, I think that's news to me. Can you please expand a bit > on why is this mechanism necessary? xtensa cores may have greatly varying register and instruction sets, depending on options configured at processor creation time. So parts of gdb that deal with registers and instructions are generated during processor build and need to be replaced in the gdb source code to produce gdb that can work with a particular xtensa core. > In any case, that mechanism must already by outputting: > > #include "xtensa-config.h" > #include "xtensa-tdep.h" > > Just make it output #include "defs.h" as well? Yes, that can be done for the newly generated files, but there are existing configuration overlays which we need to patch in this case. It can be done, it's just much more changes. And more importantly, builds that used to work before 54bff650843cacd3c17a0afdb0fe32e15e1b65b0 "gdb: xtensa: fix on 64-bit host" were broken by that change.
On 04/17/2015 05:45 PM, Max Filippov wrote: > On Fri, Apr 17, 2015 at 7:21 PM, Pedro Alves <palves@redhat.com> wrote: >> On 04/17/2015 05:11 PM, Max Filippov wrote: >>> The commit 14e361d7aa3bbd8601b0457ee8558344e444c651 ("xtensa-config.c: >>> missing defs.h include") fixed the build of default xtensa configuration >>> by including defs.h in the beginning of xtensa-config.c. Unfortunately >>> this fix doesn't work when gdb is configured for another xtensa core, as >>> the file xtensa-config.c is a part of configuration overlay and it gets >>> overwritten. To fix the build for all existing configurations include >>> defs.h into gdb/xtensa-tdep.h, where the issue (reference to undeclared >>> uint32_t) actually is. >> >> Hmm, I think that's news to me. Can you please expand a bit >> on why is this mechanism necessary? > > xtensa cores may have greatly varying register and instruction sets, > depending on options configured at processor creation time. So parts > of gdb that deal with registers and instructions are generated during > processor build and need to be replaced in the gdb source code to > produce gdb that can work with a particular xtensa core. > >> In any case, that mechanism must already by outputting: >> >> #include "xtensa-config.h" >> #include "xtensa-tdep.h" >> >> Just make it output #include "defs.h" as well? > > Yes, that can be done for the newly generated files, but there are existing > configuration overlays which we need to patch in this case. It can be done, > it's just much more changes. I don't know about much more changes. We really shouldn't promise not to break out-of-tree changes. The point is this is quite brittle, and I think as is, the onus should be on you to keep it working. Nothing tells us that we might want or need to do an across-the-tree change that breaks all this. For example, for the ongoing C++ conversion. It seems that the best solution would be to design a mechanism to load this information into gdb dynamically, either through the xml target description mechanism, or through some new Python hook that reads an xtensa-specific description file. Thanks, Pedro Alves
diff --git a/gdb/xtensa-tdep.h b/gdb/xtensa-tdep.h index adacaf8..4ebe6de 100644 --- a/gdb/xtensa-tdep.h +++ b/gdb/xtensa-tdep.h @@ -21,6 +21,8 @@ /* XTENSA_TDEP_VERSION can/should be changed along with XTENSA_CONFIG_VERSION whenever the "tdep" structure changes in an incompatible way. */ +#include "defs.h" + #define XTENSA_TDEP_VERSION 0x60 /* Xtensa register type. */