Re: [patch+7.6] Fix 7.5 regression crashing GDB if gdbserver dies

[Pedro Alves <palves at redhat dot com>]

On 03/22/2013 07:18 PM, Jan Kratochvil wrote:
> On Fri, 22 Mar 2013 13:39:11 +0100, Pedro Alves wrote:
>> The 22 in "E22" is not really a "trace api error" (whatever that
>> meant for the original non-public tracepoint target in GDB years and
>> years ago; the errors handled in trace_error are not really specified
>> in the RSP docs).  It just happened that kgdb leaked
>> EINVAL (=22)
> 
> In such case the patch should be limited specifically for E22, done.
> 
> 
>> I had closed PR 12146 as invalid back in 2010, because people that
>> use kgdb are people hacking on the kernel, so if they stumble upon
>> this kgdb bug, they should pull/merge the relevant kernel fix into
>> their trees instead of gdb working around the kgdb bug.
> [...]
>> A couple of years more have passed, which makes the old kgdbs even
>> less relevant, so IMO, we should just go ahead and revert the
>> initial workaround.
> 
> Red Hat is actively supporting RHEL-5 linux-2.6.18 from 2006.  It does not yet
> had kgdb but it illustrates other production systems with kernels from
> 2008..2010 may use kgdb.   On production system one cannot feasibly change the
> kernel just because of a bugfix for debugging purposes.

Debugging a live production system with kgdb?  I don't buy
that, really.  If one cares to debug the kernel with kgdb, then
one can just as easily patch the kernel, just like one patches the
kernel for security bugs and the like.  It's not like the kernel
is never ever updated on such systems.

> Therefore your patch will be a new regression against FSF GDB 7.5.

For people debugging the an old kernel with kgdb, the people that
are fixing a bug or writing a driver and will end up patching
the kernel...  And the "regression" is only a problem for the
subset of those that can't patch the kernel.  Seems like the
empty set to me.

I don't know about the history of the original motivation for
Hui's patch.  Was pushback done to whoever found the issue, to
see if the kernel could be fixed instead?  Why couldn't that kernel
be updated instead back then?

> People still face for example the bug
> 	new gdbserver is incompat. with old gdb - ;core: suffix
> 	http://sourceware.org/bugzilla/show_bug.cgi?id=15230
> which is "fixed" in GDB (also) since 2010:
> 	commit 3d972c80ab566c07f5e55d356821fb883c9ade88
> 	Date:   Tue Jan 12 21:40:23 2010 +0000
> 

I've said before that I regret not having noticed that problem
at patch review time or before the release.  I still do.  I don't
see the parallel here though. We can't go back in time and fix older
GDBs to cope with ;core...  Users can upgrade their GDBs though.

But let's not generalize.  These problems need to be considered
case by case, and my comments apply to particulars of the kgdb
case, alone.

> As I said I leave gdbserver up to you but I do not much understand why to
> break compatibility when the fix is simple and done.

Because it's a workaround that does not need to exist.  It adds
maintenance burden in the wrong place.  The time we've wasted
collectively iterating on this shows it.

> 
> 
>> If we went this path, I believe it would be the "connection
>> closed" that should get special treatment as being a hard
>> error that usually you'd want to propagate to some higher
>> level and react by canceling all ongoing commands and cleaning
>> up.  I've occasionally thought of adding it on other occasions
>> before.
> 
> OK, done.
> 
> 
>> We should just eliminate trace_error -- it's magical error
>> handling is meaningless today.
> 
> Done.
> 

I like this version much better than the original.  If the exception
swallowing ever turns out problematic again, I'll propose reverting
the original patch again.  So in interest of moving forward, this one's
fine with me.

-- 
Pedro Alves