Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Bernd Edlinger <bernd.edlinger@hotmail.de>
Cc: GDB Patches <gdb-patches@sourceware.org>,
	Doug Evans <dje@google.com>,
	       Mike Frysinger <vapier@gentoo.org>
Subject: Re: [PATCH] Enable building GDB without installed libtermcap
Date: Thu, 26 Feb 2015 17:48:00 -0000	[thread overview]
Message-ID: <54EF5C56.9010101@redhat.com> (raw)
In-Reply-To: <DUB118-W469E26146CD54CFEE749FCE4160@phx.gbl>

On 02/24/2015 06:13 PM, Bernd Edlinger wrote:

> Ok, thanks for this hint!
> 
> That is probably exactly what I need.
> 
> This makes the patch much smaller.
> 
> Maybe the name windows-termcap.c is a bit misleading,
> because my target is mostly some embedded linux.
> 

(meanwhile the file has been renamed to stub-termcap.c)

> How about this:
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -621,7 +621,7 @@ esac
>  AC_SEARCH_LIBS(tgetent, [termcap tinfo curses ncurses])
>  
>  if test "$ac_cv_search_tgetent" = no; then
> -  AC_MSG_ERROR([no termcap library found])
> +  CONFIG_OBS="$CONFIG_OBS windows-termcap.o"
>  fi

Yes, but I think we should remove the mingw specific fallback
a bit above too.

