Bug 32265 - Import readline 8.2
Summary: Import readline 8.2
Status: RESOLVED FIXED
Alias: None
Product: gdb
Classification: Unclassified
Component: gdb (show other bugs)
Version: unknown
: P2 enhancement
Target Milestone: 16.1
Assignee: Tom Tromey
URL:
Keywords:
: 32457 (view as bug list)
Depends on:
Blocks:
 
Reported: 2024-10-12 07:07 UTC by jeff cliff
Modified: 2024-12-17 23:01 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed: 2024-10-12 00:00:00


Attachments
attachment-2025448-0.html (767 bytes, text/html)
2024-12-15 11:09 UTC, cqwrteur
Details
fix up with information from linux.die (483 bytes, patch)
2024-12-17 04:09 UTC, cqwrteur
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description jeff cliff 2024-10-12 07:07:03 UTC
gdb 15.2
CFLAGS: -std=gnu23 -Oz -march=native
gcc (GCC) 15.0.0 20240509 (experimental)
Readline: 8.2 

1) 

if configure script defines RETSIGTYPE to be void (ie ac_cv_type_signal=void)  it's defined to be void in ./readline/readline/config.h 

but if the configure script *also* does not define VOID_SIGHANDLER ( bash_cv_void_sighandler=no ) 

SIGHANDLER_RETURN is defined to be "return (0)";
but this causes return values for functions like _rl_signal_handler which remain void

this breaks compilation when an int is attempted to be returned but the function returns void (this may have just been a warning before c23)
 
ie this results in 
signals.c: In function ‘_rl_signal_handler’:
signals.c:62:36: error: ‘return’ with a value, in function returning void [-Wreturn-mismatch]
   62 | #  define SIGHANDLER_RETURN return (0)
      |                                    ^
signals.c:163:3: note: in expansion of macro ‘SIGHANDLER_RETURN’
  163 |   SIGHANDLER_RETURN;
      |   ^~~~~~~~~~~~~~~~~
signals.c:140:1: note: declared here
  140 | _rl_signal_handler (int sig)
      | ^~~~~~~~~~~~~~~~~~

one possible fix for that is to specify, when RETSIGTYPE is void but VOID_SIGHANDLER is not defined 

ie something like

#if (RETSIGTYPE) == void
#define VOID_SIGHANDLER 1
#endif
#if defined (VOID_SIGHANDLER)
#  define SIGHANDLER_RETURN return
#else
#  define SIGHANDLER_RETURN return (0)
#endif

at L65 of readline/readline/signals.c

2) 

K&R function declaration syntax is no longer valid in c23 ie for one of the functions in rlmbutil.h

diff -r gdb-15.2/readline/readline/rlmbutil.h gdb-15.2-wip/readline/readline/rlmbutil.h
129,130c129
< _rl_wcwidth (wc)
<      wchar_t wc;
---
> _rl_wcwidth (     wchar_t wc) 


3)

there's also a syntax change for empty parameter lists in c23 so perhaps this is what will be required?

diff -r gdb-15.2/gdb/testsuite/gdb.base/callfuncs.c gdb-15.2-wip/gdb/testsuite/gdb.base/callfuncs.c
23c23
< #define PARAMS(paramlist) ()
---
> #define PARAMS(paramlist) (void)

diff -r gdb-15.2/readline/readline/examples/rlfe/extern.h gdb-15.2-wip/readline/readline/examples/rlfe/extern.h
34c34
< #    define __P(protos) ()
---
> #    define __P(protos) (void)

diff -r gdb-15.2/readline/readline/tilde.h gdb-15.2-wip/readline/readline/tilde.h
38c38
< #    define PARAMS(protos) ()
---
> #    define PARAMS(protos) (void) 

4) even with this, gdb still also fails to compile at 

signals.c: In function ‘rl_sigwinch_handler’:
signals.c:358:32: error: passing argument 2 of ‘rl_set_sighandler’ from incompatible pointer type [-Wincompatible-pointer-types]
  358 |   rl_set_sighandler (SIGWINCH, rl_sigwinch_handler, &dummy_winch);
      |                                ^~~~~~~~~~~~~~~~~~~
      |                                |
      |                                void (*)(int)
In file included from rldefs.h:31,
                 from signals.c:37:
signals.c:94:51: note: expected ‘void (*)(void)’ but argument is of type ‘void (*)(int)’
   94 | static SigHandler *rl_set_sighandler PARAMS((int, SigHandler *, sighandler_cxt *));
      |                                                   ^~~~~~~~~~~~

