[RFA/RFC] libdecnumber/gstdint.h is back :-(

[RFA/RFC] libdecnumber/gstdint.h is back :-(

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 2685 bytes --]

Hello,

You might remember that we had some issues with libdecnumber/gstdint.h
when compiling dfp.c.  This was because dfp.c includes headers from
libdecnumber which in turn includes gstdint.h.  Unfortunately, GDB
also includes gnulib/stdint.h and the two header files are incompatible.
At the time, we thought we had fixed the problem by introducing
gdb/gstdint.h which only contained an include of <stdint.h>, thus
thinking that it would trick the libdecnumber header files into using
our stdint.h file while compiling GDB.

See: http://www.sourceware.org/ml/gdb-patches/2008-06/msg00513.html
and followup messages for more details.

Unfortunately, it doesn't seem to work after all. I am even wondering
whether it worked back then, but I don't want to spend the time to
do some archeology. The reason it doesn't work is because decContext.h
includes gstdint.h using double quotes:

  #include "gstdint.h"             /* C99 standard integers           */

I looked at the C90 draft that I have, and it seems to be saying
that, whether you use angled brackets or double quotes in your include
syntax, the way the file is located is implementation defined.
So, no help there. This needs to be confirmed, but it looks like
GCC will try to find the include file starting from the same directory
as the file including it when double quotes are used. In other words,
when the following directive is used from ../libdecnumber/decContext.h

   #include "gstdint.h"

GCC will first try ../libdecnumber/gstdint.h, not ./gstdint.h.
This defeats the purpose of the gstdint.h that we introduced in
the gdb subdirectory.

I experimented with using angled brackets to include gstdint.h,
just out of curiousity, and it does indeed fix the build failure.
However, I don't think that libdecnumber would agree to that
change.

What occured to me while thinking about all this is that the
root of the problem is that we have two incompatible files that
should be identical in practice. Since we are always building
libdecnumber, can we use libdecnumber's gstdint.h and ditch
gnulib's stdint.h?

Attached is a patch that implements that.  It fixes the build problem
on Tru64, for instance.

2008-09-13  Joel Brobecker  <brobecker@adacore.com>

        * defs.h: Replace include of gnulib's stdint.h by libdecnumber's
        gstdint.h.
        * Makefile.in (GNULIB_H): Remove @GNULIB_STDINT_H@.
        (HFILES_NO_SRCDIR): Remove gnulib/stdint.in.h.
        * configure.ac: Remove computation and substitution of GNULIB_STDINT_H.
        * configure: Regenerate.

Tested on x86-linux with a full rebuild.  No regression.
Also fixes the build failure on Tru64.

Thoughts? OK to apply?

Thanks,
-- 
Joel

[-- Attachment #2: gstdint.diff --]
[-- Type: text/plain, Size: 3443 bytes --]

diff -r e03176e728c7 gdb/defs.h
--- a/gdb/defs.h	Fri Sep 12 20:42:16 2008 -0700
+++ b/gdb/defs.h	Fri Sep 12 21:24:21 2008 -0700
@@ -28,7 +28,7 @@
 #include <stdio.h>
 #include <errno.h>		/* System call error return status.  */
 #include <limits.h>
-#include <stdint.h>
+#include <gstdint.h>
 
 #ifdef HAVE_STDDEF_H
 #include <stddef.h>
diff -r e03176e728c7 gdb/gstdint.h
--- a/gdb/gstdint.h	Fri Sep 12 20:42:16 2008 -0700
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,33 +0,0 @@
-/* Copyright (C) 2008 Free Software Foundation, Inc.
-
-   This file is part of GDB.
-
-   This program is free software; you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation; either version 3 of the License, or
-   (at your option) any later version.
-
-   This program is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
-
-/* The libdecnumber library, on which GDB depends, includes a header file
-   called gstdint.h instead of relying directly on stdint.h.  GDB, on the
-   other hand, includes stdint.h directly, relying on the fact that gnulib
-   generates a copy if the system doesn't provide one or if it is missing
-   some features.  Unfortunately, gstdint.h and stdint.h cannot be included
-   at the same time, which may happen when we include a file from
-   libdecnumber.
-
-   The purpose of this file is to override the libdecnumber/gstdint.h file
-   to use stdint.h instead.  Thanks to the order of our include path,
-   files from the GDB source tree have precedence over files from our
-   dependencies.  So the presence of this file will ensure that we
-   consistently use the same stdint.h file when compiling GDB source
-   files.  */
-
-#include <stdint.h>
diff -r e03176e728c7 gdb/Makefile.in
--- a/gdb/Makefile.in	Fri Sep 12 20:42:16 2008 -0700
+++ b/gdb/Makefile.in	Fri Sep 12 21:24:21 2008 -0700
@@ -178,7 +178,7 @@
 
 # Generated headers in the gnulib directory.  These must be listed
 # so that they are generated before other files are compiled.
-GNULIB_H = gnulib/string.h @GNULIB_STDINT_H@
+GNULIB_H = gnulib/string.h
 
 #
 # CLI sub directory definitons
@@ -682,7 +682,7 @@
 cli/cli-decode.h cli/cli-cmds.h cli/cli-utils.h cli/cli-dump.h \
 cli/cli-script.h macrotab.h symtab.h version.h gnulib/wchar.in.h \
 gnulib/string.in.h gnulib/str-two-way.h gnulib/extra/link-warning.h \
-gnulib/stdint.in.h remote.h gdb.h sparc-nat.h gdbserver/win32-low.h \
+remote.h gdb.h sparc-nat.h gdbserver/win32-low.h \
 gdbserver/i387-fp.h gdbserver/server.h gdbserver/terminal.h \
 gdbserver/mem-break.h gdbserver/wincecompat.h gdbserver/target.h \
 gdbserver/linux-low.h gdbserver/gdb_proc_service.h \
diff -r e03176e728c7 gdb/configure.ac
--- a/gdb/configure.ac	Fri Sep 12 20:42:16 2008 -0700
+++ b/gdb/configure.ac	Fri Sep 12 21:24:21 2008 -0700
@@ -76,13 +76,6 @@
 fi
 
 gl_INIT
-
-# For Makefile dependencies.
-GNULIB_STDINT_H=
-if test x"$STDINT_H" != x; then
-  GNULIB_STDINT_H=gnulib/$STDINT_H
-fi
-AC_SUBST(GNULIB_STDINT_H)
 
 PACKAGE=gdb
 AC_DEFINE_UNQUOTED(PACKAGE, "$PACKAGE", [Name of this package. ])

Re: [RFA/RFC] libdecnumber/gstdint.h is back :-(

[Daniel Jacobowitz]

On Fri, Sep 12, 2008 at 10:00:11PM -0700, Joel Brobecker wrote:
> Hello,
> I looked at the C90 draft that I have, and it seems to be saying
> that, whether you use angled brackets or double quotes in your include
> syntax, the way the file is located is implementation defined.
> So, no help there. This needs to be confirmed, but it looks like
> GCC will try to find the include file starting from the same directory
> as the file including it when double quotes are used. In other words,
> when the following directive is used from ../libdecnumber/decContext.h
> 
>    #include "gstdint.h"
> 
> GCC will first try ../libdecnumber/gstdint.h, not ./gstdint.h.
> This defeats the purpose of the gstdint.h that we introduced in
> the gdb subdirectory.

Why does ../libdecnumber/gstdint.h exist?  Is it because you're
configuring in the source directory, and if so can we just tell people
not to do that?

Another option would be to add a check for _GL_STDINT_H to
libdecnumber; I suspect they would not mind.

> What occured to me while thinking about all this is that the
> root of the problem is that we have two incompatible files that
> should be identical in practice. Since we are always building
> libdecnumber, can we use libdecnumber's gstdint.h and ditch
> gnulib's stdint.h?

This could be a problem if we pulled in gnulib modules that
include <stdint.h> from a replacement header.  We already use
it in the building of gnulib (str-two-way.h).

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA/RFC] libdecnumber/gstdint.h is back :-(

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 1601 bytes --]

> Why does ../libdecnumber/gstdint.h exist?  Is it because you're
> configuring in the source directory, and if so can we just tell people
> not to do that?

Yes, I like doing my builds in-tree. I tried changing my habits to
building out-of-tree, but having source files and generated files
in different trees is sometimes a pain, so I ended up reverting to
my old habits.  If this is going to cause too much trouble, then
I'll try to change my habits again. (Note: If this is the case,
then we need to adjust the README file, which still mentions both
options).

> Another option would be to add a check for _GL_STDINT_H to
> libdecnumber; I suspect they would not mind.

In the same category, here is the temporary change I installed
in our tree. I called it a horrible hack, but maybe that was an
overstatement...

> > What occured to me while thinking about all this is that the
> > root of the problem is that we have two incompatible files that
> > should be identical in practice. Since we are always building
> > libdecnumber, can we use libdecnumber's gstdint.h and ditch
> > gnulib's stdint.h?
> 
> This could be a problem if we pulled in gnulib modules that
> include <stdint.h> from a replacement header.  We already use
> it in the building of gnulib (str-two-way.h).

And I am also wondering about building gdbserver without configuring
GDB - that might be a problem too.

I can rework a little the comment, but what do you think of the idea?
I think it's similar to your suggestion, except that the knowledge is
kept on the "client" side rather than the library side.

Thanks,
-- 
Joel

[-- Attachment #2: stdint.diff --]
[-- Type: text/plain, Size: 1795 bytes --]

Modified: gdb/defs.h
===================================================================
--- gdb/defs.h	2008-09-13 01:57:56 UTC (rev 135288)
+++ gdb/defs.h	2008-09-13 04:09:52 UTC (rev 135289)
@@ -30,6 +30,31 @@
 #include <limits.h>
 #include <stdint.h>
 
+/* The libdecnumber library, on which GDB depends, includes a header file
+   called gstdint.h instead of relying directly on stdint.h.  GDB, on the
+   other hand, includes stdint.h directly, relying on the fact that gnulib
+   generates a copy if the system doesn't provide one or if it is missing
+   some features.  Unfortunately, gstdint.h and stdint.h cannot be included
+   at the same time, which may happen when we include a file from
+   libdecnumber.
+
+   The first time we encountered this problem, we thought we had resolved
+   it by having a file named gstdint.h in this directory that would simply
+   contain a #include of stdint.h, thus hoping to "redirect" the gstdint.h
+   include in libdecnumber.  Unfortunately, it turns out that this doesn't
+   work after all, mostly because libdecnumber includes gstdint.h using
+   the double quotes as opposed to the angle brackets.  The search path
+   in implementation defined, but it seems that the gstdint.h files in
+   libdecnumber is picked up after all, thus defeating our trick.
+
+   The following macro definition is a horrible hack that prevents
+   the effective inclusion of gstdint.h, as all the definitions it
+   provides are guarded against GCC_GENERATED_STDINT_H.  We already
+   have gnulib/stdint.h included, so it's ok to blank out gstdint.h.
+   It's horrible but it allows to fix the build issues it gstdint.h
+   causes while we work on a more suitable fix.  */
+#define GCC_GENERATED_STDINT_H 1
+
 #ifdef HAVE_STDDEF_H
 #include <stddef.h>
 #endif

Re: [RFA/RFC] libdecnumber/gstdint.h is back :-(

[Daniel Jacobowitz]

On Sat, Sep 13, 2008 at 10:08:14AM -0700, Joel Brobecker wrote:
> I can rework a little the comment, but what do you think of the idea?
> I think it's similar to your suggestion, except that the knowledge is
> kept on the "client" side rather than the library side.

In my opinion, this patch would be fine.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA/RFC] libdecnumber/gstdint.h is back :-(

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 510 bytes --]

> > I can rework a little the comment, but what do you think of the idea?
> > I think it's similar to your suggestion, except that the knowledge is
> > kept on the "client" side rather than the library side.
> 
> In my opinion, this patch would be fine.

Cool. Here is what I ended up checking in:

2008-09-13  Joel Brobecker  <brobecker@adacore.com>

        * defs.h (GCC_GENERATED_STDINT_H): Define.

Tested on x86-linux. Also verified that GDB builds when configured
in-tree on Tru64.

Thank you!
-- 
Joel

[-- Attachment #2: defs.h.diff --]
[-- Type: text/plain, Size: 1134 bytes --]

Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.236
diff -u -p -r1.236 defs.h
--- defs.h	11 Sep 2008 14:21:49 -0000	1.236
+++ defs.h	14 Sep 2008 06:36:51 -0000
@@ -30,6 +30,20 @@
 #include <limits.h>
 #include <stdint.h>
 
+/* The libdecnumber library, on which GDB depends, includes a header file
+   called gstdint.h instead of relying directly on stdint.h.  GDB, on the
+   other hand, includes stdint.h directly, relying on the fact that gnulib
+   generates a copy if the system doesn't provide one or if it is missing
+   some features.  Unfortunately, gstdint.h and stdint.h cannot be included
+   at the same time, which may happen when we include a file from
+   libdecnumber.
+
+   The following macro definition effectively prevents the inclusion of
+   gstdint.h, as all the definitions it provides are guarded against
+   the GCC_GENERATED_STDINT_H macro.  We already have gnulib/stdint.h
+   included, so it's ok to blank out gstdint.h.  */
+#define GCC_GENERATED_STDINT_H 1
+
 #ifdef HAVE_STDDEF_H
 #include <stddef.h>
 #endif

Re: [RFA/RFC] libdecnumber/gstdint.h is back :-(

[Joel Brobecker]

> Cool. Here is what I ended up checking in:
> 
> 2008-09-13  Joel Brobecker  <brobecker@adacore.com>
> 
>         * defs.h (GCC_GENERATED_STDINT_H): Define.

And I think I we should no longer need gdb/gstdint.h, now, right?

2008-09-14  Joel Brobecker  <brobecker@adacore.com>

        * gstdint.h: Delete.

?

-- 
Joel

Re: [RFA/RFC] libdecnumber/gstdint.h is back :-(

[Daniel Jacobowitz]

On Sun, Sep 14, 2008 at 09:03:46AM -0700, Joel Brobecker wrote:
> > Cool. Here is what I ended up checking in:
> > 
> > 2008-09-13  Joel Brobecker  <brobecker@adacore.com>
> > 
> >         * defs.h (GCC_GENERATED_STDINT_H): Define.
> 
> And I think I we should no longer need gdb/gstdint.h, now, right?
> 
> 2008-09-14  Joel Brobecker  <brobecker@adacore.com>
> 
>         * gstdint.h: Delete.
> 
> ?

Yup.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA/RFC] libdecnumber/gstdint.h is back :-(

[Joel Brobecker]

> > 2008-09-14  Joel Brobecker  <brobecker@adacore.com>
> > 
> >         * gstdint.h: Delete.
> 
> Yup.

Thanks for confirming. File removed.

-- 
Joel