>  
>  AC_ARG_WITH([system-readline],
> diff --git a/gdb/windows-termcap.c b/gdb/windows-termcap.c
> index caafc47..b6e0d08 100644
> --- a/gdb/windows-termcap.c
> +++ b/gdb/windows-termcap.c
> @@ -32,6 +32,8 @@ extern char* tgetstr (char *name, char **area);
>  extern int tputs (char *string, int nlines, int (*outfun) ());
>  extern char *tgoto (const char *cap, int col, int row);
>  
> +char PC, *BC, *UP;
> +

I was doing what I suggest above, and tripped on the need for this
too, without realizing you had this here already.  GDB is converting
to C++ (and people build with -fno-common too, e.g., 
https://sourceware.org/ml/gdb-patches/2015-02/msg00364.html), and
there's a problem with just defining these symbols unconditionally
here.  See patch below for details, and let me know what you (all) think.

----
[PATCH] Fallback to stub-termcap.c on all hosts

Currently building gdb is impossible without an installed termcap or
curses library.  But, GDB already has a very minimal termcap in the
tree to handle this situation for Windows -- gdb/stub-termcap.c.  This
patch makes that the fallback for all hosts.

Testing this on GNU/Linux (by simply hacking away the termcap/curses
detection in gdb/configure.ac), I tripped on:

 ../readline/libreadline.a(terminal.o): In function `_rl_init_terminal_io':
 /home/pedro/gdb/mygit/src/readline/terminal.c:527: undefined reference to `PC'
 /home/pedro/gdb/mygit/src/readline/terminal.c:528: undefined reference to `BC'
 /home/pedro/gdb/mygit/src/readline/terminal.c:529: undefined reference to `UP'
 /home/pedro/gdb/mygit/src/readline/terminal.c:538: undefined reference to `PC'
 /home/pedro/gdb/mygit/src/readline/terminal.c:539: undefined reference to `BC'
 /home/pedro/gdb/mygit/src/readline/terminal.c:540: undefined reference to `UP'

These are globals that are normally defined by termcap (or ncurses'
termcap emulation).

Now, we could just define replacements in stub-termcap.c, but
readline/terminal.c (at least the copy in our tree) has this "beauty":

 #if !defined (__linux__) && !defined (NCURSES_VERSION)
 #  if defined (__EMX__) || defined (NEED_EXTERN_PC)
 extern
 #  endif /* __EMX__ || NEED_EXTERN_PC */
 char PC, *BC, *UP;
 #endif /* !__linux__ && !NCURSES_VERSION */

which can result in readline defining the globals too.  That will
usually work out in C, given that "-fcommon" is usually the default
for C compilers, but that won't work for C++, or C with -fno-common
(link fails with "multiple definition" errors)...

Mirroring those #ifdef conditions in the stub termcap screams
"brittle" to me -- I can see them changing in latter readline
versions.

My idea to work around that is to simply use __attribute__((weak)).
Of all supported hosts
(https://sourceware.org/gdb/wiki/Systems#Supported_Hosts),
Windows/PE/COFF would be the one that I'd be worried about WRT use of
weak, but the limited weak support in PE/COFF seems to work here.  A
cross build using x86_64-w64-mingw32 on Fedora 20 builds fine with
this.

gdb/ChangeLog:
2015-02-26  Bernd Edlinger  <bernd.edlinger@hotmail.de>
	    Pedro Alves  <palves@redhat.com>

	* configure.ac: Remove the mingw32-specific stub-termcap.o
	fallback, and instead fallback to the stub termcap on all hosts.
	* configure: Regenerate.
	* stub-termcap.c (PC, BC, UP): Define as weak symbols.
---
 gdb/configure.ac   |  7 +------
 gdb/stub-termcap.c | 13 ++++++++++++-
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/gdb/configure.ac b/gdb/configure.ac
index 6ac8adb..79fc115 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -610,18 +610,13 @@ case $host_os in
   go32* | *djgpp*)
     ac_cv_search_tgetent="none required"
     ;;
-  *mingw32*)
-    if test x"$curses_found" != xyes; then
-      ac_cv_search_tgetent="none required"
-      CONFIG_OBS="$CONFIG_OBS stub-termcap.o"
-    fi ;;
 esac
 
 # These are the libraries checked by Readline.
 AC_SEARCH_LIBS(tgetent, [termcap tinfo curses ncurses])
 
 if test "$ac_cv_search_tgetent" = no; then
-  AC_MSG_ERROR([no termcap library found])
+  CONFIG_OBS="$CONFIG_OBS stub-termcap.o"
 fi
 
 AC_ARG_WITH([system-readline],
diff --git a/gdb/stub-termcap.c b/gdb/stub-termcap.c
index cc8632c..8704695 100644
--- a/gdb/stub-termcap.c
+++ b/gdb/stub-termcap.c
@@ -32,9 +32,20 @@ extern char* tgetstr (char *name, char **area);
 extern int tputs (char *string, int nlines, int (*outfun) ());
 extern char *tgoto (const char *cap, int col, int row);
 
+/* These are global termcap variables that readline references.
+   Actually, depending on preprocessor conditions that we don't want
+   to mirror here (as they may change depending on readline versions),
+   readline may define these globals as well, relying on the linker
+   merging them if needed (-fcommon).  That doesn't work with
+   -fno-common or C++, so instead we define the symbols as weak.  */
+char PC __attribute__((weak));
+char *BC __attribute__((weak));
+char *UP __attribute__((weak));
+
 /* Each of the files below is a minimal implementation of the standard
    termcap function with the same name, suitable for use in a Windows
-   console window.  */
+   console window, or when a real termcap/curses library isn't
+   available.  */
 
 int
 tgetent (char *buffer, char *termtype)
-- 
1.9.3



  parent reply	other threads:[~2015-02-26 17:48 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <DUB118-W1951908395674D538A9BBDE4280@phx.gbl>
2015-02-23 16:06 ` Pedro Alves
2015-02-23 16:18   ` Pedro Alves
2015-02-23 19:27     ` Mike Frysinger
2015-02-23 19:44       ` Eli Zaretskii
2015-02-23 20:33         ` Mike Frysinger
2015-02-23 21:00           ` Eli Zaretskii
2015-02-24 18:18     ` Bernd Edlinger
2015-02-24 19:34       ` Mike Frysinger
2015-02-24 20:29         ` Doug Evans
2015-02-26 17:29           ` Pedro Alves
2015-02-26 17:48       ` Pedro Alves [this message]
2015-02-26 18:00         ` Eli Zaretskii
2015-02-26 18:45           ` Pedro Alves
2015-02-26 18:55             ` Eli Zaretskii
2015-02-26 19:14               ` Pedro Alves
2015-04-02 17:34                 ` Bernd Edlinger
2015-04-06 11:42                   ` Pedro Alves
2015-02-26 19:15         ` Bernd Edlinger

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=54EF5C56.9010101@redhat.com \
    --to=palves@redhat.com \
    --cc=bernd.edlinger@hotmail.de \
    --cc=dje@google.com \
    --cc=gdb-patches@sourceware.org \
    --cc=vapier@gentoo.org \
    /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