From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25564 invoked by alias); 21 Jun 2013 14:45:04 -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 25538 invoked by uid 89); 21 Jun 2013 14:45:03 -0000 X-Spam-SWARE-Status: No, score=-8.2 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; Fri, 21 Jun 2013 14:45:02 +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 r5LEixQ7028953 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 21 Jun 2013 10:44:59 -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 r5LEivjj028559; Fri, 21 Jun 2013 10:44:58 -0400 Message-ID: <51C466E8.1040706@redhat.com> Date: Fri, 21 Jun 2013 14:58: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> <51C350A6.1020302@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-06/txt/msg00589.txt.bz2 On 06/20/2013 09:42 PM, Maciej W. Rozycki wrote: > On Thu, 20 Jun 2013, Pedro Alves wrote: >> 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. > > Well, the rule isn't exactly that -- the rule is the kernel headers > installed in the system for use by user programs must be exactly those the > C library (typically glibc) has been built against. Otherwise all hell is > bound to break loose. Ah. Yeah, that was it. > Of course not everyone is prepared to build their > own C library, and we cannot expect one to be. Certainly. > Actually the case is I believe even more complicated. The rule expressed > by Linus as I remember it used to be not to make use of the kernel headers > in user programs at all. Any definitions and declarations required for > user programs were expected to be either folded into the relevant C > library headers (e.g. glibc frequently chooses for such > stuff), or bundled with the piece of user software that relies on the > relevant subsystem (that used to be the case with the more obscure stuff, > e.g. all the programs relying on the netlink stuff used to do that, and > perhaps still do). > > What we currently have in GDB with regard to MIPS hardware watchpoint > functionality carefully follows the second choice. Now times are changing > and I reckon over the years a project has been under way to split the > kernel headers into a strictly internal set and an exported user-ABI set, > with the latter intended to make user program developers' lives easier. Yeah, I'm aware of that UAPI header split project as well. I'd sooner believe we ended up with the structs in GDB because of the desire to use the feature in GDB with an updated kernel, but not an updated libc (Daney did the kernel side bits, afterall), rather than due to special care to follow the second choice, though. > I haven't tracked progress there and I don't know offhand how far the > project has gone, but unless you have a better knowledge of what the > situation is there and can give good counterarguments, we certainly have > to be careful especially with such an old Linux port the MIPS one is. No special knowledge, but I wasn't arguing for not being careful either. >> 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. > > Well, with my upstream reviewer's hat on I try to avoid assuming one's > build environment is going to be as sophisticated as one used by those who > professionally work on toolchain components. The minimal and probably > still typical recipe of a random developer or even system administrator > out there roughly is: > > $ ./configure && make && make install Certainly. The details were escaping me, but indeed I was thinking of glibc building. > > and I think our objective is to make this as smooth as possible, avoiding > making people pull their hair. Professional software packagers will cope > either way, we don't need to care that much about them. *nod* >> 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? > > We have the choice between PTRACE_GET_WATCH_REGS and > PTRACE_SET_WATCH_REGS only here, there have been no other macros added > with the Linux commit 0926bf953ee79b8f139741b442e5a18520f81705 Yao > referred to above. Looks like those are good enough to me. I notice the code added to GDB does: #ifndef PTRACE_GET_WATCH_REGS # define PTRACE_GET_WATCH_REGS 0xd0 #endif #ifndef PTRACE_SET_WATCH_REGS # define PTRACE_SET_WATCH_REGS 0xd1 #endif and only does that #ifndef dance in those two particular macros... But I'd hope that was more an historical accident than the case of Daney finding a system with conflicting PTRACE_GET/SET_WATCH_REGS definitions (and no corresponding structs)... > The change regrettably has the maximum architectural > number of watchpoint register sets possible to implement in hardware > hardcoded throughout as literal 8 rather than using a macro like our > MAX_DEBUG_REGISTER (a misnomer actually, as I noted elsewhere). Yeah, to make that accept anything but 8, we'd have to change the struct's layouts. Could have worked for SET without breaking the ABI if num_valid was the first field in the struct, alas, it isn't... For GET, we'd be stuck anyway, as the caller would have no indication of the size of the struct the kernel would fill in. As is, MAX_DEBUG_REGISTER is just an ABI array length constant. -- Pedro Alves