GDB 12.0.90 available for testing

GDB 12.0.90 available for testing

[Joel Brobecker]

Hello,

I have just finished creating the gdb-12.0.90 pre-release.
It is available for download at the following location:

    ftp://sourceware.org/pub/gdb/snapshots/branch/gdb-12.0.90.tar.xz

A gzip'ed version is also available: gdb-12.0.90.tar.gz.

Please give it a test if you can and report any problems you might find.

On behalf of all the GDB contributors, thank you!
-- 
Joel Brobecker

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Sun, 20 Mar 2022 09:58:15 +0400 (+04)
> From: Joel Brobecker via Gdb-patches <gdb-patches@sourceware.org>
> 
> I have just finished creating the gdb-12.0.90 pre-release.
> It is available for download at the following location:
> 
>     ftp://sourceware.org/pub/gdb/snapshots/branch/gdb-12.0.90.tar.xz
> 
> A gzip'ed version is also available: gdb-12.0.90.tar.gz.
> 
> Please give it a test if you can and report any problems you might find.

I'm just starting to look at this pretest, but one thing I already see
is that Gnulib's import was not updated.  The upstream Gnulib includes
several fixes for problems I reported against the version that came
with GDB 11.1, but those fixes are not in gnulib/import/.

Was that done intentionally?  If so, what is the reason for not
updating Gnulib?  (I'm not insisting on the update, just asking.  If
this is not an omission, I can easily apply the same patches I did in
GDB 11.1 when building.)

Thanks.

Re: GDB 12.0.90 available for testing

[Joel Brobecker]

Hi Eli,

> > Date: Sun, 20 Mar 2022 09:58:15 +0400 (+04)
> > From: Joel Brobecker via Gdb-patches <gdb-patches@sourceware.org>
> > 
> > I have just finished creating the gdb-12.0.90 pre-release.
> > It is available for download at the following location:
> > 
> >     ftp://sourceware.org/pub/gdb/snapshots/branch/gdb-12.0.90.tar.xz
> > 
> > A gzip'ed version is also available: gdb-12.0.90.tar.gz.
> > 
> > Please give it a test if you can and report any problems you might find.
> 
> I'm just starting to look at this pretest, but one thing I already see
> is that Gnulib's import was not updated.  The upstream Gnulib includes
> several fixes for problems I reported against the version that came
> with GDB 11.1, but those fixes are not in gnulib/import/.
> 
> Was that done intentionally?  If so, what is the reason for not
> updating Gnulib?  (I'm not insisting on the update, just asking.  If
> this is not an omission, I can easily apply the same patches I did in
> GDB 11.1 when building.)

My understanding is that gnulib updates in GDB are performed only
as needed, by the people who need it.

-- 
Joel

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Sat, 26 Mar 2022 19:53:12 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Joel Brobecker <brobecker@adacore.com>, gdb-patches@sourceware.org
> 
> > >     ftp://sourceware.org/pub/gdb/snapshots/branch/gdb-12.0.90.tar.xz
> > > 
> > > A gzip'ed version is also available: gdb-12.0.90.tar.gz.
> > > 
> > > Please give it a test if you can and report any problems you might find.
> > 
> > I'm just starting to look at this pretest, but one thing I already see
> > is that Gnulib's import was not updated.  The upstream Gnulib includes
> > several fixes for problems I reported against the version that came
> > with GDB 11.1, but those fixes are not in gnulib/import/.
> > 
> > Was that done intentionally?  If so, what is the reason for not
> > updating Gnulib?  (I'm not insisting on the update, just asking.  If
> > this is not an omission, I can easily apply the same patches I did in
> > GDB 11.1 when building.)
> 
> My understanding is that gnulib updates in GDB are performed only
> as needed, by the people who need it.

Fine with me, thanks.

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Sun, 20 Mar 2022 09:58:15 +0400 (+04)
> From: Joel Brobecker via Gdb-patches <gdb-patches@sourceware.org>
> 
> Hello,
> 
> I have just finished creating the gdb-12.0.90 pre-release.
> It is available for download at the following location:
> 
>     ftp://sourceware.org/pub/gdb/snapshots/branch/gdb-12.0.90.tar.xz
> 
> A gzip'ed version is also available: gdb-12.0.90.tar.gz.
> 
> Please give it a test if you can and report any problems you might find.

I've built this pretest with MinGW on MS-Windows.  It generally builds
cleanly, but I found 2 issues.

First, there's this compilation warning when compiling infrun.c:

       CXX    infrun.o
     In file included from btrace.h:30,
		      from gdbthread.h:29,
		      from infrun.h:21,
		      from infrun.c:23:
     target/waitstatus.h: In function 'void stop_all_threads()':
     target/waitstatus.h:175:13: warning: 'ws.target_waitstatus::m_value' may be used uninitialized in this function [-Wmaybe-uninitialized]
       175 |     m_value = other.m_value;
	   |     ~~~~~~~~^~~~~~~~~~~~~~~

Is this a real problem?

Second, one of the selftests fails:

  Running selftest dw2_expand_symtabs_matching.
  warning: charset conversion failure for 'u8função'.
  You may have the wrong value for 'set ada source-charset'.
  warning: could not convert 'yfunc ' from the host encoding (CP1255) to UTF-32.
  This normally should not happen, please file a bug report.

AFAIU, this is because the names of these two functions are,
respectively, in UTF-8 and in Latin-1, but the charset conversion
thinks they are in CP1255.  Where does the test tell the conversion
functions what is the source encoding?

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Sat, 26 Mar 2022 20:59:04 +0300
> From: Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org>
> Cc: gdb-patches@sourceware.org
> 
> First, there's this compilation warning when compiling infrun.c:
> 
>        CXX    infrun.o
>      In file included from btrace.h:30,
> 		      from gdbthread.h:29,
> 		      from infrun.h:21,
> 		      from infrun.c:23:
>      target/waitstatus.h: In function 'void stop_all_threads()':
>      target/waitstatus.h:175:13: warning: 'ws.target_waitstatus::m_value' may be used uninitialized in this function [-Wmaybe-uninitialized]
>        175 |     m_value = other.m_value;
> 	   |     ~~~~~~~~^~~~~~~~~~~~~~~
> 
> Is this a real problem?
> 
> Second, one of the selftests fails:
> 
>   Running selftest dw2_expand_symtabs_matching.
>   warning: charset conversion failure for 'u8função'.
>   You may have the wrong value for 'set ada source-charset'.
>   warning: could not convert 'yfunc ' from the host encoding (CP1255) to UTF-32.
>   This normally should not happen, please file a bug report.
> 
> AFAIU, this is because the names of these two functions are,
> respectively, in UTF-8 and in Latin-1, but the charset conversion
> thinks they are in CP1255.  Where does the test tell the conversion
> functions what is the source encoding?

And another problem: starting this GDB under Emacs, with "M-x gdb"
(which invokes "gdb -i=mi") seems to confuse Emacs, so much so that
the debugging session is barely usable: symbols are unknown, so I
cannot set breakpoints, etc.

There's no such problem with GDB 11.1.

I wonder if this is a Windows specific issue.  Did someone try GDB 12
inside Emacs on GNU/Linux, and if so, did it work as expected?

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Sat, 26 Mar 2022 21:34:32 +0300
> X-Spam-Status: No, score=-0.3 required=5.0 tests=BAYES_00, BODY_8BITS,
>  DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF,
>  RCVD_IN_BARRACUDACENTRAL, SPF_HELO_PASS, SPF_PASS, TXREP,
>  T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.4
> From: Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org>
> Cc: gdb-patches@sourceware.org
> 
> > Date: Sat, 26 Mar 2022 20:59:04 +0300
> > From: Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org>
> > Cc: gdb-patches@sourceware.org
> > 
> > First, there's this compilation warning when compiling infrun.c:
> > 
> >        CXX    infrun.o
> >      In file included from btrace.h:30,
> > 		      from gdbthread.h:29,
> > 		      from infrun.h:21,
> > 		      from infrun.c:23:
> >      target/waitstatus.h: In function 'void stop_all_threads()':
> >      target/waitstatus.h:175:13: warning: 'ws.target_waitstatus::m_value' may be used uninitialized in this function [-Wmaybe-uninitialized]
> >        175 |     m_value = other.m_value;
> > 	   |     ~~~~~~~~^~~~~~~~~~~~~~~
> > 
> > Is this a real problem?
> > 
> > Second, one of the selftests fails:
> > 
> >   Running selftest dw2_expand_symtabs_matching.
> >   warning: charset conversion failure for 'u8função'.
> >   You may have the wrong value for 'set ada source-charset'.
> >   warning: could not convert 'yfunc ' from the host encoding (CP1255) to UTF-32.
> >   This normally should not happen, please file a bug report.
> > 
> > AFAIU, this is because the names of these two functions are,
> > respectively, in UTF-8 and in Latin-1, but the charset conversion
> > thinks they are in CP1255.  Where does the test tell the conversion
> > functions what is the source encoding?
> 
> And another problem: starting this GDB under Emacs, with "M-x gdb"
> (which invokes "gdb -i=mi") seems to confuse Emacs, so much so that
> the debugging session is barely usable: symbols are unknown, so I
> cannot set breakpoints, etc.

And yet another issue: if I start GDB with the MI interface:

   gdb -i=mi PROGRAM

then whatever I type at the prompt GDB thinks I typed "g":

  (gdb)
  r -Q
  &"g\n"
  &"Ambiguous command \"g\": gcore, generate-core-file, goto-bookmark, gr, gu, guile, guile-repl.\n"
  ^error,msg="Ambiguous command \"g\": gcore, generate-core-file, goto-bookmark, gr, gu, guile, guile-repl."
  (gdb)

As you see, I typed "r -Q", but GDB thinks I typed "g".

Any idea how to debug this?

(There's no such problem if I invoke GDB with the CLI interpreter.)

Re: GDB 12.0.90 available for testing

[Simon Marchi]

> I've built this pretest with MinGW on MS-Windows.  It generally builds
> cleanly, but I found 2 issues.
> 
> First, there's this compilation warning when compiling infrun.c:
> 
>        CXX    infrun.o
>      In file included from btrace.h:30,
> 		      from gdbthread.h:29,
> 		      from infrun.h:21,
> 		      from infrun.c:23:
>      target/waitstatus.h: In function 'void stop_all_threads()':
>      target/waitstatus.h:175:13: warning: 'ws.target_waitstatus::m_value' may be used uninitialized in this function [-Wmaybe-uninitialized]
>        175 |     m_value = other.m_value;
> 	   |     ~~~~~~~~^~~~~~~~~~~~~~~
> 
> Is this a real problem?

I think this one is not really a problem.  It complains about this code:

	  target_waitstatus ws;
	  ws.set_no_resumed ();
	  return {NULL, minus_one_ptid, std::move (ws)};

When settnig `ws` using `set_no_resumed`, the union field ws::m_value is
left untouched, meaning it contains random bytes.  When we move `ws`, we
copy random bytes.  But that doesn't matter, because the destination
target_waitstatus is of kind TARGET_WAITKIND_NO_RESUMED, so doesn't care
about m_value.

Simon

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Sat, 26 Mar 2022 21:55:38 -0400
> Cc: gdb-patches@sourceware.org
> From: Simon Marchi <simon.marchi@polymtl.ca>
> 
> >        CXX    infrun.o
> >      In file included from btrace.h:30,
> > 		      from gdbthread.h:29,
> > 		      from infrun.h:21,
> > 		      from infrun.c:23:
> >      target/waitstatus.h: In function 'void stop_all_threads()':
> >      target/waitstatus.h:175:13: warning: 'ws.target_waitstatus::m_value' may be used uninitialized in this function [-Wmaybe-uninitialized]
> >        175 |     m_value = other.m_value;
> > 	   |     ~~~~~~~~^~~~~~~~~~~~~~~
> > 
> > Is this a real problem?
> 
> I think this one is not really a problem.  It complains about this code:
> 
> 	  target_waitstatus ws;
> 	  ws.set_no_resumed ();
> 	  return {NULL, minus_one_ptid, std::move (ws)};
> 
> When settnig `ws` using `set_no_resumed`, the union field ws::m_value is
> left untouched, meaning it contains random bytes.  When we move `ws`, we
> copy random bytes.  But that doesn't matter, because the destination
> target_waitstatus is of kind TARGET_WAITKIND_NO_RESUMED, so doesn't care
> about m_value.

Maybe we should initialize ws::m_value, just to shut up the compiler?
Because GDB 12 compiles cleanly other than this warning.

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Sat, 26 Mar 2022 21:51:49 +0300
> From: Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org>
> Cc: gdb-patches@sourceware.org
> 
> And yet another issue: if I start GDB with the MI interface:
> 
>    gdb -i=mi PROGRAM
> 
> then whatever I type at the prompt GDB thinks I typed "g":
> 
>   (gdb)
>   r -Q
>   &"g\n"
>   &"Ambiguous command \"g\": gcore, generate-core-file, goto-bookmark, gr, gu, guile, guile-repl.\n"
>   ^error,msg="Ambiguous command \"g\": gcore, generate-core-file, goto-bookmark, gr, gu, guile, guile-repl."
>   (gdb)
> 
> As you see, I typed "r -Q", but GDB thinks I typed "g".

This is now a Bugzilla PR 29002.  I posted my proposed solution there,
see

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

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Sat, 26 Mar 2022 21:34:32 +0300
> From: Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org>
> Cc: gdb-patches@sourceware.org
> 
> And another problem: starting this GDB under Emacs, with "M-x gdb"
> (which invokes "gdb -i=mi") seems to confuse Emacs, so much so that
> the debugging session is barely usable: symbols are unknown, so I
> cannot set breakpoints, etc.
> 
> There's no such problem with GDB 11.1.
> 
> I wonder if this is a Windows specific issue.  Did someone try GDB 12
> inside Emacs on GNU/Linux, and if so, did it work as expected?

Turns out this is also due to the same commit that modified
gdb_readline_no_editing_callback to call setbuf on all streams and any
number of times, which also caused the input breakage.  See PR 29002
for the details and the proposed fix.

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Sat, 26 Mar 2022 20:59:04 +0300
> From: Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org>
> Cc: gdb-patches@sourceware.org
> 
> Second, one of the selftests fails:
> 
>   Running selftest dw2_expand_symtabs_matching.
>   warning: charset conversion failure for 'u8função'.
>   You may have the wrong value for 'set ada source-charset'.
>   warning: could not convert 'yfunc ' from the host encoding (CP1255) to UTF-32.
>   This normally should not happen, please file a bug report.
> 
> AFAIU, this is because the names of these two functions are,
> respectively, in UTF-8 and in Latin-1, but the charset conversion
> thinks they are in CP1255.  Where does the test tell the conversion
> functions what is the source encoding?

Ping!  Can someone please help me debugging this selftest failure?
Where should I look for the definitions of the host charset used by
this selftest?

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Sun, 27 Mar 2022 09:27:52 +0300
> From: Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org>
> Cc: gdb-patches@sourceware.org
> 
> > Date: Sat, 26 Mar 2022 21:51:49 +0300
> > From: Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org>
> > Cc: gdb-patches@sourceware.org
> > 
> > And yet another issue: if I start GDB with the MI interface:
> > 
> >    gdb -i=mi PROGRAM
> > 
> > then whatever I type at the prompt GDB thinks I typed "g":
> > 
> >   (gdb)
> >   r -Q
> >   &"g\n"
> >   &"Ambiguous command \"g\": gcore, generate-core-file, goto-bookmark, gr, gu, guile, guile-repl.\n"
> >   ^error,msg="Ambiguous command \"g\": gcore, generate-core-file, goto-bookmark, gr, gu, guile, guile-repl."
> >   (gdb)
> > 
> > As you see, I typed "r -Q", but GDB thinks I typed "g".
> 
> This is now a Bugzilla PR 29002.  I posted my proposed solution there,
> see
> 
>   https://sourceware.org/bugzilla/show_bug.cgi?id=29002

Ping!  The two issues described in this PR are IMO serious and should
be fixed before GDB 12.1 is released.  Could someone please respond to
what I wrote there, and propose how to fix this without adversely
affecting GNU/Linux (and possibly other non-Windows platforms)?

Re: GDB 12.0.90 available for testing

[Pedro Alves]

On 2022-03-31 07:21, Eli Zaretskii via Gdb-patches wrote:
>> Date: Sat, 26 Mar 2022 20:59:04 +0300
>> From: Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org>
>> Cc: gdb-patches@sourceware.org
>>
>> Second, one of the selftests fails:
>>
>>   Running selftest dw2_expand_symtabs_matching.
>>   warning: charset conversion failure for 'u8função'.
>>   You may have the wrong value for 'set ada source-charset'.
>>   warning: could not convert 'yfunc ' from the host encoding (CP1255) to UTF-32.
>>   This normally should not happen, please file a bug report.
>>
>> AFAIU, this is because the names of these two functions are,
>> respectively, in UTF-8 and in Latin-1, but the charset conversion
>> thinks they are in CP1255.  Where does the test tell the conversion
>> functions what is the source encoding?
> 
> Ping!  Can someone please help me debugging this selftest failure?
> Where should I look for the definitions of the host charset used by
> this selftest?

This is not really a failure, it's just a warning, though the message
gdb prints sounds scary.  I chatted with Tromey about it last week, and the
issue is that there's a unit test that always exercises a symbol with a
latin-1 character (0xff).  I added that testcase originally, and IIRC, that
was about making sure that the name lookup index was able to sort
strings properly with the 0xff character, because the code
does "ch+1" at some point in the sorting/lookup algorithm, which overflows
in that case.

It may be that fix is to make the unit test temporarily set the
host charset, and also to remove that "should not happen" warning, as
I think that it should be possible to come up with such symbol names
with escape codes, thus it's not always really a bug.

But in nutshell, this isn't really a GDB bug, and it shouldn't block the release.

Re: GDB 12.0.90 available for testing

[Pedro Alves]

[Adding Andrew]

On 2022-03-31 07:23, Eli Zaretskii via Gdb-patches wrote:
>> Date: Sun, 27 Mar 2022 09:27:52 +0300
>> From: Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org>
>> Cc: gdb-patches@sourceware.org
>>
>>> Date: Sat, 26 Mar 2022 21:51:49 +0300
>>> From: Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org>
>>> Cc: gdb-patches@sourceware.org
>>>
>>> And yet another issue: if I start GDB with the MI interface:
>>>
>>>    gdb -i=mi PROGRAM
>>>
>>> then whatever I type at the prompt GDB thinks I typed "g":
>>>
>>>   (gdb)
>>>   r -Q
>>>   &"g\n"
>>>   &"Ambiguous command \"g\": gcore, generate-core-file, goto-bookmark, gr, gu, guile, guile-repl.\n"
>>>   ^error,msg="Ambiguous command \"g\": gcore, generate-core-file, goto-bookmark, gr, gu, guile, guile-repl."
>>>   (gdb)
>>>
>>> As you see, I typed "r -Q", but GDB thinks I typed "g".
>>
>> This is now a Bugzilla PR 29002.  I posted my proposed solution there,
>> see
>>
>>   https://sourceware.org/bugzilla/show_bug.cgi?id=29002
> 
> Ping!  The two issues described in this PR are IMO serious and should
> be fixed before GDB 12.1 is released.  Could someone please respond to
> what I wrote there, and propose how to fix this without adversely
> affecting GNU/Linux (and possibly other non-Windows platforms)?

So from the bugzilla discussion, it sounds like calling setvbuf all the
time is a bad idea.  So we should call it once for a given stream early
on, just once.  That sounds reasonable to me.  However, the patch you
attached in bugzilla doesn't seem to be doing that correctly:


836	  if (!done_once && !ISATTY (stream))
837	    {
838	      setbuf (stream, NULL);
839	      done_once = 1;
840	    }
836	
841	

because that will only do it once for all streams, and we need to do this
once per stream.

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Thu, 31 Mar 2022 10:48:18 +0100
> Cc: gdb-patches@sourceware.org, Andrew Burgess <aburgess@redhat.com>
> From: Pedro Alves <pedro@palves.net>
> 
> >>   https://sourceware.org/bugzilla/show_bug.cgi?id=29002
> > 
> > Ping!  The two issues described in this PR are IMO serious and should
> > be fixed before GDB 12.1 is released.  Could someone please respond to
> > what I wrote there, and propose how to fix this without adversely
> > affecting GNU/Linux (and possibly other non-Windows platforms)?
> 
> So from the bugzilla discussion, it sounds like calling setvbuf all the
> time is a bad idea.  So we should call it once for a given stream early
> on, just once.  That sounds reasonable to me.  However, the patch you
> attached in bugzilla doesn't seem to be doing that correctly:
> 
> 
> 836	  if (!done_once && !ISATTY (stream))
> 837	    {
> 838	      setbuf (stream, NULL);
> 839	      done_once = 1;
> 840	    }
> 836	
> 841	
> 
> because that will only do it once for all streams, and we need to do this
> once per stream.

The above was what we did in GDB 11 and before.

I'm okay with testing an alternative patch, or trying to write one
myself, but for the latter I need some guidance, as I'm not familiar
with how streams are managed in GDB.  Specifically, how do I "do this
once per stream"?  It would require some flag for each stream (in
'struct ui') or something?

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Thu, 31 Mar 2022 10:44:21 +0100
> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
> 
> On 2022-03-31 07:21, Eli Zaretskii via Gdb-patches wrote:
> >> Date: Sat, 26 Mar 2022 20:59:04 +0300
> >> From: Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org>
> >> Cc: gdb-patches@sourceware.org
> >>
> >> Second, one of the selftests fails:
> >>
> >>   Running selftest dw2_expand_symtabs_matching.
> >>   warning: charset conversion failure for 'u8função'.
> >>   You may have the wrong value for 'set ada source-charset'.
> >>   warning: could not convert 'yfunc ' from the host encoding (CP1255) to UTF-32.
> >>   This normally should not happen, please file a bug report.
> >>
> >> AFAIU, this is because the names of these two functions are,
> >> respectively, in UTF-8 and in Latin-1, but the charset conversion
> >> thinks they are in CP1255.  Where does the test tell the conversion
> >> functions what is the source encoding?
> > 
> > Ping!  Can someone please help me debugging this selftest failure?
> > Where should I look for the definitions of the host charset used by
> > this selftest?
> 
> This is not really a failure, it's just a warning, though the message
> gdb prints sounds scary.  I chatted with Tromey about it last week, and the
> issue is that there's a unit test that always exercises a symbol with a
> latin-1 character (0xff).  I added that testcase originally, and IIRC, that
> was about making sure that the name lookup index was able to sort
> strings properly with the 0xff character, because the code
> does "ch+1" at some point in the sorting/lookup algorithm, which overflows
> in that case.
> 
> It may be that fix is to make the unit test temporarily set the
> host charset, and also to remove that "should not happen" warning, as
> I think that it should be possible to come up with such symbol names
> with escape codes, thus it's not always really a bug.

