Patchwork Do not accidentally include in-tree readline headers

login
register
mail settings
Submitter Tom Tromey
Date Sept. 26, 2018, 1:57 p.m.
Message ID <87r2hgjr9v.fsf@tromey.com>
Download mbox | patch
Permalink /patch/29551/
State New
Headers show

Comments

Tom Tromey - Sept. 26, 2018, 1:57 p.m.
>>>>> "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.
    
    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.
Andrew Burgess - Sept. 26, 2018, 4:47 p.m.
* 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"
>  
>  
>  /* 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
Tom Tromey - Sept. 26, 2018, 8:18 p.m.
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> I don't really understand what the last sentence is telling me.  It
Andrew> feels like there's a bit missing from the end...

Oops, sorry.  I changed it to:

    The basic problem is that OPCODES_CFLAGS uses -I$(OPCODES_SRC)/..  so
    that #include "opcodes/..." works.  However, this also makes it so the
    #include <readline/...> includes are found in the srcdir.

Tom

Patch

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"
 
 
 /* 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