Allow to link with ncursesw

Message ID bb52db91-88bc-164e-bbea-b53a14dde34e@ubuntu.com
State New, archived
Headers

Commit Message

Matthias Klose Sept. 13, 2017, 10:29 a.m. UTC
  Triggered by https://launchpad.net/bugs/1275210, to be able to cope with UTF-8
characters in gdbtui. Ok for the trunk?

Matthias

	* configure.ac: Search ncursesw before ncurses.
	Check ncursesw/ncurses.h before ncurses/ncurses.h.
	* gdb_curses.h: Include <ncursesw/ncurses.h>
	* config.in, configure: Regenerate.
  

Comments

Pedro Alves Sept. 20, 2017, 6:39 p.m. UTC | #1
On 09/13/2017 11:29 AM, Matthias Klose wrote:
> Triggered by https://launchpad.net/bugs/1275210, to be able to cope with UTF-8
> characters in gdbtui. Ok for the trunk?

> 
> Matthias
> 
> 	* configure.ac: Search ncursesw before ncurses.
> 	Check ncursesw/ncurses.h before ncurses/ncurses.h.
> 	* gdb_curses.h: Include <ncursesw/ncurses.h>
> 	* config.in, configure: Regenerate.
> 

This sounds a bit scary if readline (or Python?) links with ncurses.
Looking both upstream readline sources and our local copy in the tree,
I don't see anything checking for ncursesw.  Looking around for
ncurses + ncursesw, I found:

  https://bugs.python.org/issue9408
  https://bugs.python.org/issue7384

"Python3 now links _curses.so to ncurses library (bytes version) instead
of ncursesw library (unicode version) if readline is linked to ncurses."
....
"Thomas Dickey recommended not to load readline+ncurses and ncursesw
at the same time."

And:

 https://bugs.mageia.org/show_bug.cgi?id=2156
 bugs.debian.org/cgi-bin/bugreport.cgi?bug=602720

I see you've been involved with all this before.

Did you reach out to readline/bash, see if they're willing
to try ncursesw before ncurses too?  Don't we need at least
a local patch to our local readline copy, to avoid breaking
those that use it and have it link with ncurses?

Thanks,
Pedro Alves
  
Matthias Klose Sept. 20, 2017, 7:51 p.m. UTC | #2
On 20.09.2017 20:39, Pedro Alves wrote:
> On 09/13/2017 11:29 AM, Matthias Klose wrote:
>> Triggered by https://launchpad.net/bugs/1275210, to be able to cope with UTF-8
>> characters in gdbtui. Ok for the trunk?
> 
>>
>> Matthias
>>
>> 	* configure.ac: Search ncursesw before ncurses.
>> 	Check ncursesw/ncurses.h before ncurses/ncurses.h.
>> 	* gdb_curses.h: Include <ncursesw/ncurses.h>
>> 	* config.in, configure: Regenerate.
>>
> 
> This sounds a bit scary if readline (or Python?) links with ncurses.
> Looking both upstream readline sources and our local copy in the tree,
> I don't see anything checking for ncursesw.  Looking around for
> ncurses + ncursesw, I found:
> 
>   https://bugs.python.org/issue9408
>   https://bugs.python.org/issue7384
> 
> "Python3 now links _curses.so to ncurses library (bytes version) instead
> of ncursesw library (unicode version) if readline is linked to ncurses."
> ....
> "Thomas Dickey recommended not to load readline+ncurses and ncursesw
> at the same time."
> 
> And:
> 
>  https://bugs.mageia.org/show_bug.cgi?id=2156
>  bugs.debian.org/cgi-bin/bugreport.cgi?bug=602720
> 
> I see you've been involved with all this before.
> 
> Did you reach out to readline/bash, see if they're willing
> to try ncursesw before ncurses too?  Don't we need at least
> a local patch to our local readline copy, to avoid breaking
> those that use it and have it link with ncurses?

