From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14214 invoked by alias); 22 Mar 2013 19:54:12 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 14200 invoked by uid 89); 22 Mar 2013 19:54:04 -0000 X-Spam-SWARE-Status: No, score=-8.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 22 Mar 2013 19:54:01 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r2MJrw9L009777 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 22 Mar 2013 15:53:58 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r2MJru6n015102; Fri, 22 Mar 2013 15:53:57 -0400 Message-ID: <514CB6D4.9070909@redhat.com> Date: Fri, 22 Mar 2013 20:45:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4 MIME-Version: 1.0 To: Jan Kratochvil CC: gdb-patches@sourceware.org, Hui Zhu Subject: Re: [patch+7.6] Fix 7.5 regression crashing GDB if gdbserver dies References: <20130315195359.GA19841@host2.jankratochvil.net> <514C50EF.6030202@redhat.com> <20130322191841.GA29259@host2.jankratochvil.net> In-Reply-To: <20130322191841.GA29259@host2.jankratochvil.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-03/txt/msg00864.txt.bz2 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