Does this test fail on GNU/Linux?  If not, can you (or someone else)
tell what is the difference between GNU/Linux and Windows for this
purpose?  Neither is using Latin-1 as the default host charset, right?

> But in nutshell, this isn't really a GDB bug, and it shouldn't block the release.

I didn't want to imply that the release should be blocked.  I'm just
trying to use the pretest for what i's intended: to find bugs and fix
them, preferably before the release.

TIA

Re: GDB 12.0.90 available for testing

[Pedro Alves]

On 2022-03-31 12:58, Eli Zaretskii wrote:
>> Date: Thu, 31 Mar 2022 10:44:21 +0100
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <pedro@palves.net>

>> This is not really a failure, it's just a warning, though the message
>> gdb prints sounds scary.  I chatted with Tromey about it last week, and the
>> issue is that there's a unit test that always exercises a symbol with a
>> latin-1 character (0xff).  I added that testcase originally, and IIRC, that
>> was about making sure that the name lookup index was able to sort
>> strings properly with the 0xff character, because the code
>> does "ch+1" at some point in the sorting/lookup algorithm, which overflows
>> in that case.
>>
>> It may be that fix is to make the unit test temporarily set the
>> host charset, and also to remove that "should not happen" warning, as
>> I think that it should be possible to come up with such symbol names
>> with escape codes, thus it's not always really a bug.
> 
> Does this test fail on GNU/Linux?  If not, can you (or someone else)
> tell what is the difference between GNU/Linux and Windows for this
> purpose?  Neither is using Latin-1 as the default host charset, right?

I see the same warning on GNU/Linux, just different host encoding:

(gdb) maint selftest 
...
Running selftest dw2_expand_symtabs_matching.
warning: could not convert 'yfunc�' from the host encoding (UTF-8) to UTF-32.
This normally should not happen, please file a bug report.
warning: charset conversion failure for 'yfunc�'.
You may have the wrong value for 'set ada source-charset'.
...
Ran 145 unit tests, 0 failed
(gdb) 

Note the "0 failed" at the end.  I assume you get that too?  That's what I mean
by "this is not really a failure, it's just a warning".

> 
>> But in nutshell, this isn't really a GDB bug, and it shouldn't block the release.
> 
> I didn't want to imply that the release should be blocked.  I'm just
> trying to use the pretest for what i's intended: to find bugs and fix
> them, preferably before the release.

Understood.

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Thu, 31 Mar 2022 13:05:17 +0100
> Cc: brobecker@adacore.com, gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
> 
> > Does this test fail on GNU/Linux?  If not, can you (or someone else)
> > tell what is the difference between GNU/Linux and Windows for this
> > purpose?  Neither is using Latin-1 as the default host charset, right?
> 
> I see the same warning on GNU/Linux, just different host encoding:
> 
> (gdb) maint selftest 
> ...
> Running selftest dw2_expand_symtabs_matching.
> warning: could not convert 'yfunc�' from the host encoding (UTF-8) to UTF-32.
> This normally should not happen, please file a bug report.
> warning: charset conversion failure for 'yfunc�'.
> You may have the wrong value for 'set ada source-charset'.
> ...
> Ran 145 unit tests, 0 failed
> (gdb) 
> 
> Note the "0 failed" at the end.  I assume you get that too?  That's what I mean
> by "this is not really a failure, it's just a warning".

Yes, I also get a zero.

Thanks, if this produces warnings on GNU/Linux as well, I guess it can
be considered "normal".

Re: GDB 12.0.90 available for testing

[Andrew Burgess]

Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org> writes:

>> Date: Thu, 31 Mar 2022 10:48:18 +0100
>> Cc: gdb-patches@sourceware.org, Andrew Burgess <aburgess@redhat.com>
>> From: Pedro Alves <pedro@palves.net>
>> 
>> >>   https://sourceware.org/bugzilla/show_bug.cgi?id=29002
>> > 
>> > Ping!  The two issues described in this PR are IMO serious and should
>> > be fixed before GDB 12.1 is released.  Could someone please respond to
>> > what I wrote there, and propose how to fix this without adversely
>> > affecting GNU/Linux (and possibly other non-Windows platforms)?
>> 
>> So from the bugzilla discussion, it sounds like calling setvbuf all the
>> time is a bad idea.  So we should call it once for a given stream early
>> on, just once.  That sounds reasonable to me.  However, the patch you
>> attached in bugzilla doesn't seem to be doing that correctly:
>> 
>> 
>> 836	  if (!done_once && !ISATTY (stream))
>> 837	    {
>> 838	      setbuf (stream, NULL);
>> 839	      done_once = 1;
>> 840	    }
>> 836	
>> 841	
>> 
>> because that will only do it once for all streams, and we need to do this
>> once per stream.
>
> The above was what we did in GDB 11 and before.

But what we used to do wasn't good enough.  On Linux we have to turn off
buffering even when 'ISATTY (stream)' is true, otherwise glibc/kernel
will conspire to hide input from us.

Could you give the patch below a try, please.  It tries to move the
setbuf calls out more so (I hope) they only get done once per stream,
and before we've started reading anything from the stream.

Thanks,
Andrew

---

diff --git a/gdb/event-top.c b/gdb/event-top.c
index b628f0397cb..a55338d78a2 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -818,19 +818,6 @@ gdb_readline_no_editing_callback (gdb_client_data client_data)
   FILE *stream = ui->instream != nullptr ? ui->instream : ui->stdin_stream;
   gdb_assert (stream != nullptr);
 
