Patchwork [v2,0/6] Move gdbsupport to top level

login
register
mail settings
Submitter Pedro Alves
Date Jan. 15, 2020, 2:41 p.m.
Message ID <8a8de6a9-37b8-cad3-c818-be903037fe48@redhat.com>
Download mbox | patch
Permalink /patch/37393/
State New
Headers show

Comments

Pedro Alves - Jan. 15, 2020, 2:41 p.m.
On 1/15/20 2:09 PM, Pedro Alves wrote:

> So it seems like the issue is that that '#include <config.h>' picks up
> bfd's config.h instead of gnulib's by mistake.

I guess that this problem doesn't trigger in the gdb
dir because there we generate config.h under that exact name so
gnulib's libc-config.h ends up picking gdb's config.h instead
of gnulib.c and that ends up harmless.  

In gdbsupport, the config.h file is really named support-config.h,
so that '#include <config.h>' in libc-config.h doesn't pick it
like it would if it had the conventional config.h name.

gnulib's config.h is generated under build/gnulib/ directly,
so adding that to the include path fixes it for me.

Don't know what I think of gnulib headers including <config.h>.
Maybe we should rename gdb's config.h to gdb-config.h too.

From e212b0a4e5d332ddcc4ea12ce51481a3bf90ef34 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 15 Jan 2020 14:12:43 +0000
Subject: [PATCH] Fix gdbsupport build

I'm seeing this on F27 (a clean build from scratch):

~~~~~~~~~~~~~~~~~~~~~~~~~~~
 make[3]: Entering directory '/home/pedro/brno/pedro/gdb/binutils-gdb/build/gdbsupport'
   CC       gdb_tilde_expand.o
 In file included from /home/pedro/gdb/binutils-gdb/src/gdbsupport/../gnulib/import/libc-config.h:33:0,
                  from ../gnulib/import/glob.h:544,
                  from /home/pedro/gdb/binutils-gdb/src/gdbsupport/gdb_tilde_expand.c:22:
 ../bfd/config.h:7:4: error: #error config.h must be #included before system headers
  #  error config.h must be #included before system headers
     ^~~~~
~~~~~~~~~~~~~~~~~~~~~~~~~~~

libc-config.h, where it includes config.h, says:

~~~~~~~~~~~~~~~~~~~~~~~~~~~
 /* This is intended to be a good-enough substitute for glibc system
    macros like those defined in <sys/cdefs.h>, so that Gnulib code
    shared with glibc can do this as the first #include:

      #ifndef _LIBC
      # include <libc-config.h>
      #endif

    When compiled as part of glibc this is a no-op; when compiled as
    part of Gnulib this includes Gnulib's <config.h> and defines macros
    that glibc library code would normally assume.  */

 #include <config.h>
~~~~~~~~~~~~~~~~~~~~~~~~~~~

The issue is that that '#include <config.h>' picks up bfd's config.h
instead of gnulib's.

I guess that this problem doesn't trigger in the gdb dir because there
we generate config.h under that exact name so gnulib's libc-config.h
ends up picking gdb's config.h instead of gnulib.c and that ends up
harmless.

In gdbsupport, the config.h file is really named support-config.h, so
that '#include <config.h>' in libc-config.h doesn't pick it like it
would if it had the conventional config.h name.

gnulib's config.h is generated under build/gnulib/ directly, so adding
that to the include path fixes it for me.

gdbsupport/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* Makefile.am (AM_CPPFLAGS): Add -I../gnulib.
	* Makefile.in: Regenerate.
---
 gdbsupport/Makefile.am | 2 +-
 gdbsupport/Makefile.in | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)


base-commit: aad09917e04b33da463f1703aab8d057cfe3f54e
Pedro Alves - Jan. 15, 2020, 2:55 p.m.
On 1/15/20 2:41 PM, Pedro Alves wrote:
> Don't know what I think of gnulib headers including <config.h>.
> Maybe we should rename gdb's config.h to gdb-config.h too.

Hit reply to soon.  I meant to add,

... and then, add a manually-written config.h in the build
dir that does:

 #include <gdbsupport/support-config.h>
 #include <gdb-config.h>

We'd do the same to gdbsupport, add a config.h in its
build dir that does:

 #include "gnulib/config.h"
 #include <support-config.h>

Those config.h files would go in the build dirs so that
they're not picked by other build directories.

With that, any "#include <config.h>" in any header ends up
picking the currently-being-built project's config.h, plus
the dependencies' config.h files.

Just a half-baked thought.  Not sure it's the best idea.

Thanks,
Pedro Alves

Patch

diff --git a/gdbsupport/Makefile.am b/gdbsupport/Makefile.am
index 1a001a00817..fdf376b2e12 100644
--- a/gdbsupport/Makefile.am
+++ b/gdbsupport/Makefile.am
@@ -21,7 +21,7 @@  AUTOMAKE_OPTIONS = no-dist foreign
 ACLOCAL_AMFLAGS = -I . -I ../config
 
 AM_CPPFLAGS = -I$(srcdir)/../include -I$(srcdir)/../gdb \
-    -I../gnulib/import -I$(srcdir)/../gnulib/import \
+    -I../gnulib -I../gnulib/import -I$(srcdir)/../gnulib/import \
     -I.. -I$(srcdir)/.. $(INCINTL) -I../bfd -I$(srcdir)/../bfd
 
 override CC := $(CXX)
diff --git a/gdbsupport/Makefile.in b/gdbsupport/Makefile.in
index 5723ae5e97e..c3e6b744ec0 100644
--- a/gdbsupport/Makefile.in
+++ b/gdbsupport/Makefile.in
@@ -347,7 +347,7 @@  top_srcdir = @top_srcdir@
 AUTOMAKE_OPTIONS = no-dist foreign
 ACLOCAL_AMFLAGS = -I . -I ../config
 AM_CPPFLAGS = -I$(srcdir)/../include -I$(srcdir)/../gdb \
-    -I../gnulib/import -I$(srcdir)/../gnulib/import \
+    -I../gnulib -I../gnulib/import -I$(srcdir)/../gnulib/import \
     -I.. -I$(srcdir)/.. $(INCINTL) -I../bfd -I$(srcdir)/../bfd
 
 noinst_LIBRARIES = libgdbsupport.a