[PATCH] gdb: fix xtensa build with custom overlay

[PATCH] gdb: fix xtensa build with custom overlay

[Max Filippov]

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(+)

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.  */
-- 
1.8.1.4

Re: [PATCH] gdb: fix xtensa build with custom overlay

[Pedro Alves]

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

Re: [PATCH] gdb: fix xtensa build with custom overlay

[Max Filippov]

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.

-- 
Thanks.
-- Max

Re: [PATCH] gdb: fix xtensa build with custom overlay

[Pedro Alves]

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