afaik, this is only the case if readline is linked with one of the curses
libraries.  However these days everybody seems to have readline linked to just
tinfo, so this shouldn't be an issue?

Python itself doesn't link to a curses library, it uses the only which is used
for readline.

Matthias
  
Pedro Alves Sept. 20, 2017, 9:22 p.m. UTC | #3
On 09/20/2017 08:51 PM, Matthias Klose wrote:
> On 20.09.2017 20:39, Pedro Alves wrote:

>> Did you reach out to readline/bash, see if they're willing
>> to try ncursesw before ncurses too?  Don't we need at least
>> a local patch to our local readline copy, to avoid breaking
>> those that use it and have it link with ncurses?
> 
> afaik, this is only the case if readline is linked with one of the curses
> libraries.  However these days everybody seems to have readline linked to just
> tinfo, so this shouldn't be an issue?

Everybody on GNU/Linux, it seems.  However, the Python bug
report talked about FreeBSD's readline linked with ncurses
Do you know whether they've switched to tinfo as well
meanwhile?  [Adding John as FreeBSD maintainer.]

> Python itself doesn't link to a curses library, it uses the only which is used
> for readline.
> 

And if readline doesn't use curses, what does Python do?

Thanks,
Pedro Alves
  
Pedro Alves Sept. 20, 2017, 9:25 p.m. UTC | #4
On 09/20/2017 10:22 PM, Pedro Alves wrote:
> On 09/20/2017 08:51 PM, Matthias Klose wrote:

>> Python itself doesn't link to a curses library, it uses the only which is used
>> for readline.
>>
> 
> And if readline doesn't use curses, what does Python do?