-  /* Unbuffer the input stream, so that, later on, the calls to fgetc
-     fetch only one char at the time from the stream.  The fgetc's will
-     get up to the first newline, but there may be more chars in the
-     stream after '\n'.  If we buffer the input and fgetc drains the
-     stream, getting stuff beyond the newline as well, a select, done
-     afterwards will not trigger.
-
-     This unbuffering was, at one point, not applied if the input stream
-     was a tty, however, the buffering can cause problems, even for a tty,
-     in some cases.  Please ensure that any changes in this area run the MI
-     tests with the FORCE_SEPARATE_MI_TTY=1 flag being passed.  */
-  setbuf (stream, NULL);
-
   /* We still need the while loop here, even though it would seem
      obvious to invoke gdb_readline_no_editing_callback at every
      character entered.  If not using the readline library, the
diff --git a/gdb/top.c b/gdb/top.c
index ecd31456f03..456130856e5 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -283,6 +283,8 @@ ui::ui (FILE *instream_, FILE *outstream_, FILE *errstream_)
 {
   buffer_init (&line_buffer);
 
+  setbuf (instream_, NULL);
+
   if (ui_list == NULL)
     ui_list = this;
   else
@@ -412,6 +414,8 @@ read_command_file (FILE *stream)
 {
   struct ui *ui = current_ui;
 
+  setbuf (stream, nullptr);
+
   scoped_restore save_instream
     = make_scoped_restore (&ui->instream, stream);
 

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: gdb-patches@sourceware.org, brobecker@adacore.com
> Date: Fri, 01 Apr 2022 11:12:18 +0100
> 
> Could you give the patch below a try, please.  It tries to move the
> setbuf calls out more so (I hope) they only get done once per stream,
> and before we've started reading anything from the stream.

Thanks, this fixes the case of using GDB from Emacs's gdb-mi.el
front-end.  But if I invoke GDB from the shell prompt with the -i=mi
option, it still thinks I type "g\n" no matter what I actually type at
the prompt.

So I guess there are problems with making the console input stream
unbuffered, at least on MS-Windows?

P.S. Aren't these problems visible in the MinGW64 builds of GDB 12?
IOW, is this only a problem with the MinGW flavor I'm using to build
GDB?

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Fri, 01 Apr 2022 14:18:53 +0300
> From: Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org>
> Cc: brobecker@adacore.com, gdb-patches@sourceware.org, pedro@palves.net
> 
> Thanks, this fixes the case of using GDB from Emacs's gdb-mi.el
> front-end.  But if I invoke GDB from the shell prompt with the -i=mi
> option, it still thinks I type "g\n" no matter what I actually type at
> the prompt.
> 
> So I guess there are problems with making the console input stream
> unbuffered, at least on MS-Windows?

Or maybe read_command_file shouldn't call setbuf for the same stream
repeatedly, but only once?

Re: GDB 12.0.90 available for testing

[Joel Brobecker]

> > Could you give the patch below a try, please.  It tries to move the
> > setbuf calls out more so (I hope) they only get done once per stream,
> > and before we've started reading anything from the stream.
> 
> Thanks, this fixes the case of using GDB from Emacs's gdb-mi.el
> front-end.  But if I invoke GDB from the shell prompt with the -i=mi
> option, it still thinks I type "g\n" no matter what I actually type at
> the prompt.
> 
> So I guess there are problems with making the console input stream
> unbuffered, at least on MS-Windows?
> 
> P.S. Aren't these problems visible in the MinGW64 builds of GDB 12?
> IOW, is this only a problem with the MinGW flavor I'm using to build
> GDB?

We test that configuration every night, and I confirm we haven't seen
that issue. Don't know about cygwin, though.

-- 
Joel

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Fri, 1 Apr 2022 05:36:32 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Andrew Burgess <aburgess@redhat.com>, pedro@palves.net,
> 	gdb-patches@sourceware.org, brobecker@adacore.com
> 
> > P.S. Aren't these problems visible in the MinGW64 builds of GDB 12?
> > IOW, is this only a problem with the MinGW flavor I'm using to build
> > GDB?
> 
> We test that configuration every night, and I confirm we haven't seen
> that issue.

With -i=mi or with the CLI?  The latter doesn't show any problems
here.

Do you also use GDB via Emacs's "M-x gdb" command (which invokes GDB
with -i=mi) or with "M-x gud-gdb" (which uses the CLI and
annotations)?

If this problem only happens with mingw.org's MinGW, I'm okay with
having local workarounds.  Although I'm puzzled how can that be, since
both MinGW flavors are supposed to share the same runtime.  Or do you
build GDB against the UCRT (as opposed to MSVCRT) runtime?

Re: GDB 12.0.90 available for testing

[Joel Brobecker]

> > > P.S. Aren't these problems visible in the MinGW64 builds of GDB 12?
> > > IOW, is this only a problem with the MinGW flavor I'm using to build
> > > GDB?
> > 
> > We test that configuration every night, and I confirm we haven't seen
> > that issue.
> 
> With -i=mi or with the CLI?  The latter doesn't show any problems
> here.

The testing includes both modes.

> Do you also use GDB via Emacs's "M-x gdb" command (which invokes GDB
> with -i=mi) or with "M-x gud-gdb" (which uses the CLI and
> annotations)?

We don't.

> If this problem only happens with mingw.org's MinGW, I'm okay with
> having local workarounds.  Although I'm puzzled how can that be, since
> both MinGW flavors are supposed to share the same runtime.  Or do you
> build GDB against the UCRT (as opposed to MSVCRT) runtime?

I don't know how to answer that question. Is that something
I can figure out from looking at how we configure the build
of MinGW-w64?

-- 
Joel

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Fri, 1 Apr 2022 07:12:06 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Joel Brobecker <brobecker@adacore.com>, aburgess@redhat.com,
> 	pedro@palves.net, gdb-patches@sourceware.org
> 
> > If this problem only happens with mingw.org's MinGW, I'm okay with
> > having local workarounds.  Although I'm puzzled how can that be, since
> > both MinGW flavors are supposed to share the same runtime.  Or do you
> > build GDB against the UCRT (as opposed to MSVCRT) runtime?
> 
> I don't know how to answer that question. Is that something
> I can figure out from looking at how we configure the build
> of MinGW-w64?

It's enough (and simpler) to look at the DLLs on which the GDB
executable on Windows depends, with this command:

  objdump -x gdb.exe | fgrep "DLL Name"

Here's the output on my system:

  (standard input):72:    DLL Name: ADVAPI32.DLL
  (standard input):82:    DLL Name: libguile-2.0-22.dll
  (standard input):219:   DLL Name: KERNEL32.dll
  (standard input):326:   DLL Name: msvcrt.dll
  (standard input):353:   DLL Name: msvcrt.dll
  (standard input):501:   DLL Name: libncurses5.dll
  (standard input):578:   DLL Name: libsource-highlight-4.dll
  (standard input):585:   DLL Name: USER32.dll
  (standard input):594:   DLL Name: WS2_32.dll
  (standard input):615:   DLL Name: zlib1.dll
  (standard input):629:   DLL Name: libgcc_s_dw2-1.dll
  (standard input):645:   DLL Name: libstdc++-6.dll
  (standard input):736:   DLL Name: python34.dll

Note the two instances of msvcrt.dll -- this is the (traditional) MS C
runtime library.  If you see UCRT instead, it might explain the
difference in our observations.

Re: GDB 12.0.90 available for testing

[Joel Brobecker]

> > I don't know how to answer that question. Is that something
> > I can figure out from looking at how we configure the build
> > of MinGW-w64?
> 
> It's enough (and simpler) to look at the DLLs on which the GDB
> executable on Windows depends, with this command:
> 
>   objdump -x gdb.exe | fgrep "DLL Name"

Thanks for the tip. Here's what I have:

    | $ objdump.exe -x gdb.exe | grep "DLL Name"
    | 	DLL Name: python39.dll
    | 	DLL Name: ADVAPI32.dll
    | 	DLL Name: KERNEL32.dll
    | 	DLL Name: msvcrt.dll
    | 	DLL Name: USER32.dll
    | 	DLL Name: WS2_32.dll


> Note the two instances of msvcrt.dll -- this is the (traditional) MS C
> runtime library.  If you see UCRT instead, it might explain the
> difference in our observations.

No luck with that lead, it seems...

-- 
Joel

Re: GDB 12.0.90 available for testing

[Andrew Burgess]

Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org> writes:

>> Date: Fri, 01 Apr 2022 14:18:53 +0300
>> From: Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org>
>> Cc: brobecker@adacore.com, gdb-patches@sourceware.org, pedro@palves.net
>> 
>> Thanks, this fixes the case of using GDB from Emacs's gdb-mi.el
>> front-end.  But if I invoke GDB from the shell prompt with the -i=mi
>> option, it still thinks I type "g\n" no matter what I actually type at
>> the prompt.
>> 
>> So I guess there are problems with making the console input stream
>> unbuffered, at least on MS-Windows?
>
> Or maybe read_command_file shouldn't call setbuf for the same stream
> repeatedly, but only once?

I think it must be the former, as far as I can tell with the patch I
posted we call setbuf just once on stdin (for your -i=mi case).  The
read_command_file will only be called if you have gdbinit files to read
in.  You could try adding the -nx and -nh options when starting GDB to
prevent any gdbinit files being read, but I'd be amazed if that makes a
difference.

Sorry I can't offer more insight.

Thanks,
Andrew

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: pedro@palves.net, gdb-patches@sourceware.org, brobecker@adacore.com
> Date: Fri, 01 Apr 2022 16:21:35 +0100
> 
> >> So I guess there are problems with making the console input stream
> >> unbuffered, at least on MS-Windows?
> >
> > Or maybe read_command_file shouldn't call setbuf for the same stream
> > repeatedly, but only once?
> 
> I think it must be the former, as far as I can tell with the patch I
> posted we call setbuf just once on stdin (for your -i=mi case).  The
> read_command_file will only be called if you have gdbinit files to read
> in.  You could try adding the -nx and -nh options when starting GDB to
> prevent any gdbinit files being read, but I'd be amazed if that makes a
> difference.

It indeed doesn't.

> Sorry I can't offer more insight.

So what would be the way forward?  The patch below fixes the problem
for me.  But given that Joel says the problem doesn't happen in your
MinGW builds, maybe we shouldn't install it, even for MinGW?  Or
condition it only by symbols available in mingw.org's MingW?

--- gdb/top.c~1	2022-03-28 16:27:31.211375000 +0300
+++ gdb/top.c	2022-04-01 19:12:33.617625000 +0300
@@ -286,6 +286,9 @@ ui::ui (FILE *instream_, FILE *outstream
 {
   buffer_init (&line_buffer);
 
+  if (!ISATTY (instream_))
+    setbuf (instream_, NULL);
+
   if (ui_list == NULL)
     ui_list = this;
   else
@@ -415,6 +418,8 @@ read_command_file (FILE *stream)
 {
   struct ui *ui = current_ui;
 
+  setbuf (stream, nullptr);
+
   scoped_restore save_instream
     = make_scoped_restore (&ui->instream, stream);
 

Re: GDB 12.0.90 available for testing

[Hannes Domani]

 Am Freitag, 1. April 2022, 18:18:34 MESZ hat Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org> Folgendes geschrieben:

> > From: Andrew Burgess <aburgess@redhat.com>
> > Cc: pedro@palves.net, gdb-patches@sourceware.org, brobecker@adacore.com
> > Date: Fri, 01 Apr 2022 16:21:35 +0100
> >
> > >> So I guess there are problems with making the console input stream
> > >> unbuffered, at least on MS-Windows?
> > >
> > > Or maybe read_command_file shouldn't call setbuf for the same stream
> > > repeatedly, but only once?
> >
> > I think it must be the former, as far as I can tell with the patch I
> > posted we call setbuf just once on stdin (for your -i=mi case).  The
> > read_command_file will only be called if you have gdbinit files to read
> > in.  You could try adding the -nx and -nh options when starting GDB to
> > prevent any gdbinit files being read, but I'd be amazed if that makes a
> > difference.
>
> It indeed doesn't.
>
>
> > Sorry I can't offer more insight.
>
>
> So what would be the way forward?  The patch below fixes the problem
> for me.  But given that Joel says the problem doesn't happen in your
> MinGW builds, maybe we shouldn't install it, even for MinGW?  Or
> condition it only by symbols available in mingw.org's MingW?

I just finished my build of gdb 12 branch with a mingw-w64 gcc, and I also have that
problem when starting gdb with -i=mi:

(gdb)
r -Q
&"g\n"
&"Ambiguous command \"g\": gcore, generate-core-file, goto-bookmark, gr, gu, guile, guile-repl.\n"
^error,msg="Ambiguous command \"g\": gcore, generate-core-file, goto-bookmark, gr, gu, guile, guile-repl."


If I "delete" that not-visible 'g' with backspace before I enter the command, it works again.
So there definitely is something weird going on here.


Not related, but on my first test with TUI I noticed that a line-break is missing
after the "Load new symbol table" question:

(gdb) file c:/heob/heob64.exe
Load new symbol table from "c:/heob/heob64.exe"? (y or n) yReading symbols from c:/heob/heob64.exe...
(gdb) file c:/heob/heob32.exe
Load new symbol table from "c:/heob/heob32.exe"? (y or n) yReading symbols from c:/heob/heob32.exe...

It's fine outside of TUI:

(gdb) file c:/heob/heob64.exe
Load new symbol table from "c:/heob/heob64.exe"? (y or n) y
Reading symbols from c:/heob/heob64.exe...
(gdb) file c:/heob/heob32.exe
Load new symbol table from "c:/heob/heob32.exe"? (y or n) y
Reading symbols from c:/heob/heob32.exe...


I will try to debug these, maybe I can figure them out.


Regards
Hannes

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Sun, 3 Apr 2022 13:02:31 +0000 (UTC)
> From: Hannes Domani <ssbssa@yahoo.de>
> Cc: "brobecker@adacore.com" <brobecker@adacore.com>, 
> 	"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>, 
> 	"pedro@palves.net" <pedro@palves.net>
> 
> I just finished my build of gdb 12 branch with a mingw-w64 gcc, and I also have that
> problem when starting gdb with -i=mi:
> 
> (gdb)
> r -Q
> &"g\n"
> &"Ambiguous command \"g\": gcore, generate-core-file, goto-bookmark, gr, gu, guile, guile-repl.\n"
> ^error,msg="Ambiguous command \"g\": gcore, generate-core-file, goto-bookmark, gr, gu, guile, guile-repl."
> 
> 
> If I "delete" that not-visible 'g' with backspace before I enter the command, it works again.
> So there definitely is something weird going on here.

Thanks.  It is therefore strange that Joel reported he and others
didn't see this in their MinGW builds.

Re: GDB 12.0.90 available for testing

[Joel Brobecker]

> I just finished my build of gdb 12 branch with a mingw-w64 gcc, and I
> also have that problem when starting gdb with -i=mi:

Do you guys see the same issue with master?

What about regular MI commands (e.g. "-exec-run")?

-- 
Joel

Re: GDB 12.0.90 available for testing

[Hannes Domani]

 Am Sonntag, 3. April 2022, 16:02:51 MESZ hat Joel Brobecker <brobecker@adacore.com> Folgendes geschrieben:

> > I just finished my build of gdb 12 branch with a mingw-w64 gcc, and I
> > also have that problem when starting gdb with -i=mi:
>
> Do you guys see the same issue with master?

Rebuilding on Windows takes a while on my PC, so I don't really want to do
that right now, but I wouldn't expect a different behavior there.


> What about regular MI commands (e.g. "-exec-run")?

It doesn't matter at all which command I try, it always gets 'g' as first
character.


But I found out now that on Windows the combination of
setbuf/WaitForMultipleObjects doesn't work correctly with fgetc.
WaitForMultipleObjects is called by console_select_thread in ser-mingw.c,
so the following is a minimal reproducer of the problem (but for some reason
I always see character 'f' instead of 'g'):

#include <stdio.h>
#include <windows.h>
int main()
{
  setbuf(stdin, NULL);
  while (1)
  {
    printf("input: ");
    HANDLE in = GetStdHandle(STD_INPUT_HANDLE);
    WaitForMultipleObjects(1, &in, FALSE, INFINITE);
    while (1)
    {
      int c = fgetc(stdin);
      if (c == '\n')
        break;
      else
        printf("char: %c\n", c);
    }
  }
  return 0;
}

I get the following result:

C:\src\tests>gcc -o fgetc.exe fgetc.c

C:\src\tests>fgetc
input: abcde
char: f
char:
char: c
char: d
char: e
input: aaaaa
char: f
char:
char: a
char: a
char: a


Note that I'm on Windows 7, so on a newer system the behavior might be
different.

I don't really see a solution beside some "#ifdef _WIN32" code to fix this.

Regards
Hannes

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Sun, 3 Apr 2022 15:26:38 +0000 (UTC)
> From: Hannes Domani <ssbssa@yahoo.de>
> Cc: Andrew Burgess <aburgess@redhat.com>, Eli Zaretskii <eliz@gnu.org>, 
> 	"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>, 
> 	"pedro@palves.net" <pedro@palves.net>
> 
>  Am Sonntag, 3. April 2022, 16:02:51 MESZ hat Joel Brobecker <brobecker@adacore.com> Folgendes geschrieben:
> 
> But I found out now that on Windows the combination of
> setbuf/WaitForMultipleObjects doesn't work correctly with fgetc.
> WaitForMultipleObjects is called by console_select_thread in ser-mingw.c,
> so the following is a minimal reproducer of the problem (but for some reason
> I always see character 'f' instead of 'g'):

In general, to switch the console to unbuffered reads, shouldn't we
call SetConsoleMode to force raw input from the console?

> I don't really see a solution beside some "#ifdef _WIN32" code to fix this.

That was my conclusion as well.

Thanks.

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Sun, 03 Apr 2022 18:38:21 +0300
> From: Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org>
> Cc: pedro@palves.net, aburgess@redhat.com, gdb-patches@sourceware.org,
>  brobecker@adacore.com
> 
> > Date: Sun, 3 Apr 2022 15:26:38 +0000 (UTC)
> > From: Hannes Domani <ssbssa@yahoo.de>
> > Cc: Andrew Burgess <aburgess@redhat.com>, Eli Zaretskii <eliz@gnu.org>, 
> > 	"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>, 
> > 	"pedro@palves.net" <pedro@palves.net>
> > 
> >  Am Sonntag, 3. April 2022, 16:02:51 MESZ hat Joel Brobecker <brobecker@adacore.com> Folgendes geschrieben:
> > 
> > But I found out now that on Windows the combination of
> > setbuf/WaitForMultipleObjects doesn't work correctly with fgetc.
> > WaitForMultipleObjects is called by console_select_thread in ser-mingw.c,
> > so the following is a minimal reproducer of the problem (but for some reason
> > I always see character 'f' instead of 'g'):
> 
> In general, to switch the console to unbuffered reads, shouldn't we
> call SetConsoleMode to force raw input from the console?
> 
> > I don't really see a solution beside some "#ifdef _WIN32" code to fix this.
> 
> That was my conclusion as well.

Can we please finalize the fix for this?  The following patch, which
is a variation on the patch that Andrew sent, works for me.  If there
are no objections, I'd like to install it on both branches (with a
suitable log message describing the misbehavior on MS-Windows).

Thanks.

--- gdb/event-top.c~0	2022-03-20 06:59:56.000000000 +0200
+++ gdb/event-top.c	2022-04-01 14:06:22.164500000 +0300
@@ -821,19 +821,6 @@ gdb_readline_no_editing_callback (gdb_cl
   FILE *stream = ui->instream != nullptr ? ui->instream : ui->stdin_stream;
   gdb_assert (stream != nullptr);
 
-  /* Unbuffer the input stream, so that, later on, the calls to fgetc
-     fetch only one char at the time from the stream.  The fgetc's will
-     get up to the first newline, but there may be more chars in the
-     stream after '\n'.  If we buffer the input and fgetc drains the
-     stream, getting stuff beyond the newline as well, a select, done
-     afterwards will not trigger.
-
-     This unbuffering was, at one point, not applied if the input stream
-     was a tty, however, the buffering can cause problems, even for a tty,
-     in some cases.  Please ensure that any changes in this area run the MI
-     tests with the FORCE_SEPARATE_MI_TTY=1 flag being passed.  */
-  setbuf (stream, NULL);
-
   /* We still need the while loop here, even though it would seem
      obvious to invoke gdb_readline_no_editing_callback at every
      character entered.  If not using the readline library, the
--- gdb/top.c~0	2022-04-07 14:01:19.479625000 +0300
+++ gdb/top.c	2022-04-07 13:56:54.995250000 +0300
@@ -286,6 +286,15 @@ ui::ui (FILE *instream_, FILE *outstream
 {
   buffer_init (&line_buffer);
 
+#ifdef __MINGW32__
+  /* With MS-Windows runtime, making stdin unbuffered when it's
+     connected to the terminal causes it to misbehave.  */
+  if (!ISATTY (instream_))
+    setbuf (instream_, NULL);
+#else
+  setbuf (instream_, NULL);
+#endif
+
   if (ui_list == NULL)
     ui_list = this;
   else
