From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19442 invoked by alias); 28 Aug 2015 16:07: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 18783 invoked by uid 89); 28 Aug 2015 16:07:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pa0-f41.google.com Received: from mail-pa0-f41.google.com (HELO mail-pa0-f41.google.com) (209.85.220.41) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 28 Aug 2015 16:07:00 +0000 Received: by pabzx8 with SMTP id zx8so67081174pab.1 for ; Fri, 28 Aug 2015 09:06:58 -0700 (PDT) X-Received: by 10.68.136.234 with SMTP id qd10mr16669975pbb.162.1440778018687; Fri, 28 Aug 2015 09:06:58 -0700 (PDT) Received: from seba.sebabeach.org.gmail.com (173-13-178-53-sfba.hfc.comcastbusiness.net. [173.13.178.53]) by smtp.gmail.com with ESMTPSA id a2sm6228957pas.47.2015.08.28.09.06.57 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 28 Aug 2015 09:06:58 -0700 (PDT) From: Doug Evans To: "Ulrich Weigand" Cc: gdb-patches@sourceware.org Subject: Re: Cell multi-arch symbol lookup broken (Re: [PATCH] solib_global_lookup: Fetch arch from objfile.) References: <20150828133818.F0A1B8775@oc7340732750.ibm.com> Date: Fri, 28 Aug 2015 16:07:00 -0000 In-Reply-To: <20150828133818.F0A1B8775@oc7340732750.ibm.com> (Ulrich Weigand's message of "Fri, 28 Aug 2015 15:38:18 +0200 (CEST)") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2015-08/txt/msg00817.txt.bz2 "Ulrich Weigand" writes: > Doug Evans wrote: > >> So I get now that there is an inferior gdbarch that is potentially >> distinct from whatever gdbarch one has at hand (e.g., an objfile's), >> [that's the easy part] >> and that certain API calls *have* to use the inferior gdbarch. >> [that's the subtle part] >> >> One question that comes to mind is why don't the other gdbarches >> in the system, e.g., the spu's, delegate these calls to its "parent" gdbarch? >> Any design that requires global state like this is suboptimal, >> and I'd like to see gdb move away from it wherever possible. >> [Here we have a gdbarch from the objfile, but we can't use it.] >> >> Another thought that comes to mind is, if we don't want "child gdbarches" >> to delegate to "parent gdbarches" (and I haven't decided myself), then >> another way to go is to use different types. >> E.g., inferior_gdbarch and gdbarch. >> [I might name inferior_gdbarch differently, depending on what API calls >> it contains, but this would be another way to avoid potential confusion >> over what the correct gdbarch to use is in any particular situation.] > > Well, it's a bit more subtle, even. First of all, there's two distinct > "classes" of gdbarch structures, corresponding to the "symbol side" and > the "target side" of GDB, respectively. On the symbol side, we have all > the information that is determined solely by looking at an objfile and > well-known ABI conventions. On the target side, we have in addition all > the information that is determined by the running target (e.g. register > names etc.). For more details, see here: > https://sourceware.org/ml/gdb-patches/2007-12/msg00142.html Thanks for the reference. > The "symbol side" gdbarch is used as the architecture associated with > objfiles, types, and symbols. The "target side" gdbarch is used as > the architecture associated with the inferior as a whole, with a > thread, or with a stack frame. > > Now, in both symbol side and target side, there may be different > architecture instances active during a single debug session. In > particular, during Cell/B.E. debugging, there will be PowerPC > objfiles and SPU objfiles. In addition, some threads and/or stack > frames will be of PowerPC architecture, and others of SPU architecture. > The main inferior will be of PowerPC architecture (in mixed debugging) > or of SPU architecture (when doing SPU stand-alone debugging). > (With multi-inferior debugging, we may have different main inferior > architectures at the same time as well.) This much I get. :-) > Now, I tend to agree that there should be some sort of child/parent > relationship between a symbol-side gdbarch and a target-side gdbarch. > Specifically, a target-side gdbarch should *contain* a symbol-side > gdbarch *and additional information*. ... and we should make them separate types. > However, this should still > apply only to the same basic architecture, i.e. the PowerPC target > gdbarch should include the PowerPC symbol gdbarch, and likewise > for SPU. [ I originally wanted to implement this as two distinct > data types, but this will require a significant amount of refactoring > work, and in the end I never got around to doing this. ] Heh. Good to know. [And I'm not complaining btw.] > But I don't think there should be a direct relationship between > the SPU arch and the PowerPC arch, as you describe above. > Execution may be in a PowerPC frame on an SPU thread running > in a PowerPC inferior ... but there's no relationship between > the architectures as such. When using any of these, you just > have to know whether you're interested in a property of the > current frame, current thread, or current inferior. Well, I wouldn't read too much into my suggestion. It was pragmatic and pragmatism has tradeoffs by definition. [For reference sake, I was envisioning setting up the hierarchy at runtime when we knew what kind of hierarchy was actually present. As for *which* architecture to ultimately use that problem is still up to the user/caller.] > Now, the "target_gdbarch" is a bit of a special case. It used > to have a distinct semantics: the architecture used at the > target interface level. (Think of it as the architecture that > defines the contents of the register packets of the remote > gdbserver protocol.) However, since the remote protocol was > extended to actually support using *multiple* different > architectures, there's no longer a need for target_gdbarch > as a distinct concept. And in fact, it's now simply defined > as the architecture of the current inferior: > > struct gdbarch * > target_gdbarch (void) > { > return current_inferior ()->gdbarch; > } > > I'd be in favor of inlining this function into every user, > and starting to replace "current_inferior" with any given > inferior that we may be actually operating on in these > places. I'm all for removal of references to global state (where appropriate) and passing in the needed context. IWBN to also clean up the types while we're at it. >> I'm ok with reverting this particular part of the patch, >> though it'd be helpful to add a comment explaining >> why things are the way they are. > > There is a comment in gdbarch.h, but that may not be in > a prominent enough place: > > /* The architecture associated with the inferior through the > connection to the target. > > The architecture vector provides some information that is really a > property of the inferior, accessed through a particular target: > ptrace operations; the layout of certain RSP packets; the solib_ops > vector; etc. To differentiate architecture accesses to > per-inferior/target properties from > per-thread/per-frame/per-objfile properties, accesses to > per-inferior/target properties should be made through this > gdbarch. */ Yeah, I found that, but too late. I was suggesting putting a comment at the call site. [While putting such comments at all such call sites has the potential to reduce readability, not improve it (if taken to an extreme :-)); until we use types/APIs that make the code self-documenting, I like the comments.] > Maybe one way to make this obvious would be to change solib_ops > to take an inferior instead of a gdbarch as argument ... In the particular case of solib_ops it does seem like gdbarch is the wrong "this/self" parameter. Thanks.