Hmm, maybe Python is not relevant here, because we have,
in gdb/python/py-gdb-readline.c:

 /* Initialize Python readline support.  */

 void
 gdbpy_initialize_gdb_readline (void)
 {
  /* Python's readline module conflicts with GDB's use of readline
     since readline is not reentrant.  Ideally, a reentrant wrapper to
     GDB's readline should be implemented to replace Python's readline
     and prevent conflicts.  For now, this file implements a
     sys.meta_path finder that simply fails to import the readline
     module.  */

That still leaves open the question about non-GNU/Linux OSs, though.

Thanks,
Pedro Alves
  
John Baldwin Sept. 20, 2017, 11:56 p.m. UTC | #5
On Wednesday, September 20, 2017 10:22:29 PM Pedro Alves wrote:
> On 09/20/2017 08:51 PM, Matthias Klose wrote:
> > On 20.09.2017 20:39, Pedro Alves wrote:
> 
> >> Did you reach out to readline/bash, see if they're willing
> >> to try ncursesw before ncurses too?  Don't we need at least
> >> a local patch to our local readline copy, to avoid breaking
> >> those that use it and have it link with ncurses?
> > 
> > afaik, this is only the case if readline is linked with one of the curses
> > libraries.  However these days everybody seems to have readline linked to just
> > tinfo, so this shouldn't be an issue?
> 
> Everybody on GNU/Linux, it seems.  However, the Python bug
> report talked about FreeBSD's readline linked with ncurses
> Do you know whether they've switched to tinfo as well
> meanwhile?  [Adding John as FreeBSD maintainer.]

FreeBSD still links libreadline against curses:

% ldd /usr/local/lib/libreadline.so.7
/usr/local/lib/libreadline.so.7:
        libncursesw.so.8 => /lib/libncursesw.so.8 (0x801251000)
        libc.so.7 => /lib/libc.so.7 (0x800825000)

However, it seems to be linked against libncursesw.  gdb on this same
system (11.1) is linked against both ncurses libraries:

% ldd /usr/local/bin/gdb80 
/usr/local/bin/gdb80:
        libreadline.so.7 => /usr/local/lib/libreadline.so.7 (0x801dda000)
        libncurses.so.8 => /lib/libncurses.so.8 (0x80202b000)
        libkvm.so.7 => /lib/libkvm.so.7 (0x802281000)
        libutil.so.9 => /lib/libutil.so.9 (0x80248f000)
        libm.so.5 => /lib/libm.so.5 (0x8026a3000)
        libthr.so.3 => /lib/libthr.so.3 (0x8028cd000)
        libintl.so.8 => /usr/local/lib/libintl.so.8 (0x802af5000)
        libpython2.7.so.1 => /usr/local/lib/libpython2.7.so.1 (0x802d00000)
        libexpat.so.1 => /usr/local/lib/libexpat.so.1 (0x8030e3000)
        liblzma.so.5 => /usr/lib/liblzma.so.5 (0x80330e000)
        libc++.so.1 => /usr/lib/libc++.so.1 (0x803537000)
        libcxxrt.so.1 => /lib/libcxxrt.so.1 (0x803801000)
        libgcc_s.so.1 => /lib/libgcc_s.so.1 (0x803a20000)
        libc.so.7 => /lib/libc.so.7 (0x803c2f000)
        libncursesw.so.8 => /lib/libncursesw.so.8 (0x803fec000)
        libelf.so.2 => /lib/libelf.so.2 (0x80424b000)

Given that readline on FreeBSD now uses ncursesw and that the original
python report was from several years ago when that probably wasn't true,
I'm inclined to think that using ncursesw might be fine.  I'll try a test
build of this patch tomorrow.
  
Pedro Alves Sept. 22, 2017, 10:21 a.m. UTC | #6
On 09/21/2017 12:56 AM, John Baldwin wrote:
> On Wednesday, September 20, 2017 10:22:29 PM Pedro Alves wrote:
>> On 09/20/2017 08:51 PM, Matthias Klose wrote:
>>> On 20.09.2017 20:39, Pedro Alves wrote:
>>
>>>> Did you reach out to readline/bash, see if they're willing
>>>> to try ncursesw before ncurses too?  Don't we need at least
>>>> a local patch to our local readline copy, to avoid breaking
>>>> those that use it and have it link with ncurses?
>>>
>>> afaik, this is only the case if readline is linked with one of the curses
>>> libraries.  However these days everybody seems to have readline linked to just
>>> tinfo, so this shouldn't be an issue?
>>
>> Everybody on GNU/Linux, it seems.  However, the Python bug
>> report talked about FreeBSD's readline linked with ncurses
>> Do you know whether they've switched to tinfo as well
>> meanwhile?  [Adding John as FreeBSD maintainer.]
> 
> FreeBSD still links libreadline against curses:
> 
> % ldd /usr/local/lib/libreadline.so.7
> /usr/local/lib/libreadline.so.7:
>         libncursesw.so.8 => /lib/libncursesw.so.8 (0x801251000)
>         libc.so.7 => /lib/libc.so.7 (0x800825000)
> 
> However, it seems to be linked against libncursesw.  gdb on this same
> system (11.1) is linked against both ncurses libraries:
> 
> % ldd /usr/local/bin/gdb80 
> /usr/local/bin/gdb80:
>         libreadline.so.7 => /usr/local/lib/libreadline.so.7 (0x801dda000)
>         libncurses.so.8 => /lib/libncurses.so.8 (0x80202b000)
>         libkvm.so.7 => /lib/libkvm.so.7 (0x802281000)
>         libutil.so.9 => /lib/libutil.so.9 (0x80248f000)
>         libm.so.5 => /lib/libm.so.5 (0x8026a3000)
>         libthr.so.3 => /lib/libthr.so.3 (0x8028cd000)
>         libintl.so.8 => /usr/local/lib/libintl.so.8 (0x802af5000)
>         libpython2.7.so.1 => /usr/local/lib/libpython2.7.so.1 (0x802d00000)
>         libexpat.so.1 => /usr/local/lib/libexpat.so.1 (0x8030e3000)
>         liblzma.so.5 => /usr/lib/liblzma.so.5 (0x80330e000)
>         libc++.so.1 => /usr/lib/libc++.so.1 (0x803537000)
>         libcxxrt.so.1 => /lib/libcxxrt.so.1 (0x803801000)
>         libgcc_s.so.1 => /lib/libgcc_s.so.1 (0x803a20000)
>         libc.so.7 => /lib/libc.so.7 (0x803c2f000)
>         libncursesw.so.8 => /lib/libncursesw.so.8 (0x803fec000)
>         libelf.so.2 => /lib/libelf.so.2 (0x80424b000)
> 
> Given that readline on FreeBSD now uses ncursesw and that the original
> python report was from several years ago when that probably wasn't true,
> I'm inclined to think that using ncursesw might be fine.  I'll try a test
> build of this patch tomorrow.
> 

Thanks!  If you could also confirm with the in-tree readline
as well (i.e., without --with-system-readline), that'd be great.  The
original Python reports I linked at were about readline compiled against
ncurses and then something else linked against ncursesw, IIUC.  I don't
know whether that might make a difference, but I guess it could, if
readline is compiled against some curses structure that ends up
mismatching with the called curses functions due to odd interposition
effects.

If that causes a problem, then we may need to patch our local
readline copy matching gdb.

Otherwise, if all is fine, then I think we've given this
due diligence and Matthias' patch is fine with me.

Thanks,
Pedro Alves
  
John Baldwin Sept. 25, 2017, 5:36 p.m. UTC | #7
On Friday, September 22, 2017 11:21:58 AM Pedro Alves wrote:
> 
> On 09/21/2017 12:56 AM, John Baldwin wrote:
> > On Wednesday, September 20, 2017 10:22:29 PM Pedro Alves wrote:
> >> On 09/20/2017 08:51 PM, Matthias Klose wrote:
> >>> On 20.09.2017 20:39, Pedro Alves wrote:
> >>
> >>>> Did you reach out to readline/bash, see if they're willing
> >>>> to try ncursesw before ncurses too?  Don't we need at least
> >>>> a local patch to our local readline copy, to avoid breaking
> >>>> those that use it and have it link with ncurses?
> >>>
> >>> afaik, this is only the case if readline is linked with one of the curses
> >>> libraries.  However these days everybody seems to have readline linked to just
> >>> tinfo, so this shouldn't be an issue?
> >>
> >> Everybody on GNU/Linux, it seems.  However, the Python bug
> >> report talked about FreeBSD's readline linked with ncurses
> >> Do you know whether they've switched to tinfo as well
> >> meanwhile?  [Adding John as FreeBSD maintainer.]
> > 
> > FreeBSD still links libreadline against curses:
> > 
> > % ldd /usr/local/lib/libreadline.so.7
> > /usr/local/lib/libreadline.so.7:
> >         libncursesw.so.8 => /lib/libncursesw.so.8 (0x801251000)
> >         libc.so.7 => /lib/libc.so.7 (0x800825000)
> > 
> > However, it seems to be linked against libncursesw.  gdb on this same
> > system (11.1) is linked against both ncurses libraries:
> > 
> > % ldd /usr/local/bin/gdb80 
> > /usr/local/bin/gdb80:
> >         libreadline.so.7 => /usr/local/lib/libreadline.so.7 (0x801dda000)
> >         libncurses.so.8 => /lib/libncurses.so.8 (0x80202b000)
> >         libkvm.so.7 => /lib/libkvm.so.7 (0x802281000)
> >         libutil.so.9 => /lib/libutil.so.9 (0x80248f000)
> >         libm.so.5 => /lib/libm.so.5 (0x8026a3000)
> >         libthr.so.3 => /lib/libthr.so.3 (0x8028cd000)
> >         libintl.so.8 => /usr/local/lib/libintl.so.8 (0x802af5000)
> >         libpython2.7.so.1 => /usr/local/lib/libpython2.7.so.1 (0x802d00000)
> >         libexpat.so.1 => /usr/local/lib/libexpat.so.1 (0x8030e3000)
> >         liblzma.so.5 => /usr/lib/liblzma.so.5 (0x80330e000)
> >         libc++.so.1 => /usr/lib/libc++.so.1 (0x803537000)
> >         libcxxrt.so.1 => /lib/libcxxrt.so.1 (0x803801000)
> >         libgcc_s.so.1 => /lib/libgcc_s.so.1 (0x803a20000)
> >         libc.so.7 => /lib/libc.so.7 (0x803c2f000)
> >         libncursesw.so.8 => /lib/libncursesw.so.8 (0x803fec000)
> >         libelf.so.2 => /lib/libelf.so.2 (0x80424b000)
> > 
> > Given that readline on FreeBSD now uses ncursesw and that the original
> > python report was from several years ago when that probably wasn't true,
> > I'm inclined to think that using ncursesw might be fine.  I'll try a test
> > build of this patch tomorrow.
> > 
> 
> Thanks!  If you could also confirm with the in-tree readline
> as well (i.e., without --with-system-readline), that'd be great.  The
> original Python reports I linked at were about readline compiled against
> ncurses and then something else linked against ncursesw, IIUC.  I don't
> know whether that might make a difference, but I guess it could, if
> readline is compiled against some curses structure that ends up
> mismatching with the called curses functions due to odd interposition
> effects.
> 
> If that causes a problem, then we may need to patch our local
> readline copy matching gdb.
> 
> Otherwise, if all is fine, then I think we've given this
> due diligence and Matthias' patch is fine with me.

This seems to work fine in my testing both in the port (which with this
change now only links against libncursesw instead of both) and in my own
builds (which do not use --with-system-readline).
  
Pedro Alves Sept. 26, 2017, 3:26 p.m. UTC | #8
On 09/25/2017 06:36 PM, John Baldwin wrote:
> On Friday, September 22, 2017 11:21:58 AM Pedro Alves wrote:
>>
>> On 09/21/2017 12:56 AM, John Baldwin wrote:

>> Thanks!  If you could also confirm with the in-tree readline
>> as well (i.e., without --with-system-readline), that'd be great.  The
>> original Python reports I linked at were about readline compiled against
>> ncurses and then something else linked against ncursesw, IIUC.  I don't
>> know whether that might make a difference, but I guess it could, if
>> readline is compiled against some curses structure that ends up
>> mismatching with the called curses functions due to odd interposition
>> effects.
>>
>> If that causes a problem, then we may need to patch our local
>> readline copy matching gdb.
>>
>> Otherwise, if all is fine, then I think we've given this
>> due diligence and Matthias' patch is fine with me.
> 
> This seems to work fine in my testing both in the port (which with this
> change now only links against libncursesw instead of both) and in my own
> builds (which do not use --with-system-readline).

Thanks much!  I've merged the patch to master, then.

Thanks,
Pedro Alves
  

Patch


	* configure.ac: Search ncursesw before ncurses.
	Check ncursesw/ncurses.h before ncurses/ncurses.h.
	* gdb_curses.h: Include <ncursesw/ncurses.h>
	* config.in, configure: Regenerate.

Index: b/gdb/configure.ac
===================================================================
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -569,7 +569,7 @@  if test x"$prefer_curses" = xyes; then
   # search /usr/local/include, if ncurses is installed in /usr/local.  A
   # default installation of ncurses on alpha*-dec-osf* will lead to such
   # a situation.
-  AC_SEARCH_LIBS(waddstr, [ncurses cursesX curses])
+  AC_SEARCH_LIBS(waddstr, [ncursesw ncurses cursesX curses])
 
   if test "$ac_cv_search_waddstr" != no; then
     curses_found=yes
@@ -611,7 +611,7 @@  case $host_os in
 esac
 
 # These are the libraries checked by Readline.
-AC_SEARCH_LIBS(tgetent, [termcap tinfo curses ncurses])
+AC_SEARCH_LIBS(tgetent, [termcap tinfo curses ncursesw ncurses])
 
 if test "$ac_cv_search_tgetent" = no; then
   CONFIG_OBS="$CONFIG_OBS stub-termcap.o"
@@ -1314,7 +1314,7 @@  case $host_os in
    Solaris 2.[789] when using GCC. ])
     fi ;;
 esac