@@ -415,6 +424,8 @@ read_command_file (FILE *stream)
 {
   struct ui *ui = current_ui;
 
+  setbuf (stream, nullptr);
+
   scoped_restore save_instream
     = make_scoped_restore (&ui->instream, stream);
 

Re: GDB 12.0.90 available for testing

[Tom Tromey]

>> > Was that done intentionally?  If so, what is the reason for not
>> > updating Gnulib?  (I'm not insisting on the update, just asking.  If
>> > this is not an omission, I can easily apply the same patches I did in
>> > GDB 11.1 when building.)
>> 
>> My understanding is that gnulib updates in GDB are performed only
>> as needed, by the people who need it.

Eli> Fine with me, thanks.

If there's a particular gnulib import that's needed, I'm happy to do it.
I'd just need to know the revision.  Or, maybe we should just adopt a
policy of doing an import from gnulib HEAD on trunk after a release
branch is made?

I'm not sure whether we want to attempt an update on the GDB 12 branch.

Tom

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Cc: Joel Brobecker <brobecker@adacore.com>,  Eli Zaretskii <eliz@gnu.org>
> Date: Thu, 07 Apr 2022 10:08:19 -0600
> 
> >> My understanding is that gnulib updates in GDB are performed only
> >> as needed, by the people who need it.
> 
> Eli> Fine with me, thanks.
> 
> If there's a particular gnulib import that's needed, I'm happy to do it.
> I'd just need to know the revision.

It isn't a single update.  I identified at least 2, maybe 3 changes
Gnulib installed based on my reports of problems found in GDB 11.

> Or, maybe we should just adopt a policy of doing an import from
> gnulib HEAD on trunk after a release branch is made?

IMO, it would be best.  But that's not my call, especially since Joel
says there seems to be a policy about that.

> I'm not sure whether we want to attempt an update on the GDB 12 branch.

No, I don't think we should.  It's too late for GDB 12, IMO.

Re: GDB 12.0.90 available for testing

[Tom Tromey]

>> >      target/waitstatus.h: In function 'void stop_all_threads()':
>> >      target/waitstatus.h:175:13: warning: 'ws.target_waitstatus::m_value' may be used uninitialized in this function [-Wmaybe-uninitialized]
>> >        175 |     m_value = other.m_value;
>> > 	   |     ~~~~~~~~^~~~~~~~~~~~~~~

Eli> Maybe we should initialize ws::m_value, just to shut up the compiler?
Eli> Because GDB 12 compiles cleanly other than this warning.

I have a simple patch for this that I will send in a bit.

We patched around this same issue in gdb::optional and in other spots,
so doing it here seems fine as well.

Tom

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Cc: Simon Marchi <simon.marchi@polymtl.ca>,  Eli Zaretskii <eliz@gnu.org>,
>   brobecker@adacore.com
> Date: Thu, 07 Apr 2022 10:13:13 -0600
> 
> >> >      target/waitstatus.h: In function 'void stop_all_threads()':
> >> >      target/waitstatus.h:175:13: warning: 'ws.target_waitstatus::m_value' may be used uninitialized in this function [-Wmaybe-uninitialized]
> >> >        175 |     m_value = other.m_value;
> >> > 	   |     ~~~~~~~~^~~~~~~~~~~~~~~
> 
> Eli> Maybe we should initialize ws::m_value, just to shut up the compiler?
> Eli> Because GDB 12 compiles cleanly other than this warning.
> 
> I have a simple patch for this that I will send in a bit.
> 
> We patched around this same issue in gdb::optional and in other spots,
> so doing it here seems fine as well.

Thanks.

Re: GDB 12.0.90 available for testing

[Joel Brobecker]

> > If there's a particular gnulib import that's needed, I'm happy to do it.
> > I'd just need to know the revision.
> 
> It isn't a single update.  I identified at least 2, maybe 3 changes
> Gnulib installed based on my reports of problems found in GDB 11.
> 
> > Or, maybe we should just adopt a policy of doing an import from
> > gnulib HEAD on trunk after a release branch is made?
> 
> IMO, it would be best.  But that's not my call, especially since Joel
> says there seems to be a policy about that.

There isn't really a policy per se. It's just my understanding
that we currently do not have a process where updates are brought in
on a regular basis -- we stay with a given version until someone
who needs an upgrade send a patch to make the changes he needs.

I agree that having a policy can be helpful. Tom's proposal sounds
fine to me, especially since we have someone who kindly volunteers
to make it happen, at least this time around.

Meanwhile, even if the group decides to reject this as a policy,
I don't think we'll reject patches that upgrade our import of gnulib,
as this is something we're bound to do the next time we need some
fixes made upstream. So, anyone willing to do an update can propose it,
whether we have a policy or not, and it'll be a useful change on its
own right, IMO.

> > I'm not sure whether we want to attempt an update on the GDB 12 branch.
> 
> No, I don't think we should.  It's too late for GDB 12, IMO.

Agreed. On the branch, we should make local and targeted changes
instead.

-- 
Joel

Re: GDB 12.0.90 available for testing

[Joel Brobecker]

> Can we please finalize the fix for this?  The following patch, which
> is a variation on the patch that Andrew sent, works for me.  If there
> are no objections, I'd like to install it on both branches (with a
> suitable log message describing the misbehavior on MS-Windows).

The patch seems reasonable to me, but this is only FWIW, as I do not
have good knowledge of this area.

I'd like to do some testing, to see if I can reproduce the issue
you've seen, since Hannes was also able to reproduce. And I will
also try to test your patch in our setup as well.

Hopefully this weekend.

-- 
Joel

Re: GDB 12.0.90 available for testing

[Simon Marchi]

On 2022-04-07 14:00, Joel Brobecker via Gdb-patches wrote:
>>> If there's a particular gnulib import that's needed, I'm happy to do it.
>>> I'd just need to know the revision.
>>
>> It isn't a single update.  I identified at least 2, maybe 3 changes
>> Gnulib installed based on my reports of problems found in GDB 11.
>>
>>> Or, maybe we should just adopt a policy of doing an import from
>>> gnulib HEAD on trunk after a release branch is made?
>>
>> IMO, it would be best.  But that's not my call, especially since Joel
>> says there seems to be a policy about that.
> 
> There isn't really a policy per se. It's just my understanding
> that we currently do not have a process where updates are brought in
> on a regular basis -- we stay with a given version until someone
> who needs an upgrade send a patch to make the changes he needs.
> 
> I agree that having a policy can be helpful. Tom's proposal sounds
> fine to me, especially since we have someone who kindly volunteers
> to make it happen, at least this time around.
> 
> Meanwhile, even if the group decides to reject this as a policy,
> I don't think we'll reject patches that upgrade our import of gnulib,
> as this is something we're bound to do the next time we need some
> fixes made upstream. So, anyone willing to do an update can propose it,
> whether we have a policy or not, and it'll be a useful change on its
> own right, IMO.

I think that systematically upgrading right after a branch creation is a
good idea.  It is less risky for us, since the following release to use
that code is quite far away.  And it is a good idea to consume the new
gnulib code as early as possible, to weed out and report any problem
upstream.  It is better to report problems early than reporting them two
years after the change has been made.

Simon

Re: GDB 12.0.90 available for testing

[Tom Tromey]

Eli> Can we please finalize the fix for this?  The following patch, which
Eli> is a variation on the patch that Andrew sent, works for me.  If there
Eli> are no objections, I'd like to install it on both branches (with a
Eli> suitable log message describing the misbehavior on MS-Windows).

I applied this patch locally, built on Windows, and ran the AdaCore
internal test suite.  This passed, so at least for our part, there's no
issue.

I also built and regression tested with this patch on x86-64 Fedora 34.

Based on this, I think this patch is ready to check in.  Thank you.

Tom

Re: GDB 12.0.90 available for testing

[Tom Tromey]

Eli> It isn't a single update.  I identified at least 2, maybe 3 changes
Eli> Gnulib installed based on my reports of problems found in GDB 11.

I didn't see the details, can you resend them?  gnulib commit ids would
be the most convenient but other forms would be alright.

On the release branch we could import selected patches if we think they
are safe.

Tom

Re: GDB 12.0.90 available for testing

[Tom Tromey]

Joel> I agree that having a policy can be helpful. Tom's proposal sounds
Joel> fine to me, especially since we have someone who kindly volunteers
Joel> to make it happen, at least this time around.

I've got a patch.

I'm not sure I can really send it to the list.  It's quite large and
it's nearly all mechanical, except the appended.

I built & regtested it on x86-64 Fedora 34.
I also used the Fedora mingw cross toolchain to do a mingw build.

Tom

diff --git a/gnulib/update-gnulib.sh b/gnulib/update-gnulib.sh
index e4a910ff6c5..6025290deb3 100755
--- a/gnulib/update-gnulib.sh
+++ b/gnulib/update-gnulib.sh
@@ -84,7 +84,7 @@ IMPORTED_GNULIB_MODULES="\
 "
 
 # The gnulib commit ID to use for the update.
-GNULIB_COMMIT_SHA1="776af40e09b476a41073131a90022572f448c189"
+GNULIB_COMMIT_SHA1="58c597d13bc57dce3e97ea97856573f2d68ccb8c"
 
 # The expected version number for the various auto tools we will
 # use after the import.

Re: GDB 12.0.90 available for testing

[Joel Brobecker]

Hi Tom,

> I've got a patch.
> 
> I'm not sure I can really send it to the list.  It's quite large and
> it's nearly all mechanical, except the appended.
> 
> I built & regtested it on x86-64 Fedora 34.
> I also used the Fedora mingw cross toolchain to do a mingw build.
> 
> Tom
> 
> diff --git a/gnulib/update-gnulib.sh b/gnulib/update-gnulib.sh
> index e4a910ff6c5..6025290deb3 100755
> --- a/gnulib/update-gnulib.sh
> +++ b/gnulib/update-gnulib.sh
> @@ -84,7 +84,7 @@ IMPORTED_GNULIB_MODULES="\
>  "
>  
>  # The gnulib commit ID to use for the update.
> -GNULIB_COMMIT_SHA1="776af40e09b476a41073131a90022572f448c189"
> +GNULIB_COMMIT_SHA1="58c597d13bc57dce3e97ea97856573f2d68ccb8c"
>  
>  # The expected version number for the various auto tools we will
>  # use after the import.

Is it just code, or does this update also cause the imports to
somehow change? The import changes would be the only thing that
might raise red-flags for me. Other than that, I don't think
I'd need to see the diff, really. The import process being
automated IIRC, it's all mechanical, as you say.

-- 
Joel

Re: GDB 12.0.90 available for testing

[Pedro Alves]

