Message ID | 1E55DA7D-4F01-49BB-99FF-5A79933415A4@crelg.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 58327 invoked by alias); 13 Sep 2016 21:21:07 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 58293 invoked by uid 89); 13 Sep 2016 21:21:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=1.9 required=5.0 tests=AWL, BAYES_00, EXECUTABLE_URI, KAM_ASCII_DIVIDERS, KAM_EXEURI, RCVD_IN_DNSWL_NONE, SPF_SOFTFAIL autolearn=no version=3.3.2 spammy=hardly, mingw64, mingww64, H*F:U*gdb X-HELO: telf.telf.com Received: from tel.telf.com (HELO telf.telf.com) (192.254.205.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 13 Sep 2016 21:20:56 +0000 Received: from rrcs-70-60-125-182.midsouth.biz.rr.com ([70.60.125.182]:61556 helo=[10.0.9.33]) by telf.telf.com with esmtpsa (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.80.1) (envelope-from <gdb@crelg.com>) id 1bjv8R-0001Gl-2Y for gdb-patches@sourceware.org; Tue, 13 Sep 2016 17:20:55 -0400 From: Giah de Barag <gdb@crelg.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Enabled TUI mode on MSYS2 Message-Id: <1E55DA7D-4F01-49BB-99FF-5A79933415A4@crelg.com> Date: Tue, 13 Sep 2016 17:20:52 -0400 To: GDB Patches <gdb-patches@sourceware.org> Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\)) X-Get-Message-Sender-Via: telf.telf.com: authenticated_id: gdb@crelg.com |
Commit Message
Giah de Barag
Sept. 13, 2016, 9:20 p.m. UTC
2016-09-13 Giah de Barag <gdb@crelg.com> Summary ------- Here is a patch and a build recipe for GDB on MSYS2 to enable TUI mode. The current gdb (7.11.1) in MSYS2 does not support TUI mode. However, GDB TUI mode works fine on MSYS2. It depends upon python, curses, and three environment variables: PYTHONHOME (/mingw32 or /mingw64) and TERMINFO (/usr/share/terminfo) and TERM. Impact ------ This patch only affects builds where (#if defined __MINGW32__) is true. Explanation of Changes ---------------------- GDB works fine in emacs, in bash, and in mintty. In the Windows Console (start cmd.exe) TUI mode works also. TUI mode depends upon ncurses and python. Have them ready before running configure. Eli Zaretski (https://www.sourceware.org/ml/gdb/2014-12/msg00039.html) comments that he had to “hide libncurses from the configure script in order to build a working debugger.” This comment might be dated. GDB with TUI mode works fine on MSYS2 with curses, with this small patch. I saw that getch() (which becomes the ncurses getch() when you link in ncurses) in input.c returns EOF as soon as you hit any character. Fortunately, Windows conio has deprecated getch() and has defined _getch() in its place. So I changed it to _getch(), to access the conio version. There is a tiny issue now that with this conio _getch(), gdb does not properly process backspace characters. Not having backspace characters in plain non-TUI mode in the Windows Console is hardly bothersome, because you can type “tui enable” and have a better environment. If you have to be in non-TUI mode for some reason, a workaround is [Ctrl]-[L] to refresh the line. Therefore I am using conio _getch() not ncurses getch(). Note to GDB Maintainers ----------------------- Someone with understanding of event handling by gdb, Windows conio, and UNIX curses: Can you please look where there is “getch()” in rl_getc() in input.c and see why it returns EOF no matter what (when linking with ncurses). Then do as I did in this patch and change it to _getch() to access the conio version and see why backspace characters are not handled properly. They are processed, but the display is not refreshed, and you have to hit [Ctrl]-[L] to refresh. Also, this is the first time I am doing something like this, so if I am neglecting any rule of communication of this list, please inform me, and I will correct it. MSYS2 GDB Build Recipe ---------------------- # Steps to produce a (debuggable) gdb with TUI mode support on MSYS2: # These steps are tested with Server 2008 R2 which is supposed to be # similar to Windows 7. # Install MSYS2 # see also http://msys2.github.io wget http://repo.msys2.org/distrib/msys2-x86-64-latest.exe # update msys2 pacman --noconfirm -Sy pacman # update pacman (quit mintty and restart) pacman --noconfirm -Syu # update package db (quit mintty and restart) pacman --noconfirm -Su # update packages (repeat until nothing changes) # get a bunch of stuff arch=i686 # OR arch=x86-64 # These are all the packages I currently use. You probably don’t need all # these packages, but you definitely need python and curses. pacman --needed --noconfirm -S base-devel git svn tar zip unzip asciidoc pacman --needed --noconfirm -S mingw-w64-${arch}-emacs pacman --needed --noconfirm -S mingw-w64-${arch}-toolchain pacman --needed --noconfirm -S mingw-w64-${arch}-libxml2 pacman --needed --noconfirm -S mingw-w64-${arch}-gnutls pacman --needed --noconfirm -S mingw-w64-${arch}-libxslt pacman --needed --noconfirm -S mingw-w64-${arch}-libjpeg-turbo pacman --needed --noconfirm -S mingw-w64-${arch}-libtiff pacman --needed --noconfirm -S mingw-w64-${arch}-giflib pacman --needed --noconfirm -S mingw-w64-${arch}-icu pacman --needed --noconfirm -S mingw-w64-${arch}-libsndfile pacman --needed --noconfirm -S mingw-w64-${arch}-aspell pacman --needed --noconfirm -S mingw-w64-${arch}-lcms pacman --needed --noconfirm -S mingw-w64-${arch}-lcms2 pacman --needed --noconfirm -S mingw-w64-${arch}-sqlite3 pacman --needed --noconfirm -S mingw-w64-${arch}-windows-default-manifest # Get and build GDB (same version as in MSYS2 pacman) # see https://www.gnu.org/software/gdb/current/ git clone -b gdb-7.11-branch git://sourceware.org/git/binutils-gdb.git ./configure make # Debug GDB with itself. Use existing gdb in emacs to debug the newly-built # gdb, in tui mode, in a Windows Console. To improve debugging experience, # modify the CFLAGS that you just used to build GDB. cd binutils-gdb/gdb runemacs M-x gdb gdb --annotate=0 -i=mi ./gdb set new-console on set args -tui run Note to MSYS2 Maintainers ------------------------- Is this enough info for you to be able to build an improved GDB that supports TUI mode? Or do you need me to refine down the list of packages that has to be installed before building GDB? Patches ------- Attached is patch relative to gdb-7.11-branch. All it does is change getch() to _getch() in input.c (and remove compiler warnings about implicitly declared Windows conio functions _kbhit() and _getch()). ----- CUT HERE -----
Comments
> From: Giah de Barag <gdb@crelg.com> > Date: Tue, 13 Sep 2016 17:20:52 -0400 > > Here is a patch and a build recipe for GDB on MSYS2 to enable TUI mode. I don't think there's a need for any patches, I regularly build GDB for Windows with TUI support. See the binaries on the ezwinports site. All that's needed is to specify TUI support at configure time. > Eli Zaretski (https://www.sourceware.org/ml/gdb/2014-12/msg00039.html) > comments that he had to “hide libncurses from the configure script in order to > build a working debugger.” This comment might be dated. Yes, it is. The issues that caused me do that were fixed long ago. > GDB with TUI mode works fine on MSYS2 with curses, with this small patch. I saw > that getch() (which becomes the ncurses getch() when you link in ncurses) in > input.c returns EOF as soon as you hit any character. Fortunately, Windows > conio has deprecated getch() and has defined _getch() in its place. So I > changed it to _getch(), to access the conio version. I submitted such a patch to the Readline maintainer many moons ago, and the patch is already in the Readline sources. The problem is that GDB uses its own copy of an old Readline, and all my requests to import a new version of Readline failed. Until GDB does importa a new version of Readline, this (and other) patches I submitted for Readline will not be in GDB. > There is a tiny issue now that with this conio _getch(), gdb does not properly > process backspace characters. I see no such problems in my binaries. > Someone with understanding of event handling by gdb, Windows conio, and UNIX > curses: Can you please look where there is “getch()” in rl_getc() in input.c > and see why it returns EOF no matter what (when linking with ncurses). That's what ncurses' getch does. It's not a bug. GDB simply should not use that function in the MinGW build. Thanks.
> From: Eli Zaretskii <eliz@gnu.org> > Date: Sep 13, 2016, at 22:42 > >> From: Giah de Barag <gdb@crelg.com> >> Date: Tue, 13 Sep 2016 17:20:52 -0400 >> >> Here is a patch and a build recipe for GDB on MSYS2 to enable TUI mode. > > I don't think there's a need for any patches, I regularly build GDB > for Windows with TUI support. See the binaries on the ezwinports > site. All that's needed is to specify TUI support at configure time. Question below refers to this statement. >> Eli Zaretski (https://www.sourceware.org/ml/gdb/2014-12/msg00039.html) >> comments that he had to “hide libncurses from the configure script in order to >> build a working debugger.” This comment might be dated. > > Yes, it is. The issues that caused me do that were fixed long ago. > >> GDB with TUI mode works fine on MSYS2 with curses, with this small patch. I saw >> that getch() (which becomes the ncurses getch() when you link in ncurses) in >> input.c returns EOF as soon as you hit any character. Fortunately, Windows >> conio has deprecated getch() and has defined _getch() in its place. So I >> changed it to _getch(), to access the conio version. > > I submitted such a patch to the Readline maintainer many moons ago, > and the patch is already in the Readline sources. The problem is that > GDB uses its own copy of an old Readline, and all my requests to > import a new version of Readline failed. Until GDB does importa a new > version of Readline, this (and other) patches I submitted for Readline > will not be in GDB. > >> There is a tiny issue now that with this conio _getch(), gdb does not properly >> process backspace characters. > > I see no such problems in my binaries. >> Someone with understanding of event handling by gdb, Windows conio, and UNIX >> curses: Can you please look where there is “getch()” in rl_getc() in input.c >> and see why it returns EOF no matter what (when linking with ncurses). > > That's what ncurses' getch does. It's not a bug. GDB simply should > not use that function in the MinGW build. I do not understand the apparent conflict between this statement, that “GDB simply should not use that function in the MinGW build,” and the statement above, “I don't think there's a need for any patches.” How should one not use that function without affecting the source? Has this patch (changing getch to _getch) already been applied somewhere other than 7.11 (where I happen to be focused)? > Thanks. Thank you.
> From: Giah de Barag <gdb@crelg.com> > Date: Wed, 14 Sep 2016 02:32:09 -0400 > Cc: gdb-patches@sourceware.org > > I do not understand the apparent conflict between this statement, that “GDB simply should not use that function in the MinGW build,” and the statement above, “I don't think there's a need for any patches.” How should one not use that function without affecting the source? Has this patch (changing getch to _getch) already been applied somewhere other than 7.11 (where I happen to be focused)? Sorry for not explaining this clearly enough. Let me try again. The function getch from ncurses can only work when GDB is invoked with the -tui command line option, i.e. when you activate the TUI user interface. Otherwise, it will always return EOF because no text-mode window was created by GDB, and ncurses requires a window before it can take any input. When GDB is linked with ncurses, the getch function call by Readline is resolved to the ncurses version of that function, and not to getch in the MS-Windows runtime library. (The call to getch in Readline is MinGW-specific, so this problem doesn't show on other platforms.) Readline only calls getch when TUI is _not_ in use, so the solution is to replace the call to getch with a call to _getch. The Readline library is maintained separately from GDB; GDB imports Readline into its repository, and has its own copy there. I submitted the patch to use _getch (and several other MinGW-related patches) to the Readline maintainer about 2 years ago, and those patches are already in the latest versions of Readline. Unfortunately, GDB did not yet import those latest versions of Readline, so you don't see the patches in the GDB sources. (When I produce the binaries I use and make available from the ezwinports site, I patch Readline to fix all thos problems.) I hope this explains the issue. The bottom line is that GDB should import a newer version of Readline, and then these problems will be fixed without any need for further patches.
On 09/14/2016 06:21 PM, Eli Zaretskii wrote: > I hope this explains the issue. The bottom line is that GDB should > import a newer version of Readline, and then these problems will be > fixed without any need for further patches. If the readline patch in question is upstream already, I wouldn't object backporting it to our readline copy. The problem with rebasing our readline copy is simply that it requires someone motivated to spend the effort to do it. Patrick Palka tried to import readline 7 a while ago, and the import made it to master even, but then a few problems were discovered and the patch was reverted: https://sourceware.org/ml/gdb-patches/2015-07/msg00759.html There was no movement after that. Thanks, Pedro Alves
> On Sep 14, 2016, at 13:33, Pedro Alves <palves@redhat.comwrote: > >> On 09/14/2016 06:21 PM, Eli Zaretskii wrote: >> >> I hope this explains the issue. The bottom line is that GDB should >> import a newer version of Readline, and then these problems will be >> fixed without any need for further patches. Eli: Yes, I see now. Thank you for that update. That also explains why I’m seeing a problem with the backspace handling. I should import the entire newer readline library not just change getch to _getch. > If the readline patch in question is upstream already, > I wouldn't object backporting it to our readline copy. > > The problem with rebasing our readline copy is simply that it > requires someone motivated to spend the effort to do it. > Patrick Palka tried to import readline 7 a while ago, and the import > made it to master even, but then a few problems were discovered > and the patch was reverted: > > https://sourceware.org/ml/gdb-patches/2015-07/msg00759.html > > There was no movement after that. > > Thanks, > Pedro Alves Pedro: Seeing that Eli is able to build and run GDB with the new readline, we should be able to move forward. I am willing to help. What do you need from us to move this forward?
On 09/14/2016 07:32 PM, Giah de Barag wrote: > Pedro: Seeing that Eli is able to build and run GDB with the new > readline, we should be able to move forward. I am willing to help. > What do you need from us to move this forward? See the url I pointed at and the discussion surrounding it. Someone needs to investigate the issues Patrick originally stumbled on, and come up with some way to fix them. Thanks, Pedro Alves
> Cc: GDB Patches <gdb-patches@sourceware.org> > From: Pedro Alves <palves@redhat.com> > Date: Wed, 14 Sep 2016 19:36:23 +0100 > > On 09/14/2016 07:32 PM, Giah de Barag wrote: > > > Pedro: Seeing that Eli is able to build and run GDB with the new > > readline, we should be able to move forward. I am willing to help. > > What do you need from us to move this forward? > > See the url I pointed at and the discussion surrounding it. > Someone needs to investigate the issues Patrick originally > stumbled on, and come up with some way to fix them. Yes. But if importing a newer Readline is not going to happen soon, I think we should apply my patches to our copy of Readline, to avoid these issues.
On 09/14/2016 07:58 PM, Eli Zaretskii wrote: >> Cc: GDB Patches <gdb-patches@sourceware.org> >> From: Pedro Alves <palves@redhat.com> >> Date: Wed, 14 Sep 2016 19:36:23 +0100 >> >> On 09/14/2016 07:32 PM, Giah de Barag wrote: >> >>> Pedro: Seeing that Eli is able to build and run GDB with the new >>> readline, we should be able to move forward. I am willing to help. >>> What do you need from us to move this forward? >> >> See the url I pointed at and the discussion surrounding it. >> Someone needs to investigate the issues Patrick originally >> stumbled on, and come up with some way to fix them. > > Yes. But if importing a newer Readline is not going to happen soon, I > think we should apply my patches to our copy of Readline, to avoid > these issues. FAOD, as mentioned upthread, that'd be fine with me. Thanks, Pedro Alves
> Cc: gdb@crelg.com, gdb-patches@sourceware.org > From: Pedro Alves <palves@redhat.com> > Date: Wed, 14 Sep 2016 20:04:09 +0100 > > > Yes. But if importing a newer Readline is not going to happen soon, I > > think we should apply my patches to our copy of Readline, to avoid > > these issues. > > FAOD, as mentioned upthread, that'd be fine with me. Done.
> Date: Sat, 17 Sep 2016 11:53:29 +0300 > From: Eli Zaretskii <eliz@gnu.org> > CC: gdb@crelg.com, gdb-patches@sourceware.org > > > Cc: gdb@crelg.com, gdb-patches@sourceware.org > > From: Pedro Alves <palves@redhat.com> > > Date: Wed, 14 Sep 2016 20:04:09 +0100 > > > > > Yes. But if importing a newer Readline is not going to happen soon, I > > > think we should apply my patches to our copy of Readline, to avoid > > > these issues. > > > > FAOD, as mentioned upthread, that'd be fine with me. > > Done. I get messages about my commit broke the build. But I don't understand the nature of the breakage: all of the errors complain about min and max in the 'gdb' directory, whereas I didn't change anything in that directory, and didn't introduce any calls or definitions of min or max. Help will be appreciated. Sorry for the mess.
On 09/17/2016 10:39 AM, Eli Zaretskii wrote: > I get messages about my commit broke the build. But I don't > understand the nature of the breakage: all of the errors complain > about min and max in the 'gdb' directory, whereas I didn't change > anything in that directory, and didn't introduce any calls or > definitions of min or max. Sounds like you're building for 32-bit. My std::min/std::max broke those, sorry about that. I don't know why the buildbot is putting the blame on your patch. Anyway, should be fixed now. Let me know if you still see build problems. > > Help will be appreciated. > > Sorry for the mess. > Thanks, Pedro Alves
On Saturday, September 17 2016, Eli Zaretskii wrote: >> Date: Sat, 17 Sep 2016 11:53:29 +0300 >> From: Eli Zaretskii <eliz@gnu.org> >> CC: gdb@crelg.com, gdb-patches@sourceware.org >> >> > Cc: gdb@crelg.com, gdb-patches@sourceware.org >> > From: Pedro Alves <palves@redhat.com> >> > Date: Wed, 14 Sep 2016 20:04:09 +0100 >> > >> > > Yes. But if importing a newer Readline is not going to happen soon, I >> > > think we should apply my patches to our copy of Readline, to avoid >> > > these issues. >> > >> > FAOD, as mentioned upthread, that'd be fine with me. >> >> Done. > > I get messages about my commit broke the build. But I don't > understand the nature of the breakage: all of the errors complain > about min and max in the 'gdb' directory, whereas I didn't change > anything in that directory, and didn't introduce any calls or > definitions of min or max. > > Help will be appreciated. BTW, I've contacted Eli off-list and will investigate this issue. Thanks,
diff --git a/readline/input.c b/readline/input.c index 7c74c99..57df227 100644 --- a/readline/input.c +++ b/readline/input.c @@ -69,6 +69,10 @@ extern int errno; #include "rlshell.h" #include "xmalloc.h" +#if defined (__MINGW32__) +#include <conio.h> +#endif + /* What kind of non-blocking I/O do we have? */ #if !defined (O_NDELAY) && defined (O_NONBLOCK) # define O_NDELAY O_NONBLOCK /* Posix style */ @@ -190,7 +194,7 @@ rl_gather_tyi () #endif result = -1; -#if defined (FIONREAD) +#if defined (FIONREAD) && !defined (__MINGW32__) errno = 0; result = ioctl (tty, FIONREAD, &chars_avail); if (result == -1 && errno == EIO) @@ -286,7 +290,7 @@ _rl_input_available () fd_set readfds, exceptfds; struct timeval timeout; #endif -#if !defined (HAVE_SELECT) && defined(FIONREAD) +#if !defined (HAVE_SELECT) && defined(FIONREAD) && !defined (__MINGW32__) int chars_avail; #endif int tty; @@ -303,7 +307,7 @@ _rl_input_available () return (select (tty + 1, &readfds, (fd_set *)NULL, &exceptfds, &timeout) > 0); #else -#if defined (FIONREAD) +#if defined (FIONREAD) && !defined (__MINGW32__) if (ioctl (tty, FIONREAD, &chars_avail) == 0) return (chars_avail); #endif @@ -466,6 +470,6 @@ rl_getc (stream) #if defined (__MINGW32__) if (isatty (fileno (stream))) - return (getch ()); + return (_getch ()); /* use conio; ncurses makes tui work, but getch() here returns EOF */ #endif result = read (fileno (stream), &c, sizeof (unsigned char));