From: Andrew Burgess <andrew.burgess@embecosm.com>
To: Tom Tromey <tom@tromey.com>
Cc: Andreas Schwab <schwab@suse.de>, gdb-patches@sourceware.org
Subject: Re: [PATCH] Do not accidentally include in-tree readline headers
Date: Wed, 26 Sep 2018 16:47:00 -0000 [thread overview]
Message-ID: <20180926164743.GM5952@embecosm.com> (raw)
In-Reply-To: <87r2hgjr9v.fsf@tromey.com>
* Tom Tromey <tom@tromey.com> [2018-09-26 07:57:00 -0600]:
> >>>>> "Andreas" == Andreas Schwab <schwab@suse.de> writes:
>
> Andreas> On Sep 26 2018, Tom Tromey <tom@tromey.com> wrote:
> >> This patch fixes the problem in a mildly hacky way: remove the
> >> offending -I option, and change gdb to use #include "../opcodes/..."
> >> instead. This continues to make it clear where the header comes from,
> >> without allowing incorrect behavior.
>
> Andreas> Most of these headers are "opcode/...", not "opcodes/...".
>
> Whoops, I made the patch via perl and didn't really read it.
> Here's a better one.
>
> Tom
>
> commit d6df2198c8b7b42a351b00507f089de9e087ac6c
> Author: Tom Tromey <tom@tromey.com>
> Date: Wed Sep 26 06:54:17 2018 -0600
>
> Do not accidentally include in-tree readline headers
>
> PR build/17077 points out that when --with-system-readline is given,
> gdb will still pick up the in-tree readline headers. Normally this is
> not a big problem, because readline is very stable and so the ABI does
> not change much; but it is clearly a bug to do this, and could bite at
> some point.
>
> The basic problem is that OPCODES_CFLAGS uses -I$(OPCODES_SRC)/.. so
> that #include "opcodes/..." works. However, this also makes it so
> the srcdir.
I don't really understand what the last sentence is telling me. It
feels like there's a bit missing from the end...
Thanks,
Andrew
>
> This patch fixes the problem in a mildly hacky way: remove the
> offending -I option, and change gdb to use #include "../opcodes/..."
> instead. This continues to make it clear where the header comes from,
> without allowing incorrect behavior.
>
> Tested by rebuilding and then looking at the *.Po files.
>
> gdb/ChangeLog
> 2018-09-26 Tom Tromey <tom@tromey.com>
>
> PR build/17077:
> * Makefile.in (OPCODES_CFLAGS): Remove "-I$(OPCODES_SRC)/..".
> * arc-tdep.c, frv-tdep.c, lm32-tdep.c, mep-tdep.c,
> microblaze-tdep.c, or1k-tdep.h: Use ../opcodes, not opcodes, in
> #include.
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index fa1ca59cc0..7f13a1ceb4 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,11 @@
> +2018-09-26 Tom Tromey <tom@tromey.com>
> +
> + PR build/17077:
> + * Makefile.in (OPCODES_CFLAGS): Remove "-I$(OPCODES_SRC)/..".
> + * arc-tdep.c, frv-tdep.c, lm32-tdep.c, mep-tdep.c,
> + microblaze-tdep.c, or1k-tdep.h: Use ../opcodes, not opcodes, in
> + #include.
> +
> 2018-09-26 Tom Tromey <tom@tromey.com>
>
> * valops.c (auto_abandon): Remove dead code.
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 3b158fa1db..3d28120fb8 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -446,8 +446,7 @@ OPCODES = $(OPCODES_DIR)/libopcodes.a
> # Where are the other opcode tables which only have header file
> # versions?
> OP_INCLUDE = $(INCLUDE_DIR)/opcode
> -# Some source files like to use #include "opcodes/file.h"
> -OPCODES_CFLAGS = -I$(OP_INCLUDE) -I$(OPCODES_SRC)/..
> +OPCODES_CFLAGS = -I$(OP_INCLUDE)
>
> # The simulator is usually nonexistent; targets that include one
> # should set this to list all the .o or .a files to be linked in.
> diff --git a/gdb/arc-tdep.c b/gdb/arc-tdep.c
> index fad9170978..b9dcbbc1e5 100644
> --- a/gdb/arc-tdep.c
> +++ b/gdb/arc-tdep.c
> @@ -33,7 +33,7 @@
>
> /* ARC header files. */
> #include "opcode/arc.h"
> -#include "opcodes/arc-dis.h"
> +#include "../opcodes/arc-dis.h"
> #include "arc-tdep.h"
>
> /* Standard headers. */
> diff --git a/gdb/frv-tdep.c b/gdb/frv-tdep.c
> index 1eed441f2b..dafab75654 100644
> --- a/gdb/frv-tdep.c
> +++ b/gdb/frv-tdep.c
> @@ -29,7 +29,7 @@
> #include "dis-asm.h"
> #include "sim-regno.h"
> #include "gdb/sim-frv.h"
> -#include "opcodes/frv-desc.h" /* for the H_SPR_... enums */
> +#include "../opcodes/frv-desc.h" /* for the H_SPR_... enums */
> #include "symtab.h"
> #include "elf-bfd.h"
> #include "elf/frv.h"
> diff --git a/gdb/lm32-tdep.c b/gdb/lm32-tdep.c
> index 942852140d..694d30ee1c 100644
> --- a/gdb/lm32-tdep.c
> +++ b/gdb/lm32-tdep.c
> @@ -35,7 +35,7 @@
> #include "regcache.h"
> #include "trad-frame.h"
> #include "reggroups.h"
> -#include "opcodes/lm32-desc.h"
> +#include "../opcodes/lm32-desc.h"
> #include <algorithm>
>
> /* Macros to extract fields from an instruction. */
> diff --git a/gdb/mep-tdep.c b/gdb/mep-tdep.c
> index 69e7fdda59..e8ceec9288 100644
> --- a/gdb/mep-tdep.c
> +++ b/gdb/mep-tdep.c
> @@ -48,8 +48,8 @@
>
> /* Get the user's customized MeP coprocessor register names from
> libopcodes. */
> -#include "opcodes/mep-desc.h"
> -#include "opcodes/mep-opc.h"
> +#include "../opcodes/mep-desc.h"
> +#include "../opcodes/mep-opc.h"
>
> \f
> /* The gdbarch_tdep structure. */
> diff --git a/gdb/microblaze-tdep.c b/gdb/microblaze-tdep.c
> index 9bac8643c4..63095d3ede 100644
> --- a/gdb/microblaze-tdep.c
> +++ b/gdb/microblaze-tdep.c
> @@ -34,8 +34,8 @@
> #include "dwarf2-frame.h"
> #include "osabi.h"
> #include "target-descriptions.h"
> -#include "opcodes/microblaze-opcm.h"
> -#include "opcodes/microblaze-dis.h"
> +#include "../opcodes/microblaze-opcm.h"
> +#include "../opcodes/microblaze-dis.h"
> #include "microblaze-tdep.h"
> #include "remote.h"
>
> diff --git a/gdb/or1k-tdep.h b/gdb/or1k-tdep.h
> index 4a93540c3a..b630c77053 100644
> --- a/gdb/or1k-tdep.h
> +++ b/gdb/or1k-tdep.h
> @@ -24,8 +24,8 @@
> #define TARGET_OR1K
> #endif
>
> -#include "opcodes/or1k-desc.h"
> -#include "opcodes/or1k-opc.h"
> +#include "../opcodes/or1k-desc.h"
> +#include "../opcodes/or1k-opc.h"
>
> /* General Purpose Registers */
> #define OR1K_ZERO_REGNUM 0
next prev parent reply other threads:[~2018-09-26 16:47 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-26 12:59 Tom Tromey
2018-09-26 13:12 ` Andreas Schwab
2018-09-26 13:57 ` Tom Tromey
2018-09-26 16:47 ` Andrew Burgess [this message]
2018-09-26 20:19 ` Tom Tromey
2018-10-07 4:46 ` Tom Tromey
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180926164743.GM5952@embecosm.com \
--to=andrew.burgess@embecosm.com \
--cc=gdb-patches@sourceware.org \
--cc=schwab@suse.de \
--cc=tom@tromey.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).