-AC_CHECK_HEADERS(curses.h cursesX.h ncurses.h ncurses/ncurses.h ncurses/term.h)
+AC_CHECK_HEADERS(curses.h cursesX.h ncurses.h ncursesw/ncurses.h ncurses/ncurses.h ncurses/term.h)
 AC_CHECK_HEADERS(term.h, [], [],
 [#if HAVE_CURSES_H
 # include <curses.h>
Index: b/gdb/config.in
===================================================================
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -300,6 +300,9 @@ 
 /* Define to 1 if you have the `monstartup' function. */
 #undef HAVE_MONSTARTUP
 
+/* Define to 1 if you have the <ncursesw/ncurses.h> header file. */
+#undef HAVE_NCURSESW_NCURSES_H
+
 /* Define to 1 if you have the <ncurses.h> header file. */
 #undef HAVE_NCURSES_H
 
Index: b/gdb/gdb_curses.h
===================================================================
--- a/gdb/gdb_curses.h
+++ b/gdb/gdb_curses.h
@@ -39,7 +39,9 @@ 
 #define NOMACROS
 #define NCURSES_NOMACROS
 
-#if defined (HAVE_NCURSES_NCURSES_H)
+#if defined (HAVE_NCURSESW_NCURSES_H)
+#include <ncursesw/ncurses.h>
+#elif defined (HAVE_NCURSES_NCURSES_H)
 #include <ncurses/ncurses.h>
 #elif defined (HAVE_NCURSES_H)
 #include <ncurses.h>
Index: b/gdb/configure
===================================================================
--- a/gdb/configure
+++ b/gdb/configure
@@ -8796,7 +8796,7 @@  return waddstr ();
   return 0;
 }
 _ACEOF
-for ac_lib in '' ncurses cursesX curses; do
+for ac_lib in '' ncursesw ncurses cursesX curses; do
   if test -z "$ac_lib"; then
     ac_res="none required"
   else
@@ -8894,7 +8894,7 @@  return tgetent ();
   return 0;
 }
 _ACEOF