[I haven't figured how rl_set_sighandler is getting void(*)(int) yet]
Comment 1 Andreas Schwab 2024-10-12 08:13:02 UTC
This has been resolved in readline 8.2 by removing all support for pre-C89.
Comment 2 Tom Tromey 2024-10-12 15:02:32 UTC
Morphing this bug into a to-do for the next readline import.
Thanks Andreas.
Comment 3 Sam James 2024-11-16 06:23:00 UTC
GCC trunk now defaults to -std=gnu23. I did take a look at importing 8.2_p13 but I also see from git log that it has a history of going wrong, so I'd prefer to not do it -- Tom, are you likely to be able to take a look soonish, or should I?

Not against doing it but I just expect to have to analyse gdb testsuite failures and I'm not really familiar enough with gdb internals yet.
Comment 4 Sam James 2024-11-16 06:27:59 UTC
Or maybe we just modify readline/configure.ac to use -std=gnu89 for now.
Comment 5 Tom Tromey 2024-11-16 16:41:01 UTC
I have a patch.
I'll try to maybe document the process a little better this time.
Comment 6 Tom Tromey 2024-11-16 18:01:38 UTC
Going to send some patches upstream as well.
mingw doesn't build.
Comment 8 Tom Tromey 2024-11-16 20:52:02 UTC
https://sourceware.org/pipermail/gdb-patches/2024-November/213381.html

I wonder if patch 1 is too large though.
Comment 9 Tom Tromey 2024-11-16 21:03:19 UTC
I think it would be good to get this in to 16.
Comment 10 jeff cliff 2024-11-18 03:40:40 UTC
>This has been resolved in readline 8.2 

does that include 

gdb-15.2/gdb/testsuite/gdb.base/callfuncs.c 

bit?
Comment 11 Tom Tromey 2024-11-22 23:51:32 UTC
(In reply to jeff cliff from comment #10)
> >This has been resolved in readline 8.2 
> 
> does that include 
> 
> gdb-15.2/gdb/testsuite/gdb.base/callfuncs.c 
> 
> bit?

Nope, sorry, I missed this in your original note.
If it's just the one change in gdb/testsuite, could
you write a commit message for just that and
git send-email it to gdb-patches?
That can go in easily and won't need copyright papers.
Comment 12 vvinayag 2024-12-12 13:55:43 UTC
Is this issue related to the gcc patch that changed the default C standard to gnu23?
https://gcc.gnu.org/pipermail/gcc-patches/2024-November/669031.html

The reason I am asking is that, if I build gcc with the above patch and use that gcc to build GDB, then I am seeing the errors described in this bugzilla report. 


readline/readline/signals.c: In function '_rl_signal_handler':

readline/readline/signals.c:62:36: error: 'return' with a value, in function returning void [-Wreturn-mismatch]

   62 | #  define SIGHANDLER_RETURN return (0)



Should GDB require gnu17 instead of gnu23?
Comment 13 Tom Tromey 2024-12-12 20:50:15 UTC
(In reply to vvinayag from comment #12)
> Is this issue related to the gcc patch that changed the default C standard
> to gnu23?

I think so.

> Should GDB require gnu17 instead of gnu23?

There's not really a need since we're going to land the upgrade
reasonably soon.
Comment 14 Andreas Schwab 2024-12-13 19:12:42 UTC
*** Bug 32457 has been marked as a duplicate of this bug. ***
Comment 15 cqwrteur 2024-12-14 06:57:04 UTC
   does not work
   make[2]: Leaving directory
   '/home/cqwrteur/toolchains_build/toolchainbuildscripts/gccbuild/x86_64-w64-mingw32/.gnuartifacts/x86_64-w64-mingw32/hostbuild/x86_64-w64-mingw32/binutils-gdb/ld'
   no checking for readline in -lreadline... no configure: error: the
   required "readline" library is missing make[1]: *** [Makefile:11825:
   configure-sim] Error 1
   On Dec 13, 2024 14:12, "schwab@linux-m68k.org"
   <sourceware-bugzilla@sourceware.org> wrote:

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

     Andreas Schwab <schwab@linux-m68k.org> changed:

     What |Removed |Added
     ----------------------------------------------------------------------------
     CC| |euloanty at live dot com

     --- Comment #14 from Andreas Schwab <schwab@linux-m68k.org> ---
     *** Bug 32457 has been marked as a duplicate of this bug. ***

     --
     You are receiving this mail because:
     You are on the CC list for the bug.
Comment 16 Tom Tromey 2024-12-14 16:12:19 UTC
>    does not work

If you're building binutils (as in the other bug) you should
--disable-gdb.

If you're building gdb you could try a system readline.
Or wait until the update lands.
Comment 17 cqwrteur 2024-12-15 11:09:46 UTC
Created attachment 15840 [details]
attachment-2025448-0.html

that is not an option at all. i am doing canadian compilation

Get Outlook for Android<https://aka.ms/AAb9ysg>
________________________________
From: tromey at sourceware dot org <sourceware-bugzilla@sourceware.org>
Sent: Saturday, December 14, 2024 11:12:19 AM
To: euloanty@live.com <euloanty@live.com>
Subject: [Bug gdb/32265] Import readline 8.2

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

--- Comment #16 from Tom Tromey <tromey at sourceware dot org> ---
>    does not work

If you're building binutils (as in the other bug) you should
--disable-gdb.

If you're building gdb you could try a system readline.
Or wait until the update lands.

--
You are receiving this mail because:
You are on the CC list for the bug.
Comment 18 Sourceware Commits 2024-12-16 20:15:19 UTC
The master branch has been updated by Tom Tromey <tromey@sourceware.org>:

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

commit 425f843d58c5748f23776759f4f1b55869ba2b89
Author: Tom Tromey <tom@tromey.com>
Date:   Sat Nov 16 10:34:35 2024 -0700

    Import GNU Readline 8.2
    
    This imports readline 8.2 patch 13.
    
    This time around I thought I would try to document the process.
    
    First I have a checkout of the upstream readline repository.  I make a
    local branch there, based on the previous upstream import.  In this
    case that was readline 8.1; see gdb commit b4f26d541aa.
    
    Then, I apply all readline changes from the gdb repository since the
    previous readline import.  In this case that is up to commit
    3dee0baea2e in the gdb repo.
    
    After this, I "git merge" from the relevant upstream commit.  In the
    past I feel like I used a tag, but readline is managed very strangely
    and I didn't see a tag.  So I just used the patch 13 commit, aka
    commit 037d85f1 upstream.
    
    Then I fixed all the merge conflicts.  Re-running autoconf requires a
    symlink from '../../config' into the gdb tree, due to the local
    m4_include addition.  It's possible other hacks like this are
    required, I don't remember how I set things up in the past.
    
    After this, I did a build + test of gdb.  I also did a mingw
    cross-hosted build, because that's caused build failures in past
    imports.
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32265
    Reviewed-by: Sam James <sam@gentoo.org>
Comment 19 Sourceware Commits 2024-12-16 20:15:24 UTC
The master branch has been updated by Tom Tromey <tromey@sourceware.org>:

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

commit 975cb893f45de2a927e226883e7a76ee5e333baa
Author: Tom Tromey <tom@tromey.com>
Date:   Sat Nov 16 13:43:22 2024 -0700

    Fix readline build on mingw
    
    Upstream readline 8.2 does not build on mingw.  This patch fixes the
    build, but I do not know how well it works.
    
    I've submitted this to readline here:
    
        https://lists.gnu.org/archive/html/bug-readline/2024-11/msg00007.html
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32265
Comment 20 Tom Tromey 2024-12-16 20:16:29 UTC
Fixed.
Comment 21 cqwrteur 2024-12-17 03:59:09 UTC
(In reply to Tom Tromey from comment #20)
> Fixed.

/home/cqwrteur/toolchains_build/binutils-gdb/readline/readline/tcap.h:56:14: note: declared here
   56 | extern char *tgoto ();
      |              ^~~~~
/home/cqwrteur/toolchains_build/binutils-gdb/readline/readline/display.c:3256:7: error: too many arguments to function 'tputs'
 3256 |       tputs (buffer, 1, _rl_output_character_function);
      |       ^~~~~
/home/cqwrteur/toolchains_build/binutils-gdb/readline/readline/tcap.h:54:12: note: declared here
   54 | extern int tputs ();


still issues
Comment 22 cqwrteur 2024-12-17 04:01:41 UTC
(In reply to Tom Tromey from comment #20)
> Fixed.


https://github.com/bminor/binutils-gdb/blob/master/readline/readline/tcap.h#L49
Comment 23 cqwrteur 2024-12-17 04:09:08 UTC
Created attachment 15846 [details]
fix up with information from linux.die

https://linux.die.net/man/3/tgetent
Comment 24 rudi 2024-12-17 10:26:35 UTC
Tested https://sourceware.org/pub/gdb/snapshots/current/gdb-weekly-16.0.50.20241217.tar.xz - working with gcc-15 and cross compiling.
Comment 25 Tom Tromey 2024-12-17 23:01:55 UTC
(In reply to cqwrteur from comment #21)

> /home/cqwrteur/toolchains_build/binutils-gdb/readline/readline/tcap.h:56:14:
> note: declared here
>    56 | extern char *tgoto ();

You are missing termcap.h.
Why is that?