[Bug build/17077] New: --with-system-readline uses bundled readline include files

[jan.kratochvil at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17077

Bug ID: 17077
Summary: --with-system-readline uses bundled readline include
files
Product: gdb
Version: HEAD
Status: NEW
Severity: normal
Priority: P2
Component: build
Assignee: unassigned at sourceware dot org
Reporter: jan.kratochvil at redhat dot com
Target: x86_64-linux-gnu

When building tui/tui-io.c one can see in gcc -E still the bundled readline
include files get used.

# 32 "./../opcodes/../readline/rltypedefs.h"
#define _FUNCTION_DEF

As the libreadline.a is linked from system it may lead to incompatible ABI.

I do not have an idea how to fix it. Fedora GDB packaging ensures the build is
correct by "rm -rf readline" in the source tree.

[naesten at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17077

Samuel Bronson <naesten at gmail dot com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |naesten at gmail dot com

--- Comment #1 from Samuel Bronson <naesten at gmail dot com> ---
I've also noticed that --with-system-readline doesn't prevent *building*
readline.

--disable-readline does, but unfortunately the headers still get used :-(.

But, uh, what's the point of using --with-system-readline if you're linking it
statically, anyway?

[jan.kratochvil at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17077

Jan Kratochvil <jan.kratochvil at redhat dot com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |jan.kratochvil at redhat dot com

--- Comment #2 from Jan Kratochvil <jan.kratochvil at redhat dot com> ---
(In reply to Samuel Bronson from comment #1)

Post by naesten at gmail dot com
But, uh, what's the point of using --with-system-readline if you're linking
it statically, anyway?

Fedora links libreadline dynamically. Comment 0 says libreadline.a just for
simplicity, I did not mean specifically the .a variant, .so is also OK.

[dilyan.palauzov at aegee dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=17077

dilyan.palauzov at aegee dot org <dilyan.palauzov at aegee dot org> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |dilyan.palauzov at aegee dot org

--- Comment #3 from dilyan.palauzov at aegee dot org <dilyan.palauzov at aegee dot org> ---
What about removing readline/ from the gdb tarball/version tracking and
insisting of having readline on the system, failing at ./configure time if this
is not the case.

The ones who can compile gdb, shall also be able to compile and install
libreadline.

[jan.kratochvil at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17077

--- Comment #4 from Jan Kratochvil <jan.kratochvil at redhat dot com> ---
That would not work as GDB needs patched readline.

Fortunately most of the patches are for non-Linux systems. But even for Linux
Fedora needs to carry one:
http://pkgs.fedoraproject.org/cgit/readline.git/tree/readline-6.2-gdb.patch

Maybe the required patches are all gone now with readline-6.3 but GDB sources
still bundle readline-6.2:
[PATCH] [RFC] Sync readline to version 6.3 patchlevel 8
https://sourceware.org/ml/gdb-patches/2015-05/msg00352.html
https://sourceware.org/ml/gdb-patches/2015-06/msg00381.html

[jan.kratochvil at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17077

--- Comment #5 from Jan Kratochvil <jan.kratochvil at redhat dot com> ---

Post by dilyan.palauzov at aegee dot org
The ones who can compile gdb, shall also be able to compile and install
libreadline.

Additionally IIRC there was a resistance against any mandatory external
dependencies for the sourceware sourcetree. The reasoning IIRC was that it is
inconvenient for cross-arch bootstraps but GDB does not belong to the
bootstraping compiler chain. So IMO it is wrong but it is so.

[dilyan.palauzov at aegee dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=17077

--- Comment #6 from dilyan.palauzov at aegee dot org <dilyan.palauzov at aegee dot org> ---
Created attachment 8429
--> https://sourceware.org/bugzilla/attachment.cgi?id=8429&action=edit
Include the bundled readline header files, only when the bundled readline is
built.

The provided patch:

-- makes use of the fact, that the preprocessor can differentiate between
#include "file" and #include <file>, when proceeding paths;

-- substitutes all occurrences of #include
"readline/{readline,history,tilde}.h" with #include
<readline/{readline,history,tilde}.h>;

-- enables the project root directory for includes (from opcode),
unconditionally only for #include "file", and for all includes ("",<>), when
the bundled readline is built (using -iquote in place of -I);

-- does not approach the problem for sim/erc32/sis.c.

[jan.kratochvil at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17077

--- Comment #7 from Jan Kratochvil <jan.kratochvil at redhat dot com> ---
I haven't tested it but -iquote seems to be a fix for this Bug. Patches should
be sent to gdb-patches ml.

[dilyan.palauzov at aegee dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=17077

--- Comment #8 from dilyan.palauzov at aegee dot org <dilyan.palauzov at aegee dot org> ---
While -iquote is supported in gcc, clang and Intel's C++, HP aC++ does not
support it.

Another approach would be to use -I- instead, which is supported by HP aC++ and
Oracle's Solaris Studio, but IBM's XL C++ compiler does not support it. For
the latter is stated explicitly, that gxlc, a utility that accepts GNU C or C++
compiler options and translates them into comparable XL C/C++ options, does not
understand -I- and therefore generates a warning.

To sum up, either the -I- option is applied, until somebody complains that a
compiler is not working, or the bintutils-gdb/readline directory is moved to
binutils-gdb/rl/readline (or alike) and -Irl is comnditionally added to
CPPFLAGS.

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=17077

Tom Tromey <tromey at sourceware dot org> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |tromey at sourceware dot org

--- Comment #9 from Tom Tromey <tromey at sourceware dot org> ---
I sent a patch for this.

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=17077

--- Comment #10 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom Tromey <***@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=a8a5dbcab8df0b3a9e04745d4fe8d64740acb323

commit a8a5dbcab8df0b3a9e04745d4fe8d64740acb323
Author: Tom Tromey <***@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

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-10-06 Tom Tromey <***@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.

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=17077

Tom Tromey <tromey at sourceware dot org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |FIXED
Target Milestone|--- |8.3

--- Comment #11 from Tom Tromey <tromey at sourceware dot org> ---
Fixed.