From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15265 invoked by alias); 20 Jun 2013 18:57:49 -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 15252 invoked by uid 89); 20 Jun 2013 18:57:48 -0000 X-Spam-SWARE-Status: No, score=-8.1 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_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; Thu, 20 Jun 2013 18:57:46 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r5KIviBX022056 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 20 Jun 2013 14:57:44 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r5KIvgED019474; Thu, 20 Jun 2013 14:57:43 -0400 Message-ID: <51C350A6.1020302@redhat.com> Date: Thu, 20 Jun 2013 19:40:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6 MIME-Version: 1.0 To: "Maciej W. Rozycki" CC: Yao Qi , gdb-patches@sourceware.org Subject: Re: [PATCH 1/3] Include asm/ptrace.h in mips-linux-nat.c References: <1369881867-11372-1-git-send-email-yao@codesourcery.com> <1369881867-11372-2-git-send-email-yao@codesourcery.com> <51BA8252.3010303@codesourcery.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-06/txt/msg00561.txt.bz2 On 06/14/2013 01:37 PM, Maciej W. Rozycki wrote: > On Fri, 14 Jun 2013, Yao Qi wrote: > >>> ../../src/gdb/mips-linux-nat.c:599: error: storage size of 'dummy_regs' >>> isn't known >>> >>> etc. for the very reason the system I used for testing has old kernel >>> headers installed (2.6.19 it would seem, according to ). >>> >>> Such a failure is not acceptable from the user's point of view; I think >>> there are three ways to deal with this: >>> >>> 1. Add an autoconf test that checks for the presence of a key >>> definition; I think 'struct pt_watch_regs' is a good >>> candidate. If that test does not succeed, then the configure process >>> fails gracefully stating the minimum released version of kernel headers >>> required. >>> >>> 2. Add the same test, except in the failure case fall back to the internal >>> definitions we already have, wrapped into #ifndef >>> HAVE_STRUCT_PT_WATCH_REGS. >>> >>> 3. Add the same test and disable hardware watchpoint support in the >>> failure case. >> >> I prefer #3. If 'struct pt_watch_regs' is not defined, hardware watchpoint is >> not supported in the kernel. Because 'struct pt_watch_regs' was added in this >> commit in linux kernel, >> >> commit 0926bf953ee79b8f139741b442e5a18520f81705 >> Author: David Daney >> Date: Tue Sep 23 00:11:26 2008 -0700 >> >> MIPS: Ptrace support for HARDWARE_WATCHPOINTS >> >> This is the final part of the watch register patch. Here we hook up >> ptrace so that the user space debugger (gdb), can set and read the >> registers. >> >> Signed-off-by: David Daney >> Signed-off-by: Ralf Baechle > > Well, kernel headers bundled with the C library installed on a system GDB > is being built on do not necessarily have to match the kernel binary the > system uses (as is the case with the system I'm using for this validation) > or GDB may be run on another system. So you cannot infer at the build > time whether the kernel used on a system GDB is going to run on supports > watchpoints until you actually query the kernel at the run time; you need > to do a run-time check anyway as hardware may not even if the kernel will. > > Therefore I think my question here is a matter of policy in GDB rather > than limitations posed by a system in a given configuration. Which makes > your preference valid, although not for the reason you stated. We don't have a written or very fixed policy, afaik. For new ports, we try to push back on adding such fallback definitions, in order push the submitter into actually adding them to their right place (the kernel headers). (otherwise, it's been somewhat common that the defines don't actually make it there! E.g., PT_TEXT_ADDR and friends...) Then, we prefer assuming minimum kernel headers version the first released version with all the necessary bits for the port. For new features, things get a bit more blurry. Given kernel headers guarantee backwards compatibility, it's my understanding that you should always be able to build against the latest kernel headers, even if you're running against an old kernel. ISTR that's how CS builds things. However, naturally not everyone will do that. Most will rely on the system's kernel version's headers. Weighing the maintenance cost of a autoconf glue plus #ifdefs around the feature's code, vs defining a couple fallback constants ourselves, I'd say the latter is simpler and less effort, thus my preference. If we're talking about a set of missing complicated structure definitions, then I may have the opposite opinion. In this case, I was borderline when I initially reviewed the Daney's watchpoints's code, but I accepted it then. Given they're already in place, it just sounds like causing unnecessary trouble to me to remove them, more so you've shown an example where they're necessary. I'd vote for keeping them. Supposedly, MAX_DEBUG_REGISTER or some other convenient define was added to the kernel's asm/ptrace.h at the same time as struct pt_watch_regs etc, so we could still include asm/ptrace.h, and only define the fallbacks if that such macro is not defined? -- Pedro Alves