From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30963 invoked by alias); 5 Jun 2009 17:54:33 -0000 Received: (qmail 30953 invoked by uid 22791); 5 Jun 2009 17:54:32 -0000 X-SWARE-Spam-Status: No, hits=-3.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from main.gmane.org (HELO ciao.gmane.org) (80.91.229.2) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 05 Jun 2009 17:54:25 +0000 Received: from list by ciao.gmane.org with local (Exim 4.43) id 1MCdcS-00016R-Dr for gdb-patches@sources.redhat.com; Fri, 05 Jun 2009 17:54:22 +0000 Received: from mobius.qnx.com ([209.226.137.108]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Fri, 05 Jun 2009 17:54:20 +0000 Received: from aristovski by mobius.qnx.com with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Fri, 05 Jun 2009 17:54:20 +0000 To: gdb-patches@sources.redhat.com From: Aleksandar Ristovski Subject: Re: ptid from core section Date: Fri, 05 Jun 2009 17:54:00 -0000 Message-ID: References: <4A23F9FF.8040708@qnx.com> <200906051539.46280.pedro@codesourcery.com> <200906051726.51861.pedro@codesourcery.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit User-Agent: Thunderbird 2.0.0.21 (Windows/20090302) In-Reply-To: <200906051726.51861.pedro@codesourcery.com> X-IsSubscribed: yes 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 X-SW-Source: 2009-06/txt/msg00104.txt.bz2 Pedro Alves wrote: > On Friday 05 June 2009 15:53:00, Aleksandar Ristovski wrote: > >> Here is a new patch. I made sure it works even if >> core_gdbarch is NULL. > > Sorry, we're going a bit in circles. The way you've now > implemented it, there are three ways to get a ptid > from a core reg section, but two of them are mostly > the same --- the default gdbarch fallbacks, and the > case of core_gdbarch being NULL handled in corelow.c itself. On nto, we are not using lwp field at all. We use thread id. In nto, I override core_open to add thread private data once core_open has finished it's work. To identify threads, I need to build ptid the way core does. I can do that, but then I will have two functions for adding thread private data (one for core ops, one for live target). Also, I would have to either figure out which field is being used in *_ptid_to_str or again provide two functions. In summary: the motivation for this patch is to avoid having to patch corelow.c. I am working on a larger nto patch that I would like to have in before next official gdb release. > > On Friday 05 June 2009 14:44:37, Pedro Alves wrote: >> change adds some unconditional accesses. The path of >> least resistence to fix this, is to move the callback defaults >> to corelow.c, make the new callbacks optional, and check >> for 'core_gdbarch && gdbarch_foo_p (core_gdbarch)' predicates >> before calling the optional callbacks. > > This meant: > > On Friday 05 June 2009 15:53:00, Aleksandar Ristovski wrote: >> +static ptid_t >> +default_ptid_from_core_section_name (struct gdbarch *gdbarch, const bfd *abfd, >> + const char *name) >> +{ >> + int thread_id; >> + ptid_t ptid; >> + const char *pos; >> + >> + pos = strchr (name, '/'); >> + if (pos == NULL) >> + pos = name + strlen (name); >> + else >> + pos++; >> + thread_id = atoi (pos); >> + ptid = ptid_build (ptid_get_pid (inferior_ptid), thread_id, 0); >> + return ptid; >> +} >> + >> +static char * >> +default_core_section_name_from_ptid (struct gdbarch *gdbarch, >> + const bfd *abfd, >> + const char *name, >> + ptid_t ptid) >> +{ >> + if (ptid_get_lwp (ptid)) >> + return xstrprintf ("%s/%ld", name, ptid_get_lwp (ptid)); >> + else >> + return xstrdup (name); >> +} > > Moving these functions to corelow.c, and making the gdbarch callbacks > optional. Wouldn't that look cleaner? Something like the patch below. Looking at core handling I think the whole thing is not clean. I see your point, but I see no particular advantage of your approach - we moved the default code to corelow but now introduced check for gdbarch_..._p. But don't get me wrong - I am not against your approach - as long as I don't have to patch corelow in order to get gdb working for neutrino. > > But, at this point, I'm now confused, and I have to re-ask: > What is it that gets confused on nto when core files store > the thread id in the lwp field of ptids instead of on the tid > field? Your original patch only took care to adjust to > read tids from the tid field in default_core_section_from_ptid, > but didn't do anything to make ptids that stored the tid > in the tid field in default_ptid_from_core_section? While > your latest patch didn't even do that in > default_ptid_from_core_section_name? I can't see how you > avoid adding gdbarch callbacks for nto. I don't, I provide my own callbacks similar to sol2-tdep.c, only they build ptid something like this: ptid_build (ptid_get_pid (ptid), 0, atoi (core_section_name + 5)); I reworked the latest patch to have exactly the same behaviour as without the patch, except for one bit where I check if both lwp and tid are 0 to set inferior_ptid. > >> I also fixed my previous patch for >> target signal by checking if there is a core_gdbarch. > > Could you go ahead, and check in just that bit > split out from the rest please? Thanks. > Sure. -- Aleksandar Ristovski QNX Software Systems