On 2022-04-07 12:09, Eli Zaretskii wrote:
> --- gdb/event-top.c~0	2022-03-20 06:59:56.000000000 +0200
> +++ gdb/event-top.c	2022-04-01 14:06:22.164500000 +0300
> @@ -821,19 +821,6 @@ gdb_readline_no_editing_callback (gdb_cl
>    FILE *stream = ui->instream != nullptr ? ui->instream : ui->stdin_stream;
>    gdb_assert (stream != nullptr);
>  
> -  /* Unbuffer the input stream, so that, later on, the calls to fgetc
> -     fetch only one char at the time from the stream.  The fgetc's will
> -     get up to the first newline, but there may be more chars in the
> -     stream after '\n'.  If we buffer the input and fgetc drains the
> -     stream, getting stuff beyond the newline as well, a select, done
> -     afterwards will not trigger.
> -
> -     This unbuffering was, at one point, not applied if the input stream
> -     was a tty, however, the buffering can cause problems, even for a tty,
> -     in some cases.  Please ensure that any changes in this area run the MI
> -     tests with the FORCE_SEPARATE_MI_TTY=1 flag being passed.  */

Don't we want to preserve this comment?

> -  setbuf (stream, NULL);
> -

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Thu, 7 Apr 2022 20:22:55 +0100
> Cc: ssbssa@yahoo.de, gdb-patches@sourceware.org, brobecker@adacore.com
> From: Pedro Alves <pedro@palves.net>
> 
> On 2022-04-07 12:09, Eli Zaretskii wrote:
> > --- gdb/event-top.c~0	2022-03-20 06:59:56.000000000 +0200
> > +++ gdb/event-top.c	2022-04-01 14:06:22.164500000 +0300
> > @@ -821,19 +821,6 @@ gdb_readline_no_editing_callback (gdb_cl
> >    FILE *stream = ui->instream != nullptr ? ui->instream : ui->stdin_stream;
> >    gdb_assert (stream != nullptr);
> >  
> > -  /* Unbuffer the input stream, so that, later on, the calls to fgetc
> > -     fetch only one char at the time from the stream.  The fgetc's will
> > -     get up to the first newline, but there may be more chars in the
> > -     stream after '\n'.  If we buffer the input and fgetc drains the
> > -     stream, getting stuff beyond the newline as well, a select, done
> > -     afterwards will not trigger.
> > -
> > -     This unbuffering was, at one point, not applied if the input stream
> > -     was a tty, however, the buffering can cause problems, even for a tty,
> > -     in some cases.  Please ensure that any changes in this area run the MI
> > -     tests with the FORCE_SEPARATE_MI_TTY=1 flag being passed.  */
> 
> Don't we want to preserve this comment?
> 
> > -  setbuf (stream, NULL);
> > -

This part of the patch is from Andrew, so I have no opinion about
that.  If we want to preserver it, I guess it should be moved to one
of the places where we call setbuf instead of this single place?

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>,  Eli Zaretskii <eliz@gnu.org>,
>   brobecker@adacore.com
> Date: Thu, 07 Apr 2022 12:30:12 -0600
> 
> Eli> It isn't a single update.  I identified at least 2, maybe 3 changes
> Eli> Gnulib installed based on my reports of problems found in GDB 11.
> 
> I didn't see the details, can you resend them?  gnulib commit ids would
> be the most convenient but other forms would be alright.

The relevant Gnulib commits are: 38d0749, c7b1e06, and 21fccfa.  The
commit messages include the URL of the reports I posted to the Gnulib
mailing list, which led to the changes.

> On the release branch we could import selected patches if we think they
> are safe.

They are safe, but I have no idea whether you can import them without
importing other parts of Gnulib.  Specifically, the patches in those
commits, if applied to GDB, will definitely work in the MinGW builds,
but I'm not sure about other platforms.  Better to consult with the
Gnulib folks on that, I guess.

Re: GDB 12.0.90 available for testing

[Tom Tromey]

>> # The gnulib commit ID to use for the update.
>> -GNULIB_COMMIT_SHA1="776af40e09b476a41073131a90022572f448c189"
>> +GNULIB_COMMIT_SHA1="58c597d13bc57dce3e97ea97856573f2d68ccb8c"
>> 
>> # The expected version number for the various auto tools we will
>> # use after the import.

Joel> Is it just code, or does this update also cause the imports to
Joel> somehow change? The import changes would be the only thing that
Joel> might raise red-flags for me. Other than that, I don't think
Joel> I'd need to see the diff, really. The import process being
Joel> automated IIRC, it's all mechanical, as you say.

I don't really know how to check this.
I re-ran update-gnulib.sh and it printed the appended module list.

Tom

Module list with included dependencies (indented):
    absolute-header
  accept
  alloca
    alloca-opt
    arpa_inet
    assure
    at-internal
    attribute
    basename-lgpl
  bind
    btowc
    builtin-expect
    c99
  canonicalize-lgpl
    chdir
    chdir-long
  chown
    clock-time
    cloexec
    close
    closedir
  connect
  count-one-bits
    ctype
    d-ino
    d-type
  dirent
  dirfd
    dirname-lgpl
    double-slash-root
    dup
    dup2
    eloop-threshold
    environ
  errno
    error
    exitfail
    extensions
    extern-inline
    fchdir
    fcntl
    fcntl-h
    fd-hook
    fd-safer-flag
    fdopendir
  ffs
    filename
    filenamecat-lgpl
    flexmember
    float
    fnmatch
  fnmatch-gnu
    fnmatch-h
    fpieee
    fpucw
    free-posix
    frexp
  frexpl
    fstat
    fstatat
    gen-header
  gendocs
  getcwd
    getcwd-lgpl
    getdelim
    getdtablesize
  getline
    getlogin_r
    getprogname
    getrandom
    gettext-h
  gettimeofday
  gitlog-to-changelog
  glob
    glob-h
    hard-locale
    idx
    include_next
  inet_ntop
    intprops
  inttypes
    inttypes-incomplete
    isblank
    isnand-nolibm
    isnanl-nolibm
    largefile
    libc-config
  limits-h
  listen
    localcharset
    locale
    lock
  lstat
    malloc-posix
    malloca
    math
    mbrtowc
    mbsinit
    mbsrtowcs
    mbtowc
  memchr
  memmem
    memmem-simple
    mempcpy
    memrchr
    minmax
  mkdir
  mkdtemp
  mkostemp
    msvc-inval
    msvc-nothrow
    multiarch
  netdb
    netinet_in
    nocrash
    open
    openat
    openat-die
    openat-h
    opendir
  pathmax
    pipe-posix
  rawmemchr
    readdir
  readlink
    realloc-posix
  rename
    rewinddir
    rmdir
    same-inode
    save-cwd
    scratch_buffer
  select
  setenv
    setlocale-null
  setsockopt
  signal-h
    snippet/_Noreturn
    snippet/arg-nonnull
    snippet/c++defs
    snippet/warn-on-use
  socket
    socketlib
    sockets
    socklen
    ssize_t
    stat
    stat-time
    std-gnu11
    stdalign
    stdbool
    stddef
    stdint
    stdio
    stdlib
  strchrnul
    strdup-posix
    streq
    strerror
    strerror-override
  strerror_r-posix
    string
    strings
    strnlen
    strnlen1
  strstr
    strstr-simple
  strtok_r
    sys_random
    sys_select
    sys_socket
  sys_stat
    sys_time
    sys_types
    sys_uio
  sys_wait
    tempname
    threadlib
    time
  time_r
  unistd
    unistd-safer
  unsetenv
  update-copyright
    vararrays
    verify
  wchar
  wctype-h
    windows-mutex
    windows-once
    windows-recmutex
    windows-rwlock
    wmemchr
    wmempcpy
    xalloc-oversized

Re: GDB 12.0.90 available for testing

[Pedro Alves]

On 2022-04-08 14:38, Tom Tromey wrote:
>>> # The gnulib commit ID to use for the update.
>>> -GNULIB_COMMIT_SHA1="776af40e09b476a41073131a90022572f448c189"
>>> +GNULIB_COMMIT_SHA1="58c597d13bc57dce3e97ea97856573f2d68ccb8c"
>>>
>>> # The expected version number for the various auto tools we will
>>> # use after the import.
> 
> Joel> Is it just code, or does this update also cause the imports to
> Joel> somehow change? The import changes would be the only thing that
> Joel> might raise red-flags for me. Other than that, I don't think
> Joel> I'd need to see the diff, really. The import process being
> Joel> automated IIRC, it's all mechanical, as you say.
> 
> I don't really know how to check this.
> I re-ran update-gnulib.sh and it printed the appended module list.
> 

It used to be that the update would tell you what dependencies were
new, and which were removed.  Either gnulib changed and doesn't print the same
helpful info anymore, or the dependencies really didn't change.
To be sure, I'd diff the logs of two different re-imports from scratch:

 - reimport gnulib at the current hash from scratch: 
     rm -rf gnulib/import
     mkdir gnulib/import/ #to keep our script happy
     ./update-gnulib.sh .... 2>&1 | tee org

 - import gnulib at the new hash from scratch: 

     rm -rf gnulib/import
     mkdir gnulib/import/ #to keep our script happy
     ./update-gnulib.sh .... 2>&1 | tee new

  diff org new, look out for new/dropped dependencies, see if we're 
  losing something that we really do no want to lose but missed
  adding as explicit dependency, check if we gained some new
  dependency that actually eliminates the need for our own portability
  code, etc.

Regardless, I would suggest that updating is useful to see the
dependency updates, and other helpful info, but for final & real
update, I'd suggest always reimporting from scratch, to avoid
trusting that gnulib's update logic worked as well, that it
removed stale files correctly, etc.

Pedro Alves

Re: GDB 12.0.90 available for testing

[Pedro Alves]

On 2022-04-01 15:31, Joel Brobecker wrote:
>>> I don't know how to answer that question. Is that something
>>> I can figure out from looking at how we configure the build
>>> of MinGW-w64?
>>
>> It's enough (and simpler) to look at the DLLs on which the GDB
>> executable on Windows depends, with this command:
>>
>>   objdump -x gdb.exe | fgrep "DLL Name"
> 
> Thanks for the tip. Here's what I have:
> 
>     | $ objdump.exe -x gdb.exe | grep "DLL Name"
>     | 	DLL Name: python39.dll
>     | 	DLL Name: ADVAPI32.dll
>     | 	DLL Name: KERNEL32.dll
>     | 	DLL Name: msvcrt.dll
>     | 	DLL Name: USER32.dll
>     | 	DLL Name: WS2_32.dll
> 
> 
>> Note the two instances of msvcrt.dll -- this is the (traditional) MS C
>> runtime library.  If you see UCRT instead, it might explain the
>> difference in our observations.
> 
> No luck with that lead, it seems...
> 

Since Hannes is able to reproduce this with a small program outside GDB,
I'd suspect this could be related to either:

 - Windows version.  Hannes mentioned Windows 7.  I don't think we heard Windows
   versions from others.

 - Which console / terminal is used.  E.g., cmd.exe in a standard Windows console,
   Powershell, cygwin/msys2 bash, Windows SSH, etc.

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Fri, 8 Apr 2022 15:44:53 +0100
> Cc: aburgess@redhat.com, gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
> 
> On 2022-04-01 15:31, Joel Brobecker wrote:
> >>> I don't know how to answer that question. Is that something
> >>> I can figure out from looking at how we configure the build
> >>> of MinGW-w64?
> >>
> >> It's enough (and simpler) to look at the DLLs on which the GDB
> >> executable on Windows depends, with this command:
> >>
> >>   objdump -x gdb.exe | fgrep "DLL Name"
> > 
> > Thanks for the tip. Here's what I have:
> > 
> >     | $ objdump.exe -x gdb.exe | grep "DLL Name"
> >     | 	DLL Name: python39.dll
> >     | 	DLL Name: ADVAPI32.dll
> >     | 	DLL Name: KERNEL32.dll
> >     | 	DLL Name: msvcrt.dll
> >     | 	DLL Name: USER32.dll
> >     | 	DLL Name: WS2_32.dll
> > 
> > 
> >> Note the two instances of msvcrt.dll -- this is the (traditional) MS C
> >> runtime library.  If you see UCRT instead, it might explain the
> >> difference in our observations.
> > 
> > No luck with that lead, it seems...
> > 
> 
> Since Hannes is able to reproduce this with a small program outside GDB,
> I'd suspect this could be related to either:
> 
>  - Windows version.  Hannes mentioned Windows 7.  I don't think we heard Windows
>    versions from others.
> 
>  - Which console / terminal is used.  E.g., cmd.exe in a standard Windows console,
>    Powershell, cygwin/msys2 bash, Windows SSH, etc.

Perhaps Joel could try that test program in his environment and see if
it produces the same behavior.  If he sees something different, we
could compare the Windows versions.

As for the shell: the problem happens only for direct connection to
the console device, which means cmd.exe, not Bash which runs via
mintty etc., which AFAIK basically appears as a pipe to the console
applications.

Re: GDB 12.0.90 available for testing

[Joel Brobecker]

> > Can we please finalize the fix for this?  The following patch, which
> > is a variation on the patch that Andrew sent, works for me.  If there
> > are no objections, I'd like to install it on both branches (with a
> > suitable log message describing the misbehavior on MS-Windows).
> 
> The patch seems reasonable to me, but this is only FWIW, as I do not
> have good knowledge of this area.
> 
> I'd like to do some testing, to see if I can reproduce the issue
> you've seen, since Hannes was also able to reproduce. And I will
> also try to test your patch in our setup as well.

For the record, I did some testing using the gdb-12-branch, and unlike
Hannes, I wasn't able to reproduce.

I tried in various scenarios:
  - CMD windows
  - ConEmu
  - Cygwin terminal (there at least, I know it's not a real tty, FWIW).

This was done on Windows Server 2016 (IIUC, this would correspond
to a version of Windows 10, verson 1607).

Since Tom already tested the patch being proposed, other than
Pedro's question about the comment being removed, I'm OK with
the patch being pushed to both master and gdb-12-branch.

-- 
Joel

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Sun, 10 Apr 2022 12:06:52 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: aburgess@redhat.com, ssbssa@yahoo.de, pedro@palves.net,
> 	gdb-patches@sourceware.org, brobecker@adacore.com
> 
> For the record, I did some testing using the gdb-12-branch, and unlike
> Hannes, I wasn't able to reproduce.
> 
> I tried in various scenarios:
>   - CMD windows
>   - ConEmu
>   - Cygwin terminal (there at least, I know it's not a real tty, FWIW).
> 
> This was done on Windows Server 2016 (IIUC, this would correspond
> to a version of Windows 10, verson 1607).
> 
> Since Tom already tested the patch being proposed, other than
> Pedro's question about the comment being removed, I'm OK with
> the patch being pushed to both master and gdb-12-branch.

Thanks.

Andrew, would you please chime in and tell what you'd like to do with
that comment?  Whatever you decide, I will do that in the final
commit.

Re: GDB 12.0.90 available for testing

[Luis Machado]

Hi Joel,

On 3/20/22 05:58, Joel Brobecker via Gdb-patches wrote:
> Hello,
> 
> I have just finished creating the gdb-12.0.90 pre-release.
> It is available for download at the following location:
> 
>      ftp://sourceware.org/pub/gdb/snapshots/branch/gdb-12.0.90.tar.xz
> 
> A gzip'ed version is also available: gdb-12.0.90.tar.gz.
> 
> Please give it a test if you can and report any problems you might find.
> 
> On behalf of all the GDB contributors, thank you!

It seems GDB doesn't build with --enable-targets=all for 32-bit Arm. I 
think this is a long-standing bug that has not been fixed yet.

I'd consider this a blocker for the release, as builds shouldn't fail.

I get the following:

binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/../common/sim-close.c:43: 
undefined reference to `bpf_cgen_cpu_close'

binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/sim-if.c:166: 
undefined reference to `bpf_cgen_cpu_open_1'

binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/sim-if.c:179: 
undefined reference to `bpf_cgen_init_dis'

This is GCC 7.5.0 on Ubuntu 18.04.

Should I go ahead and open a ticket against the release? I'm not sure 
who is responsible for handling BPF.

Re: GDB 12.0.90 available for testing

[Joel Brobecker]

Hi Luis,

> On 3/20/22 05:58, Joel Brobecker via Gdb-patches wrote:
> > Hello,
> > 
> > I have just finished creating the gdb-12.0.90 pre-release.
> > It is available for download at the following location:
> > 
> >      ftp://sourceware.org/pub/gdb/snapshots/branch/gdb-12.0.90.tar.xz
> > 
> > A gzip'ed version is also available: gdb-12.0.90.tar.gz.
> > 
> > Please give it a test if you can and report any problems you might find.
> > 
> > On behalf of all the GDB contributors, thank you!
> 
> It seems GDB doesn't build with --enable-targets=all for 32-bit Arm. I think
> this is a long-standing bug that has not been fixed yet.
> 
> I'd consider this a blocker for the release, as builds shouldn't fail.

Generally speaking, I tend to agree, but at the same time, it really
depends.

Is this specific to GDB 12, or did we have this issue with previous
releases?

We also need some kind of visibility as to how quickly we think we can
solve that issue. This usually requires someone to act as the issue's
"champion" -- that person might not be the one actually making the fix,
but they can at least try help expedite the process.

> I get the following:
> 
> binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/../common/sim-close.c:43:
> undefined reference to `bpf_cgen_cpu_close'
> 
> binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/sim-if.c:166:
> undefined reference to `bpf_cgen_cpu_open_1'
> 
> binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/sim-if.c:179:
> undefined reference to `bpf_cgen_init_dis'
> 
> This is GCC 7.5.0 on Ubuntu 18.04.
> 
> Should I go ahead and open a ticket against the release? I'm not sure who is
> responsible for handling BPF.

I'd start by asking Mike Frysinger, who's the sim maintainer.
He might not know about this particular target, but he's made
a lot of cleanups in this area.

In this case, hopefully the fix won't be too difficult.

-- 
Joel

Re: GDB 12.0.90 available for testing

[Luis Machado]

Hi Joel,

On 4/12/22 18:57, Joel Brobecker wrote:
> Hi Luis,
> 
>> On 3/20/22 05:58, Joel Brobecker via Gdb-patches wrote:
>>> Hello,
>>>
>>> I have just finished creating the gdb-12.0.90 pre-release.
>>> It is available for download at the following location:
>>>
>>>       ftp://sourceware.org/pub/gdb/snapshots/branch/gdb-12.0.90.tar.xz
>>>
>>> A gzip'ed version is also available: gdb-12.0.90.tar.gz.
>>>
>>> Please give it a test if you can and report any problems you might find.
>>>
>>> On behalf of all the GDB contributors, thank you!
>>
>> It seems GDB doesn't build with --enable-targets=all for 32-bit Arm. I think
>> this is a long-standing bug that has not been fixed yet.
>>
>> I'd consider this a blocker for the release, as builds shouldn't fail.
> 
> Generally speaking, I tend to agree, but at the same time, it really
> depends.
> 
> Is this specific to GDB 12, or did we have this issue with previous
> releases?

This has been introduced in GDB 12 development as far as I remember. It 
is similar/related to the following:

https://sourceware.org/pipermail/binutils/2021-November/118485.html

Also discussed slightly in 
https://sourceware.org/bugzilla/show_bug.cgi?id=28684.

I reported it back then: 
https://sourceware.org/pipermail/binutils/2021-November/118554.html.

It might be an easy configure adjustment, but I'm not familiar with bpf.

I know Jose Marchesi did work on bpf, but I'm not sure if he is the 
right PoC.

> 
> We also need some kind of visibility as to how quickly we think we can
> solve that issue. This usually requires someone to act as the issue's
> "champion" -- that person might not be the one actually making the fix,
> but they can at least try help expedite the process.

Agreed. I just want to make sure this has visibility so we can try to 
fix it before release.

> 
>> I get the following:
>>
>> binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/../common/sim-close.c:43:
>> undefined reference to `bpf_cgen_cpu_close'
>>
>> binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/sim-if.c:166:
>> undefined reference to `bpf_cgen_cpu_open_1'
>>
>> binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/sim-if.c:179:
>> undefined reference to `bpf_cgen_init_dis'
>>
>> This is GCC 7.5.0 on Ubuntu 18.04.
>>
>> Should I go ahead and open a ticket against the release? I'm not sure who is
>> responsible for handling BPF.
> 
> I'd start by asking Mike Frysinger, who's the sim maintainer.
> He might not know about this particular target, but he's made
> a lot of cleanups in this area.
> 
> In this case, hopefully the fix won't be too difficult.
> 

cc-ed both Mike and Jose Marchesi.

Thanks,
Luis

Re: GDB 12.0.90 available for testing

[Luis Machado]

[-- Attachment #1: Type: text/plain, Size: 3281 bytes --]

Hi,

On 4/13/22 08:36, Luis Machado wrote:
> Hi Joel,
> 
> On 4/12/22 18:57, Joel Brobecker wrote:
>> Hi Luis,
>>
>>> On 3/20/22 05:58, Joel Brobecker via Gdb-patches wrote:
>>>> Hello,
>>>>
>>>> I have just finished creating the gdb-12.0.90 pre-release.
>>>> It is available for download at the following location:
>>>>
>>>>       ftp://sourceware.org/pub/gdb/snapshots/branch/gdb-12.0.90.tar.xz
>>>>
>>>> A gzip'ed version is also available: gdb-12.0.90.tar.gz.
>>>>
>>>> Please give it a test if you can and report any problems you might 
>>>> find.
>>>>
>>>> On behalf of all the GDB contributors, thank you!
>>>
>>> It seems GDB doesn't build with --enable-targets=all for 32-bit Arm. 
>>> I think
>>> this is a long-standing bug that has not been fixed yet.
>>>
>>> I'd consider this a blocker for the release, as builds shouldn't fail.
>>
>> Generally speaking, I tend to agree, but at the same time, it really
>> depends.
>>
>> Is this specific to GDB 12, or did we have this issue with previous
>> releases?
> 
> This has been introduced in GDB 12 development as far as I remember. It 
> is similar/related to the following:
> 
> https://sourceware.org/pipermail/binutils/2021-November/118485.html
> 
> Also discussed slightly in 
> https://sourceware.org/bugzilla/show_bug.cgi?id=28684.
> 
> I reported it back then: 
> https://sourceware.org/pipermail/binutils/2021-November/118554.html.
> 
> It might be an easy configure adjustment, but I'm not familiar with bpf.
> 
> I know Jose Marchesi did work on bpf, but I'm not sure if he is the 
> right PoC.
> 
>>
>> We also need some kind of visibility as to how quickly we think we can
>> solve that issue. This usually requires someone to act as the issue's
>> "champion" -- that person might not be the one actually making the fix,
>> but they can at least try help expedite the process.
> 
> Agreed. I just want to make sure this has visibility so we can try to 
> fix it before release.
> 
>>
>>> I get the following:
>>>
>>> binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/../common/sim-close.c:43: 
>>>
>>> undefined reference to `bpf_cgen_cpu_close'
>>>
>>> binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/sim-if.c:166: 
>>>
>>> undefined reference to `bpf_cgen_cpu_open_1'
>>>
>>> binutils-gdb-armhf-bionic/sim/bpf/../../../../repos/binutils-gdb/sim/bpf/sim-if.c:179: 
>>>
>>> undefined reference to `bpf_cgen_init_dis'
>>>
>>> This is GCC 7.5.0 on Ubuntu 18.04.
>>>
>>> Should I go ahead and open a ticket against the release? I'm not sure 
>>> who is
>>> responsible for handling BPF.
>>
>> I'd start by asking Mike Frysinger, who's the sim maintainer.
>> He might not know about this particular target, but he's made
>> a lot of cleanups in this area.
>>
>> In this case, hopefully the fix won't be too difficult.
>>
> 
> cc-ed both Mike and Jose Marchesi.
> 
> Thanks,
> Luis

The attached patch makes things build again, though I see a number of 
GDB internal errors when disassembling (for some arch/abi combinations). 
I suspect we're missing some adjustments to make disassembling work

This particular combination of switches and host has not been built in a 
while, so bugs might've been introduced/uncovered.

Mike, Jose, does this look reasonable?

[-- Attachment #2: 0001-Fix-32-bit-build-for-enable-targets-all.patch --]
[-- Type: text/x-patch, Size: 2557 bytes --]

From 387ef2492403c89ac7ac817488a49a3fd7d9d4ba Mon Sep 17 00:00:00 2001
From: Luis Machado <luis.machado@arm.com>
Date: Wed, 13 Apr 2022 11:39:36 +0100
Subject: [PATCH] Fix 32-bit build for --enable-targets=all

The following fixes the GDB build for 32-bit (tested on 32-bit arm)
for the following combinations:

* --enable-targets=all --disable-sim
* --enable-targets=all

I do see quite a few internal errors when running
gdb.base/all-architectures.exp on arm 32-bit Ubuntu 18.04. They all
fail when checking for a default disassembling function, which doesn't
exists for some targets.

This particular combination of switches has not been tested for 32-bit
hosts in a while (since November/December 2021), so there might be bugs
that we need to address. The patch makes things build cleanly though.

Tested on aarch64-linux Ubuntu 20.04 and armhf-linux-gnueabi Ubuntu 18.04.

It would be nice to exercise this on other 32-bit targets.
---
 opcodes/Makefile.am | 10 ++++++++++
 opcodes/Makefile.in | 10 ++++++++++
 2 files changed, 20 insertions(+)

diff --git a/opcodes/Makefile.am b/opcodes/Makefile.am
index afd19fa7785..681fbc07584 100644
--- a/opcodes/Makefile.am
+++ b/opcodes/Makefile.am
@@ -124,6 +124,11 @@ TARGET32_LIBOPCODES_CFILES = \
 	arm-dis.c \
 	avr-dis.c \
 	bfin-dis.c \
+	bpf-asm.c \
+	bpf-desc.c \
+	bpf-dis.c \
+	bpf-ibld.c \
+	bpf-opc.c \
 	cgen-asm.c \
 	cgen-bitset.c \
 	cgen-dis.c \
@@ -178,6 +183,9 @@ TARGET32_LIBOPCODES_CFILES = \
 	lm32-ibld.c \
 	lm32-opc.c \
 	lm32-opinst.c \
+	loongarch-opc.c \
+	loongarch-dis.c \
+	loongarch-coder.c \
 	m10200-dis.c \
 	m10200-opc.c \
 	m10300-dis.c \
@@ -234,6 +242,8 @@ TARGET32_LIBOPCODES_CFILES = \
 	ppc-opc.c \
 	pru-dis.c \
 	pru-opc.c \
+	riscv-dis.c \
+	riscv-opc.c \
 	rl78-decode.c \
 	rl78-dis.c \
 	rx-decode.c \
diff --git a/opcodes/Makefile.in b/opcodes/Makefile.in
index 3ab8bfb0548..d3eee49b169 100644
--- a/opcodes/Makefile.in
+++ b/opcodes/Makefile.in
@@ -516,6 +516,11 @@ TARGET32_LIBOPCODES_CFILES = \
 	arm-dis.c \
 	avr-dis.c \
 	bfin-dis.c \
+	bpf-asm.c \
+	bpf-desc.c \
+	bpf-dis.c \
+	bpf-ibld.c \
+	bpf-opc.c \
 	cgen-asm.c \
 	cgen-bitset.c \
 	cgen-dis.c \
@@ -570,6 +575,9 @@ TARGET32_LIBOPCODES_CFILES = \
 	lm32-ibld.c \
 	lm32-opc.c \
 	lm32-opinst.c \
+	loongarch-opc.c \
+	loongarch-dis.c \
+	loongarch-coder.c \
 	m10200-dis.c \
 	m10200-opc.c \
 	m10300-dis.c \
@@ -626,6 +634,8 @@ TARGET32_LIBOPCODES_CFILES = \
 	ppc-opc.c \
 	pru-dis.c \
 	pru-opc.c \
+	riscv-dis.c \
+	riscv-opc.c \
 	rl78-decode.c \
 	rl78-dis.c \
 	rx-decode.c \
-- 
2.25.1

Re: GDB 12.0.90 available for testing

[Jose E. Marchesi]

> This particular combination of switches and host has not been built in
> a while, so bugs might've been introduced/uncovered.
>
> Mike, Jose, does this look reasonable?

Sure, for BPF.

I wonder how we missed TARGET32_LIBOCODES_CFILES back when we integrated
the bpf backend...

Thanks for fixing this.

> From 387ef2492403c89ac7ac817488a49a3fd7d9d4ba Mon Sep 17 00:00:00 2001
> From: Luis Machado <luis.machado@arm.com>
> Date: Wed, 13 Apr 2022 11:39:36 +0100
> Subject: [PATCH] Fix 32-bit build for --enable-targets=all
>
> The following fixes the GDB build for 32-bit (tested on 32-bit arm)
> for the following combinations:
>
> * --enable-targets=all --disable-sim
> * --enable-targets=all
>
> I do see quite a few internal errors when running
> gdb.base/all-architectures.exp on arm 32-bit Ubuntu 18.04. They all
> fail when checking for a default disassembling function, which doesn't
> exists for some targets.
>
> This particular combination of switches has not been tested for 32-bit
> hosts in a while (since November/December 2021), so there might be bugs
> that we need to address. The patch makes things build cleanly though.
>
> Tested on aarch64-linux Ubuntu 20.04 and armhf-linux-gnueabi Ubuntu 18.04.
>
> It would be nice to exercise this on other 32-bit targets.
> ---
>  opcodes/Makefile.am | 10 ++++++++++
>  opcodes/Makefile.in | 10 ++++++++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/opcodes/Makefile.am b/opcodes/Makefile.am
> index afd19fa7785..681fbc07584 100644
> --- a/opcodes/Makefile.am
> +++ b/opcodes/Makefile.am
> @@ -124,6 +124,11 @@ TARGET32_LIBOPCODES_CFILES = \
>  	arm-dis.c \
>  	avr-dis.c \
>  	bfin-dis.c \
> +	bpf-asm.c \
> +	bpf-desc.c \
> +	bpf-dis.c \
> +	bpf-ibld.c \
> +	bpf-opc.c \
>  	cgen-asm.c \
>  	cgen-bitset.c \
>  	cgen-dis.c \
> @@ -178,6 +183,9 @@ TARGET32_LIBOPCODES_CFILES = \
>  	lm32-ibld.c \
>  	lm32-opc.c \
>  	lm32-opinst.c \
> +	loongarch-opc.c \
> +	loongarch-dis.c \
> +	loongarch-coder.c \
>  	m10200-dis.c \
>  	m10200-opc.c \
>  	m10300-dis.c \
> @@ -234,6 +242,8 @@ TARGET32_LIBOPCODES_CFILES = \
>  	ppc-opc.c \
>  	pru-dis.c \
>  	pru-opc.c \
> +	riscv-dis.c \
> +	riscv-opc.c \
>  	rl78-decode.c \
>  	rl78-dis.c \
>  	rx-decode.c \
> diff --git a/opcodes/Makefile.in b/opcodes/Makefile.in
> index 3ab8bfb0548..d3eee49b169 100644
> --- a/opcodes/Makefile.in
> +++ b/opcodes/Makefile.in
> @@ -516,6 +516,11 @@ TARGET32_LIBOPCODES_CFILES = \
>  	arm-dis.c \
>  	avr-dis.c \
>  	bfin-dis.c \
> +	bpf-asm.c \
> +	bpf-desc.c \
> +	bpf-dis.c \
> +	bpf-ibld.c \
> +	bpf-opc.c \
>  	cgen-asm.c \
>  	cgen-bitset.c \
>  	cgen-dis.c \
> @@ -570,6 +575,9 @@ TARGET32_LIBOPCODES_CFILES = \
>  	lm32-ibld.c \
>  	lm32-opc.c \
>  	lm32-opinst.c \
> +	loongarch-opc.c \
> +	loongarch-dis.c \
> +	loongarch-coder.c \
>  	m10200-dis.c \
>  	m10200-opc.c \
>  	m10300-dis.c \
> @@ -626,6 +634,8 @@ TARGET32_LIBOPCODES_CFILES = \
>  	ppc-opc.c \
>  	pru-dis.c \
>  	pru-opc.c \
> +	riscv-dis.c \
> +	riscv-opc.c \
>  	rl78-decode.c \
>  	rl78-dis.c \
>  	rx-decode.c \

Re: GDB 12.0.90 available for testing

[Joel Brobecker]

Hi Andrew,

What do you think of Pedro's question about the comment being removed?

Thank you!

On Mon, Apr 11, 2022 at 02:42:10PM +0300, Eli Zaretskii wrote:
> > Date: Sun, 10 Apr 2022 12:06:52 -0700
> > From: Joel Brobecker <brobecker@adacore.com>
> > Cc: aburgess@redhat.com, ssbssa@yahoo.de, pedro@palves.net,
> > 	gdb-patches@sourceware.org, brobecker@adacore.com
> > 
> > For the record, I did some testing using the gdb-12-branch, and unlike
> > Hannes, I wasn't able to reproduce.
> > 
> > I tried in various scenarios:
> >   - CMD windows
> >   - ConEmu
> >   - Cygwin terminal (there at least, I know it's not a real tty, FWIW).
> > 
> > This was done on Windows Server 2016 (IIUC, this would correspond
> > to a version of Windows 10, verson 1607).
> > 
> > Since Tom already tested the patch being proposed, other than
> > Pedro's question about the comment being removed, I'm OK with
> > the patch being pushed to both master and gdb-12-branch.
> 
> Thanks.
> 
> Andrew, would you please chime in and tell what you'd like to do with
> that comment?  Whatever you decide, I will do that in the final
> commit.


-- 
Joel

Re: GDB 12.0.90 available for testing

[Joel Brobecker]

Adding binutils@ to the list, since opcode is part of binutils.

Can someone take a look at Luis' patch below, please? Luis noticed
this when he tried to build the GDB 12 release candidate with
--enable-target=all.

Thank you!

On Wed, Apr 13, 2022 at 06:20:24PM +0200, Jose E. Marchesi wrote:
> 
> > This particular combination of switches and host has not been built in
> > a while, so bugs might've been introduced/uncovered.
> >
> > Mike, Jose, does this look reasonable?
> 
> Sure, for BPF.
> 
> I wonder how we missed TARGET32_LIBOCODES_CFILES back when we integrated
> the bpf backend...
> 
> Thanks for fixing this.
> 
> > From 387ef2492403c89ac7ac817488a49a3fd7d9d4ba Mon Sep 17 00:00:00 2001
> > From: Luis Machado <luis.machado@arm.com>
> > Date: Wed, 13 Apr 2022 11:39:36 +0100
> > Subject: [PATCH] Fix 32-bit build for --enable-targets=all
> >
> > The following fixes the GDB build for 32-bit (tested on 32-bit arm)
> > for the following combinations:
> >
> > * --enable-targets=all --disable-sim
> > * --enable-targets=all
> >
> > I do see quite a few internal errors when running
> > gdb.base/all-architectures.exp on arm 32-bit Ubuntu 18.04. They all
> > fail when checking for a default disassembling function, which doesn't
> > exists for some targets.
> >
> > This particular combination of switches has not been tested for 32-bit
> > hosts in a while (since November/December 2021), so there might be bugs
> > that we need to address. The patch makes things build cleanly though.
> >
> > Tested on aarch64-linux Ubuntu 20.04 and armhf-linux-gnueabi Ubuntu 18.04.
> >
> > It would be nice to exercise this on other 32-bit targets.
> > ---
> >  opcodes/Makefile.am | 10 ++++++++++
> >  opcodes/Makefile.in | 10 ++++++++++
> >  2 files changed, 20 insertions(+)
> >
> > diff --git a/opcodes/Makefile.am b/opcodes/Makefile.am
> > index afd19fa7785..681fbc07584 100644
> > --- a/opcodes/Makefile.am
> > +++ b/opcodes/Makefile.am
> > @@ -124,6 +124,11 @@ TARGET32_LIBOPCODES_CFILES = \
> >  	arm-dis.c \
> >  	avr-dis.c \
> >  	bfin-dis.c \
> > +	bpf-asm.c \
> > +	bpf-desc.c \
> > +	bpf-dis.c \
> > +	bpf-ibld.c \
> > +	bpf-opc.c \
> >  	cgen-asm.c \
> >  	cgen-bitset.c \
> >  	cgen-dis.c \
> > @@ -178,6 +183,9 @@ TARGET32_LIBOPCODES_CFILES = \
> >  	lm32-ibld.c \
> >  	lm32-opc.c \
> >  	lm32-opinst.c \
> > +	loongarch-opc.c \
> > +	loongarch-dis.c \
> > +	loongarch-coder.c \
> >  	m10200-dis.c \
> >  	m10200-opc.c \
> >  	m10300-dis.c \
> > @@ -234,6 +242,8 @@ TARGET32_LIBOPCODES_CFILES = \
> >  	ppc-opc.c \
> >  	pru-dis.c \
> >  	pru-opc.c \
> > +	riscv-dis.c \
> > +	riscv-opc.c \
> >  	rl78-decode.c \
> >  	rl78-dis.c \
> >  	rx-decode.c \
> > diff --git a/opcodes/Makefile.in b/opcodes/Makefile.in
> > index 3ab8bfb0548..d3eee49b169 100644
> > --- a/opcodes/Makefile.in
> > +++ b/opcodes/Makefile.in
> > @@ -516,6 +516,11 @@ TARGET32_LIBOPCODES_CFILES = \
> >  	arm-dis.c \
> >  	avr-dis.c \
> >  	bfin-dis.c \
> > +	bpf-asm.c \
> > +	bpf-desc.c \
> > +	bpf-dis.c \
> > +	bpf-ibld.c \
> > +	bpf-opc.c \
> >  	cgen-asm.c \
> >  	cgen-bitset.c \
> >  	cgen-dis.c \
> > @@ -570,6 +575,9 @@ TARGET32_LIBOPCODES_CFILES = \
> >  	lm32-ibld.c \
> >  	lm32-opc.c \
> >  	lm32-opinst.c \
> > +	loongarch-opc.c \
> > +	loongarch-dis.c \
> > +	loongarch-coder.c \
> >  	m10200-dis.c \
> >  	m10200-opc.c \
> >  	m10300-dis.c \
> > @@ -626,6 +634,8 @@ TARGET32_LIBOPCODES_CFILES = \
> >  	ppc-opc.c \
> >  	pru-dis.c \
> >  	pru-opc.c \
> > +	riscv-dis.c \
> > +	riscv-opc.c \
> >  	rl78-decode.c \
> >  	rl78-dis.c \
> >  	rx-decode.c \

-- 
Joel

Re: GDB 12.0.90 available for testing

[Alan Modra]

On Sun, Apr 17, 2022 at 10:33:59AM -0700, Joel Brobecker via Binutils wrote:
> Adding binutils@ to the list, since opcode is part of binutils.
> On Wed, Apr 13, 2022 at 06:20:24PM +0200, Jose E. Marchesi wrote:
> > > From: Luis Machado <luis.machado@arm.com>
> > > diff --git a/opcodes/Makefile.am b/opcodes/Makefile.am
> > > index afd19fa7785..681fbc07584 100644
> > > --- a/opcodes/Makefile.am
> > > +++ b/opcodes/Makefile.am
> > > @@ -124,6 +124,11 @@ TARGET32_LIBOPCODES_CFILES = \
> > >  	arm-dis.c \
> > >  	avr-dis.c \
> > >  	bfin-dis.c \
> > > +	bpf-asm.c \
> > > +	bpf-desc.c \
> > > +	bpf-dis.c \
> > > +	bpf-ibld.c \
> > > +	bpf-opc.c \
> > >  	cgen-asm.c \
> > >  	cgen-bitset.c \
> > >  	cgen-dis.c \

Anything that requires 64-bit BFD support does not belong in
TARGET32_LIBOPCODES_CFILES.  In fact, the whole point of
TARGET32_LIBOPCODES_CFILES was to fix --enable-targets=all breakage on
32-bit hosts without --enable-64-bit-bfd.  Why would you want to put
bpf here?  It's a 64-bit target!

-- 
Alan Modra
Australia Development Lab, IBM

Re: GDB 12.0.90 available for testing

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

>>>> -GNULIB_COMMIT_SHA1="776af40e09b476a41073131a90022572f448c189"
>>>> +GNULIB_COMMIT_SHA1="58c597d13bc57dce3e97ea97856573f2d68ccb8c"

I re-did this with

GNULIB_COMMIT_SHA1="0cda5beb7962f6567f0c4e377df870fa05c6d681"

Pedro> It used to be that the update would tell you what dependencies were
Pedro> new, and which were removed.  Either gnulib changed and doesn't print the same
Pedro> helpful info anymore, or the dependencies really didn't change.
Pedro> To be sure, I'd diff the logs of two different re-imports from scratch:

This adds a two new imports, but they don't seem to be anything that
would really impact gdb:

    +    gen-header
    [...]
       update-copyright
    +    vararrays

Pedro> Regardless, I would suggest that updating is useful to see the
Pedro> dependency updates, and other helpful info, but for final & real
Pedro> update, I'd suggest always reimporting from scratch, to avoid
Pedro> trusting that gnulib's update logic worked as well, that it
Pedro> removed stale files correctly, etc.

Ok, will do.

Tom

Re: GDB 12.0.90 available for testing

[Tom Tromey]

Tom> I re-did this with

Tom> GNULIB_COMMIT_SHA1="0cda5beb7962f6567f0c4e377df870fa05c6d681"

I re-tested this and also did a mingw build.  I'm checking this in on
the trunk now.

thanks,
Tom

Re: GDB 12.0.90 available for testing

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org> writes:

>> I didn't see the details, can you resend them?  gnulib commit ids would
>> be the most convenient but other forms would be alright.

Eli> The relevant Gnulib commits are: 38d0749, c7b1e06, and 21fccfa.  The
Eli> commit messages include the URL of the reports I posted to the Gnulib
Eli> mailing list, which led to the changes.

>> On the release branch we could import selected patches if we think they
>> are safe.

Eli> They are safe, but I have no idea whether you can import them without
Eli> importing other parts of Gnulib.  Specifically, the patches in those
Eli> commits, if applied to GDB, will definitely work in the MinGW builds,
Eli> but I'm not sure about other platforms.  Better to consult with the
Eli> Gnulib folks on that, I guess.

I looked at the patches and I think they look safe to apply.  For the
most part they are obviously specific to Windows.  I'll send the patch
momentarily.

Tom

Re: GDB 12.0.90 available for testing

[Andrew Burgess]

Hi Eli, Joel,

Sorry for the slow reply, I've been off for the last week, so I'm only
just catching up with my GDB emails.

How does the patch below look?  This moves the setbuf call into a new
function, and includes the comment from gdb_readline_no_editing_callback.

Thanks,
Andrew

---

commit 3ee791edccae840e11b9ebe70e5547dfbec04e00
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Tue Apr 19 17:00:04 2022 +0100

    gdb: move setbuf calls out of gdb_readline_no_editing_callback
    
    After this commit:
    
      commit d08cbc5d3203118da5583296e49273cf82378042
      Date:   Wed Dec 22 12:57:44 2021 +0000
    
          gdb: unbuffer all input streams when not using readline
    
    Issues were reported with some MS-Windows hosts, see the thread
    starting here:
    
      https://sourceware.org/pipermail/gdb-patches/2022-March/187004.html
    
    The problem seems to be that calling setbuf on terminal file handles
    is not always acceptable, see this mail for more details:
    
      https://sourceware.org/pipermail/gdb-patches/2022-April/187310.html
    
    This commit does two things, first moving the setbuf calls out of
    gdb_readline_no_editing_callback so that we don't end up calling
    setbuf so often.
    
    Then, for MS-Windows hosts, we don't call setbuf for terminals, this
    appears to resolve the issues that have been reported.

diff --git a/gdb/event-top.c b/gdb/event-top.c
index b628f0397cb..a55338d78a2 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -818,19 +818,6 @@ gdb_readline_no_editing_callback (gdb_client_data client_data)
   FILE *stream = ui->instream != nullptr ? ui->instream : ui->stdin_stream;
   gdb_assert (stream != nullptr);
 
-  /* Unbuffer the input stream, so that, later on, the calls to fgetc
-     fetch only one char at the time from the stream.  The fgetc's will
-     get up to the first newline, but there may be more chars in the
-     stream after '\n'.  If we buffer the input and fgetc drains the
-     stream, getting stuff beyond the newline as well, a select, done
-     afterwards will not trigger.
-
-     This unbuffering was, at one point, not applied if the input stream
-     was a tty, however, the buffering can cause problems, even for a tty,
-     in some cases.  Please ensure that any changes in this area run the MI
-     tests with the FORCE_SEPARATE_MI_TTY=1 flag being passed.  */
-  setbuf (stream, NULL);
-
   /* We still need the while loop here, even though it would seem
      obvious to invoke gdb_readline_no_editing_callback at every
      character entered.  If not using the readline library, the
diff --git a/gdb/top.c b/gdb/top.c
index 1cfffbeee7e..263a2ce8f43 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -257,6 +257,41 @@ void (*deprecated_context_hook) (int id);
 /* The highest UI number ever assigned.  */
 static int highest_ui_num;
 
+/* Unbuffer STREAM.  This is a wrapper around setbuf(STREAM, nullptr)
+   which applies some special rules for MS-Windows hosts.  */
+
+static void
+unbuffer_stream (FILE *stream)
+{
+  /* Unbuffer the input stream so that in gdb_readline_no_editing_callback,
+     the calls to fgetc fetch only one char at the time from STREAM.
+
+     This is important because gdb_readline_no_editing_callback will read
+     from STREAM up to the first '\n' character, after this GDB returns to
+     the event loop and relies on a select on STREAM indicating that more
+     input is pending.
+
+     If STREAM is buffered then the fgetc calls may have moved all the
+     pending input from the kernel into a local buffer, after which the
+     select will not indicate that more input is pending, and input after
+     the first '\n' will not be processed immediately.
+
+     Please ensure that any changes in this area run the MI tests with the
+     FORCE_SEPARATE_MI_TTY=1 flag being passed.  */
+
+#ifdef __MINGW32__
+  /* With MS-Windows runtime, making stdin unbuffered when it's
+     connected to the terminal causes it to misbehave.  */
+  if (!ISATTY (stream))
+    setbuf (stream, nullptr);
+#else
+  /* On GNU/Linux the issues described above can impact GDB even when
+     dealing with input from a terminal.  For now we unbuffer the input
+     stream for everyone except MS-Windows.  */
+  setbuf (stream, nullptr);
+#endif
+}
+
 /* See top.h.  */
 
 ui::ui (FILE *instream_, FILE *outstream_, FILE *errstream_)
@@ -283,6 +318,8 @@ ui::ui (FILE *instream_, FILE *outstream_, FILE *errstream_)
 {
   buffer_init (&line_buffer);
 
+  unbuffer_stream (instream_);
+
   if (ui_list == NULL)
     ui_list = this;
   else
@@ -412,6 +449,8 @@ read_command_file (FILE *stream)
 {
   struct ui *ui = current_ui;
 
+  unbuffer_stream (stream);
+
   scoped_restore save_instream
     = make_scoped_restore (&ui->instream, stream);
 

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, Joel Brobecker <brobecker@adacore.com>, pedro@palves.net
> Date: Tue, 19 Apr 2022 17:12:15 +0100
> 
> Sorry for the slow reply, I've been off for the last week, so I'm only
> just catching up with my GDB emails.
> 
> How does the patch below look?  This moves the setbuf call into a new
> function, and includes the comment from gdb_readline_no_editing_callback.

Thanks.  However, the patch that I tested only used the ISATTY test in
ui::ui, not in read_command.  Not sure if that matters.

Re: GDB 12.0.90 available for testing

[Andrew Burgess]

Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org> writes:

>> From: Andrew Burgess <aburgess@redhat.com>
>> Cc: Eli Zaretskii <eliz@gnu.org>, Joel Brobecker <brobecker@adacore.com>, pedro@palves.net
>> Date: Tue, 19 Apr 2022 17:12:15 +0100
>> 
>> Sorry for the slow reply, I've been off for the last week, so I'm only
>> just catching up with my GDB emails.
>> 
>> How does the patch below look?  This moves the setbuf call into a new
>> function, and includes the comment from gdb_readline_no_editing_callback.
>
> Thanks.  However, the patch that I tested only used the ISATTY test in
> ui::ui, not in read_command.  Not sure if that matters.

I don't think it matters, the call in read_command_file will only be
called with non-tty streams, in which case for MS-Windows we'll still
make the setbuf call.

If for some reason read_command_file does end up reading from a tty then
my belief is we shouldn't be doing the setbuf call for MS-Windows (for
the reasons discussed in this thread).

Thanks,
Andrew

Re: GDB 12.0.90 available for testing

[Joel Brobecker]

Hi Eli,

> > How does the patch below look?  This moves the setbuf call into a new
> > function, and includes the comment from gdb_readline_no_editing_callback.
> 
> Thanks.  However, the patch that I tested only used the ISATTY test in
> ui::ui, not in read_command.  Not sure if that matters.

Would you mind testing this new patch to see if it still works?

-- 
Joel

Re: GDB 12.0.90 available for testing

[Eli Zaretskii]

> Date: Wed, 20 Apr 2022 10:11:36 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Andrew Burgess <aburgess@redhat.com>, gdb-patches@sourceware.org,
> 	brobecker@adacore.com, pedro@palves.net
> 
> Hi Eli,
> 
> > > How does the patch below look?  This moves the setbuf call into a new
> > > function, and includes the comment from gdb_readline_no_editing_callback.
> > 
> > Thanks.  However, the patch that I tested only used the ISATTY test in
> > ui::ui, not in read_command.  Not sure if that matters.
> 
> Would you mind testing this new patch to see if it still works?

Andrew said it wasn't needed, and I believe him.  So feel free to
install that, and thanks.

Re: GDB 12.0.90 available for testing

[Pedro Alves]

On 2022-03-20 05:58, Joel Brobecker via Gdb-patches wrote:
> Hello,
> 
> I have just finished creating the gdb-12.0.90 pre-release.
> It is available for download at the following location:
> 
>     ftp://sourceware.org/pub/gdb/snapshots/branch/gdb-12.0.90.tar.xz
> 
> A gzip'ed version is also available: gdb-12.0.90.tar.gz.
> 
> Please give it a test if you can and report any problems you might find.
> 
> On behalf of all the GDB contributors, thank you!

FYI, I spotted this regression recently:

  "show remote foo-packet" regression
  https://sourceware.org/pipermail/gdb-patches/2022-April/188086.html

I haven't looked much at it beyond bisecting.  I suspect it might just
be a presentation issue, with gdb saying the wrong thing, rather than
gdb potentially doing the wrong thing to the wrong packet.  But I really
haven't confirmed that.

Re: GDB 12.0.90 available for testing

[Joel Brobecker]

Hi Pedro,

> FYI, I spotted this regression recently:
> 
>   "show remote foo-packet" regression
>   https://sourceware.org/pipermail/gdb-patches/2022-April/188086.html
> 
> I haven't looked much at it beyond bisecting.  I suspect it might just
> be a presentation issue, with gdb saying the wrong thing, rather than
> gdb potentially doing the wrong thing to the wrong packet.  But I really
> haven't confirmed that.

Thanks for the heads up. We'll wait to see what the investigation
turns up. I'll add this item to my list of issues to keep an eye on.

-- 
Joel

Re: GDB 12.0.90 available for testing

[Joel Brobecker]

Hi everyone,

> Sorry for the slow reply, I've been off for the last week, so I'm only
> just catching up with my GDB emails.
> 
> How does the patch below look?  This moves the setbuf call into a new
> function, and includes the comment from gdb_readline_no_editing_callback.

Given the feedback received on this patch, and the fact that it was
tagged as important for the GDB 12 release, I performed one last round
of testing on Windows (using AdaCore's testsuite), and pushed Andrew's
patch below to both master and gdb-12-branch.

The one change I made was to the commit message, to add a reference
to the PR created by Eli.

Thanks everyone who contributed to finding, analyzing, fixing and
reviewing!

> commit 3ee791edccae840e11b9ebe70e5547dfbec04e00
> Author: Andrew Burgess <aburgess@redhat.com>
> Date:   Tue Apr 19 17:00:04 2022 +0100
> 
>     gdb: move setbuf calls out of gdb_readline_no_editing_callback
>     
>     After this commit:
>     
>       commit d08cbc5d3203118da5583296e49273cf82378042
>       Date:   Wed Dec 22 12:57:44 2021 +0000
>     
>           gdb: unbuffer all input streams when not using readline
>     
>     Issues were reported with some MS-Windows hosts, see the thread
>     starting here:
>     
>       https://sourceware.org/pipermail/gdb-patches/2022-March/187004.html
>     
>     The problem seems to be that calling setbuf on terminal file handles
>     is not always acceptable, see this mail for more details:
>     
>       https://sourceware.org/pipermail/gdb-patches/2022-April/187310.html
>     
>     This commit does two things, first moving the setbuf calls out of
>     gdb_readline_no_editing_callback so that we don't end up calling
>     setbuf so often.
>     
>     Then, for MS-Windows hosts, we don't call setbuf for terminals, this
>     appears to resolve the issues that have been reported.
> 
> diff --git a/gdb/event-top.c b/gdb/event-top.c
> index b628f0397cb..a55338d78a2 100644
> --- a/gdb/event-top.c
> +++ b/gdb/event-top.c
> @@ -818,19 +818,6 @@ gdb_readline_no_editing_callback (gdb_client_data client_data)
>    FILE *stream = ui->instream != nullptr ? ui->instream : ui->stdin_stream;
>    gdb_assert (stream != nullptr);
>  
> -  /* Unbuffer the input stream, so that, later on, the calls to fgetc
> -     fetch only one char at the time from the stream.  The fgetc's will
> -     get up to the first newline, but there may be more chars in the
> -     stream after '\n'.  If we buffer the input and fgetc drains the
> -     stream, getting stuff beyond the newline as well, a select, done
> -     afterwards will not trigger.
> -
> -     This unbuffering was, at one point, not applied if the input stream
> -     was a tty, however, the buffering can cause problems, even for a tty,
> -     in some cases.  Please ensure that any changes in this area run the MI
> -     tests with the FORCE_SEPARATE_MI_TTY=1 flag being passed.  */
> -  setbuf (stream, NULL);
> -
>    /* We still need the while loop here, even though it would seem
>       obvious to invoke gdb_readline_no_editing_callback at every
>       character entered.  If not using the readline library, the
> diff --git a/gdb/top.c b/gdb/top.c
> index 1cfffbeee7e..263a2ce8f43 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -257,6 +257,41 @@ void (*deprecated_context_hook) (int id);
>  /* The highest UI number ever assigned.  */
>  static int highest_ui_num;
>  
> +/* Unbuffer STREAM.  This is a wrapper around setbuf(STREAM, nullptr)
> +   which applies some special rules for MS-Windows hosts.  */
> +
> +static void
> +unbuffer_stream (FILE *stream)
> +{
> +  /* Unbuffer the input stream so that in gdb_readline_no_editing_callback,
> +     the calls to fgetc fetch only one char at the time from STREAM.
> +
> +     This is important because gdb_readline_no_editing_callback will read
> +     from STREAM up to the first '\n' character, after this GDB returns to
> +     the event loop and relies on a select on STREAM indicating that more
> +     input is pending.
> +
> +     If STREAM is buffered then the fgetc calls may have moved all the
> +     pending input from the kernel into a local buffer, after which the
> +     select will not indicate that more input is pending, and input after
> +     the first '\n' will not be processed immediately.
> +
> +     Please ensure that any changes in this area run the MI tests with the
> +     FORCE_SEPARATE_MI_TTY=1 flag being passed.  */
> +
> +#ifdef __MINGW32__
> +  /* With MS-Windows runtime, making stdin unbuffered when it's
> +     connected to the terminal causes it to misbehave.  */
> +  if (!ISATTY (stream))
> +    setbuf (stream, nullptr);
> +#else
> +  /* On GNU/Linux the issues described above can impact GDB even when
> +     dealing with input from a terminal.  For now we unbuffer the input
> +     stream for everyone except MS-Windows.  */
> +  setbuf (stream, nullptr);
> +#endif
> +}
> +
>  /* See top.h.  */
>  
>  ui::ui (FILE *instream_, FILE *outstream_, FILE *errstream_)
> @@ -283,6 +318,8 @@ ui::ui (FILE *instream_, FILE *outstream_, FILE *errstream_)
>  {
>    buffer_init (&line_buffer);
>  
> +  unbuffer_stream (instream_);
> +
>    if (ui_list == NULL)
>      ui_list = this;
>    else
> @@ -412,6 +449,8 @@ read_command_file (FILE *stream)
>  {
>    struct ui *ui = current_ui;
>  
> +  unbuffer_stream (stream);
> +
>    scoped_restore save_instream
>      = make_scoped_restore (&ui->instream, stream);
>  
> 

-- 
Joel

Re: GDB 12.0.90 available for testing

[Andrew Burgess]

Joel Brobecker via Gdb-patches <gdb-patches@sourceware.org> writes:

> Hi everyone,
>
>> Sorry for the slow reply, I've been off for the last week, so I'm only
>> just catching up with my GDB emails.
>> 
>> How does the patch below look?  This moves the setbuf call into a new
>> function, and includes the comment from gdb_readline_no_editing_callback.
>
> Given the feedback received on this patch, and the fact that it was
> tagged as important for the GDB 12 release, I performed one last round
> of testing on Windows (using AdaCore's testsuite), and pushed Andrew's
> patch below to both master and gdb-12-branch.
>
> The one change I made was to the commit message, to add a reference
> to the PR created by Eli.
>
> Thanks everyone who contributed to finding, analyzing, fixing and
> reviewing!

Thanks for merging this.

Andrew


>
>> commit 3ee791edccae840e11b9ebe70e5547dfbec04e00
>> Author: Andrew Burgess <aburgess@redhat.com>
>> Date:   Tue Apr 19 17:00:04 2022 +0100
>> 
>>     gdb: move setbuf calls out of gdb_readline_no_editing_callback
>>     
>>     After this commit:
>>     
>>       commit d08cbc5d3203118da5583296e49273cf82378042
>>       Date:   Wed Dec 22 12:57:44 2021 +0000
>>     
>>           gdb: unbuffer all input streams when not using readline
>>     
>>     Issues were reported with some MS-Windows hosts, see the thread
>>     starting here:
>>     
>>       https://sourceware.org/pipermail/gdb-patches/2022-March/187004.html
>>     
>>     The problem seems to be that calling setbuf on terminal file handles
>>     is not always acceptable, see this mail for more details:
>>     
>>       https://sourceware.org/pipermail/gdb-patches/2022-April/187310.html
>>     
>>     This commit does two things, first moving the setbuf calls out of
>>     gdb_readline_no_editing_callback so that we don't end up calling
>>     setbuf so often.
>>     
>>     Then, for MS-Windows hosts, we don't call setbuf for terminals, this
>>     appears to resolve the issues that have been reported.
>> 
>> diff --git a/gdb/event-top.c b/gdb/event-top.c
>> index b628f0397cb..a55338d78a2 100644
>> --- a/gdb/event-top.c
>> +++ b/gdb/event-top.c
>> @@ -818,19 +818,6 @@ gdb_readline_no_editing_callback (gdb_client_data client_data)
>>    FILE *stream = ui->instream != nullptr ? ui->instream : ui->stdin_stream;
>>    gdb_assert (stream != nullptr);
>>  
>> -  /* Unbuffer the input stream, so that, later on, the calls to fgetc
>> -     fetch only one char at the time from the stream.  The fgetc's will
>> -     get up to the first newline, but there may be more chars in the
>> -     stream after '\n'.  If we buffer the input and fgetc drains the
>> -     stream, getting stuff beyond the newline as well, a select, done
>> -     afterwards will not trigger.
>> -
>> -     This unbuffering was, at one point, not applied if the input stream
>> -     was a tty, however, the buffering can cause problems, even for a tty,
>> -     in some cases.  Please ensure that any changes in this area run the MI
>> -     tests with the FORCE_SEPARATE_MI_TTY=1 flag being passed.  */
>> -  setbuf (stream, NULL);
>> -
>>    /* We still need the while loop here, even though it would seem
>>       obvious to invoke gdb_readline_no_editing_callback at every
>>       character entered.  If not using the readline library, the
>> diff --git a/gdb/top.c b/gdb/top.c
>> index 1cfffbeee7e..263a2ce8f43 100644
>> --- a/gdb/top.c
>> +++ b/gdb/top.c
>> @@ -257,6 +257,41 @@ void (*deprecated_context_hook) (int id);
>>  /* The highest UI number ever assigned.  */
>>  static int highest_ui_num;
>>  
>> +/* Unbuffer STREAM.  This is a wrapper around setbuf(STREAM, nullptr)
>> +   which applies some special rules for MS-Windows hosts.  */
>> +
>> +static void
>> +unbuffer_stream (FILE *stream)
>> +{
>> +  /* Unbuffer the input stream so that in gdb_readline_no_editing_callback,
>> +     the calls to fgetc fetch only one char at the time from STREAM.
>> +
>> +     This is important because gdb_readline_no_editing_callback will read
>> +     from STREAM up to the first '\n' character, after this GDB returns to
>> +     the event loop and relies on a select on STREAM indicating that more
>> +     input is pending.
>> +
>> +     If STREAM is buffered then the fgetc calls may have moved all the
>> +     pending input from the kernel into a local buffer, after which the
>> +     select will not indicate that more input is pending, and input after
>> +     the first '\n' will not be processed immediately.
>> +
>> +     Please ensure that any changes in this area run the MI tests with the
>> +     FORCE_SEPARATE_MI_TTY=1 flag being passed.  */
>> +
>> +#ifdef __MINGW32__
>> +  /* With MS-Windows runtime, making stdin unbuffered when it's
>> +     connected to the terminal causes it to misbehave.  */
>> +  if (!ISATTY (stream))
>> +    setbuf (stream, nullptr);
>> +#else
>> +  /* On GNU/Linux the issues described above can impact GDB even when
>> +     dealing with input from a terminal.  For now we unbuffer the input
>> +     stream for everyone except MS-Windows.  */
>> +  setbuf (stream, nullptr);
>> +#endif
>> +}
>> +
>>  /* See top.h.  */
>>  
>>  ui::ui (FILE *instream_, FILE *outstream_, FILE *errstream_)
>> @@ -283,6 +318,8 @@ ui::ui (FILE *instream_, FILE *outstream_, FILE *errstream_)
>>  {
>>    buffer_init (&line_buffer);
>>  
>> +  unbuffer_stream (instream_);
>> +
>>    if (ui_list == NULL)
>>      ui_list = this;
>>    else
>> @@ -412,6 +449,8 @@ read_command_file (FILE *stream)
>>  {
>>    struct ui *ui = current_ui;
>>  
>> +  unbuffer_stream (stream);
>> +
>>    scoped_restore save_instream
>>      = make_scoped_restore (&ui->instream, stream);
>>  
>> 
>
> -- 
> Joel

Re: GDB 12.0.90 available for testing

[Luis Machado]

Hi Joel,

Just an update on this. It seems we might need further adjustments to 
makefiles to get 32-bit builds with --enable-targets=all working again.

I'm not sure if we will be able to make it for GDB 12. I'll give it a 
try. Alternatively we could have a backport post-release.

On 4/17/22 18:33, Joel Brobecker wrote:
> Adding binutils@ to the list, since opcode is part of binutils.
> 
> Can someone take a look at Luis' patch below, please? Luis noticed
> this when he tried to build the GDB 12 release candidate with
> --enable-target=all.
> 
> Thank you!
> 
> On Wed, Apr 13, 2022 at 06:20:24PM +0200, Jose E. Marchesi wrote:
>>
>>> This particular combination of switches and host has not been built in
>>> a while, so bugs might've been introduced/uncovered.
>>>
>>> Mike, Jose, does this look reasonable?
>>
>> Sure, for BPF.
>>
>> I wonder how we missed TARGET32_LIBOCODES_CFILES back when we integrated
>> the bpf backend...
>>
>> Thanks for fixing this.
>>
>>>  From 387ef2492403c89ac7ac817488a49a3fd7d9d4ba Mon Sep 17 00:00:00 2001
>>> From: Luis Machado <luis.machado@arm.com>
>>> Date: Wed, 13 Apr 2022 11:39:36 +0100
>>> Subject: [PATCH] Fix 32-bit build for --enable-targets=all
>>>
>>> The following fixes the GDB build for 32-bit (tested on 32-bit arm)
>>> for the following combinations:
>>>
>>> * --enable-targets=all --disable-sim
>>> * --enable-targets=all
>>>
>>> I do see quite a few internal errors when running
>>> gdb.base/all-architectures.exp on arm 32-bit Ubuntu 18.04. They all
>>> fail when checking for a default disassembling function, which doesn't
>>> exists for some targets.
>>>
>>> This particular combination of switches has not been tested for 32-bit
>>> hosts in a while (since November/December 2021), so there might be bugs
>>> that we need to address. The patch makes things build cleanly though.
>>>
>>> Tested on aarch64-linux Ubuntu 20.04 and armhf-linux-gnueabi Ubuntu 18.04.
>>>
>>> It would be nice to exercise this on other 32-bit targets.
>>> ---
>>>   opcodes/Makefile.am | 10 ++++++++++
>>>   opcodes/Makefile.in | 10 ++++++++++
>>>   2 files changed, 20 insertions(+)
>>>
>>> diff --git a/opcodes/Makefile.am b/opcodes/Makefile.am
>>> index afd19fa7785..681fbc07584 100644
>>> --- a/opcodes/Makefile.am
>>> +++ b/opcodes/Makefile.am
>>> @@ -124,6 +124,11 @@ TARGET32_LIBOPCODES_CFILES = \
>>>   	arm-dis.c \
>>>   	avr-dis.c \
>>>   	bfin-dis.c \
>>> +	bpf-asm.c \
>>> +	bpf-desc.c \
>>> +	bpf-dis.c \
>>> +	bpf-ibld.c \
>>> +	bpf-opc.c \
>>>   	cgen-asm.c \
>>>   	cgen-bitset.c \
>>>   	cgen-dis.c \
>>> @@ -178,6 +183,9 @@ TARGET32_LIBOPCODES_CFILES = \
>>>   	lm32-ibld.c \
>>>   	lm32-opc.c \
>>>   	lm32-opinst.c \
>>> +	loongarch-opc.c \
>>> +	loongarch-dis.c \
>>> +	loongarch-coder.c \
>>>   	m10200-dis.c \
>>>   	m10200-opc.c \
>>>   	m10300-dis.c \
>>> @@ -234,6 +242,8 @@ TARGET32_LIBOPCODES_CFILES = \
>>>   	ppc-opc.c \
>>>   	pru-dis.c \
>>>   	pru-opc.c \
>>> +	riscv-dis.c \
>>> +	riscv-opc.c \
>>>   	rl78-decode.c \
>>>   	rl78-dis.c \
>>>   	rx-decode.c \
>>> diff --git a/opcodes/Makefile.in b/opcodes/Makefile.in
>>> index 3ab8bfb0548..d3eee49b169 100644
>>> --- a/opcodes/Makefile.in
>>> +++ b/opcodes/Makefile.in
>>> @@ -516,6 +516,11 @@ TARGET32_LIBOPCODES_CFILES = \
>>>   	arm-dis.c \
>>>   	avr-dis.c \
>>>   	bfin-dis.c \
>>> +	bpf-asm.c \
>>> +	bpf-desc.c \
>>> +	bpf-dis.c \
>>> +	bpf-ibld.c \
>>> +	bpf-opc.c \
>>>   	cgen-asm.c \
>>>   	cgen-bitset.c \
>>>   	cgen-dis.c \
>>> @@ -570,6 +575,9 @@ TARGET32_LIBOPCODES_CFILES = \
>>>   	lm32-ibld.c \
>>>   	lm32-opc.c \
>>>   	lm32-opinst.c \
>>> +	loongarch-opc.c \
>>> +	loongarch-dis.c \
>>> +	loongarch-coder.c \
>>>   	m10200-dis.c \
>>>   	m10200-opc.c \
>>>   	m10300-dis.c \
>>> @@ -626,6 +634,8 @@ TARGET32_LIBOPCODES_CFILES = \
>>>   	ppc-opc.c \
>>>   	pru-dis.c \
>>>   	pru-opc.c \
>>> +	riscv-dis.c \
>>> +	riscv-opc.c \
>>>   	rl78-decode.c \
>>>   	rl78-dis.c \
>>>   	rx-decode.c \
> 

Re: GDB 12.0.90 available for testing

[Joel Brobecker]

hi Luis,

> Just an update on this. It seems we might need further adjustments to
> makefiles to get 32-bit builds with --enable-targets=all working again.
> 
> I'm not sure if we will be able to make it for GDB 12. I'll give it a try.

Thanks for the update, Luis. Seeing the emails, I realized that
I hadn't understood that this was specific to 32-bit, which I am
understanding as using a 32-bit toolchain instead of a 64-bit one.

Based on that, my first step next weekend was going to first confirm
we do not have that issue when building with a 64-bit platform.
If not, then I would say that the problem is probably not as
serious as I initially thought, and thus we can go ahead and
release without a fix.

And my next was to look at the effect of --enable-64-bit-bfd to
see if that helps. Maybe you could try that?

> Alternatively we could have a backport post-release.

That would be my recommended option anyway. I mean, maybe we'll find
a fix that's sufficiently obvious that we can safely backport it
right away and then release. But I have a feeling that it won't be
like that, in which case, I think a bit of maturing time on master
before deciding to backport might be beneficial, I think.

-- 
Joel

Re: GDB 12.0.90 available for testing

[Luis Machado]

Joel,

On 4/26/22 15:56, Joel Brobecker wrote:
> hi Luis,
> 
>> Just an update on this. It seems we might need further adjustments to
>> makefiles to get 32-bit builds with --enable-targets=all working again.
>>
>> I'm not sure if we will be able to make it for GDB 12. I'll give it a try.
> 
> Thanks for the update, Luis. Seeing the emails, I realized that
> I hadn't understood that this was specific to 32-bit, which I am
> understanding as using a 32-bit toolchain instead of a 64-bit one.
> 

That's correct. It is only a 32-bit toolchain problem, and likely the 
result of gdb/sim assuming they can include *all* the targets and BFD 
being a bit more strict about it.

> Based on that, my first step next weekend was going to first confirm
> we do not have that issue when building with a 64-bit platform.

I expect a 64-bit platform won't run into such a problem.

> If not, then I would say that the problem is probably not as
> serious as I initially thought, and thus we can go ahead and
> release without a fix.
> 
> And my next was to look at the effect of --enable-64-bit-bfd to
> see if that helps. Maybe you could try that?

If you --disable-sim, that works. It seems the simulators have some code 
in them not ready for --enable-64-bit-bfd.

So, in summary:

* --enable-targets=all without --enable-64-bit-bfd for 32-bit should not 
assume 64-bit BFD targets can be included.

* --enable-targets=all --disable-sim --enable-64-bit-bfd works correctly.

> 
>> Alternatively we could have a backport post-release.
> 
> That would be my recommended option anyway. I mean, maybe we'll find
> a fix that's sufficiently obvious that we can safely backport it
> right away and then release. But I have a feeling that it won't be
> like that, in which case, I think a bit of maturing time on master
> before deciding to backport might be beneficial, I think.
> 

Agreed.