Re: [PATCH v3 3/7] gdbserver: suppress "Detaching from process" message

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2021-12-03 18:42, Pedro Alves wrote:
> On 2021-12-01 14:44, Simon Marchi via Gdb-patches wrote:
>> From: Simon Marchi <simon.marchi@efficios.com>
>>
>> The motivation behind this patch is that a following patch in the series
>> adds a test that constantly spawns forks.  This makes GDBserver flood
>> its stdout with "Detaching from process" messages.  Because the
>> testsuite infrastructure does not consume GDBserver's stdout, the pipe
>> fills up quickly, and GDBserver eventually blocks on a write to stdout
>> forever.
>>
>> I first tried to make the testsuite consume GDBserver's stdout, but that
>> wasn't very pretty.  We would need to do it at various random points,
> 
> Did you try doing it from within gdb_test_multiple?  Like, adding something
> like '-i $server_spawnid" "." { exp_continue }' to the built-in matches?
> I guess that might require an option to suppress that match.

When re-reading this, I understood that you meant changing
gdb_test_multiple itself.  That might not be a bad idea, but as you
said, it might require an option to not do it, as some tests do manually
consume the server_spawn_id output.

Before re-reading though, I thought you mean to use gdb_test_multiple in
my test to consume the GDBserver output.  So that's what I implemented,
and I think it's a reasonable solution for now.  My last patch will be
updated with this.

> Hmm, maybe instead, use expect_after/expect_before ?

I don't need to match anything else, just consume all that GDBserver
outputs while we sleep.  So just a single gdb_test_multiple + after
works fine.

>> and it could easily break depending on the particular timing.  Note that
>> it could still be useful to have the testsuite consume GDBserver's
>> output from time to time, so that we could see any GDBserver output in
>> the logs, but it would not be a reliable way of handling this particular
>> case, where GDBserver outputs so much.
> 
> OOC, WDYM, by "not be a reliable way"?

I don't quite remember what I was thinking about when I wrote that.
When I was trying to fix this, I tried to make the testsuite consume the
GDBserver inside gdb_test_multiple, but as a separate expect call,
before the main one.  So while we would drain all GDBserver's output at
that point, the command we then issue to GDB could cause GDBserver to
output a lot of messages, and fill up its buffer again.  But if we use
a `-i $server_spawn_id` clause inside the main expect call, then we
would consume the GDBserver output while waiting for the GDB command to
finish, so it would work fine.

So, disregard what I said.

>> I'm wondering if we could simply remove that message, if anybody would
>> miss it.  Personally, I don't think it's particularly useful.  The user
>> already gets notifications in GDB, if that's what they want.
>>
>> An alternative would be to add an option to control whether we print
>> this or that message.  But I'd like to avoid adding options and increase
>> the complexity if that's not really necessary.
> 
> A "This or that" option, I agree, that'd be too much.  Maybe --verbose
> would make sense.  Not sure.  See below.
> 
>>
>> I thought about starting GDBserver with its stdout redirected to
>> /dev/null, since we don't consume it anyway, but that wouldn't work: we
>> actually consume the output a little bit in the beginning to see if
>> GDBserver was started successfully, or if the specified port was already
>> taken.
>>
>> Change-Id: Ib371a4bcaeb6dfb5c03077e816019c8473d4d198
> 
> The thing to me is that this looks like an arbitrary change in
> isolation.

It is.  I sent this knowing it wasn't right, as I was stuck without a
solution.

> I mean, GDBserver prints this on attach:
> 
>   fprintf (stderr, "Attached; pid = %d\n", pid);
> 
> and this on kill / 'k':
> 
>   fprintf (stderr, "Killing all inferiors\n");
> 
> and likely other messages in other cases, including warnings.  Do we want a
> policy that gdbserver never prints anything after startup?  Maybe we do.
> Maybe add a "from_tty" argument to some functions, and only print if
> we get there due to a command line option?  So traffic from GDB would not
> cause extra prints.  I'd think you'd want to do something about those
> messages mentioned above too and perhaps others, all at the same time.
> 
> Maybe it makes sense to have a --verbose mode that isn't full-blown
> debug output, and put messages under it.  Not sure.  Maybe we don't and
> instead move the messages under --debug.
> 
> Let's discuss the policy.  I'm fine with incremental progress toward it,
> but we need to know where we're going.

Since we don't need this for the current patch, it will be work for
another day.  But yeah, I think it would make sense for GDBserver to
output nothing by default about its regular business, except for some
error messages that could help the user know that something went wrong.

Simon