-for ac_lib in '' termcap tinfo curses ncurses; do
+for ac_lib in '' termcap tinfo curses ncursesw ncurses; do
   if test -z "$ac_lib"; then
     ac_res="none required"
   else
@@ -11713,7 +11713,7 @@  $as_echo "#define _MSE_INT_H 1" >>confde
 
     fi ;;
 esac
-for ac_header in curses.h cursesX.h ncurses.h ncurses/ncurses.h ncurses/term.h
+for ac_header in curses.h cursesX.h ncurses.h ncursesw/ncurses.h ncurses/ncurses.h ncurses/term.h
 do :
   as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
 ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header" "$ac_includes_default"
@@ -13179,7 +13179,7 @@  $as_echo "#define HAVE_LANGINFO_CODESET
   fi
 
 
-  for ac_header in linux/perf_event.h locale.h memory.h signal.h 		   sys/resource.h sys/socket.h sys/syscall.h 		   sys/un.h sys/wait.h 		   thread_db.h wait.h
+  for ac_header in linux/perf_event.h locale.h memory.h signal.h 		   sys/resource.h sys/socket.h sys/syscall.h 		   sys/un.h sys/wait.h 		   thread_db.h wait.h 		   termios.h termio.h sgtty.h
 do :
   as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
 ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header" "$ac_includes_default"