From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19686 invoked by alias); 24 Jun 2010 20:49:17 -0000 Received: (qmail 19677 invoked by uid 22791); 24 Jun 2010 20:49:16 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,SPF_HELO_PASS,TW_BJ,TW_CP,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (74.125.121.35) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 24 Jun 2010 20:49:11 +0000 Received: from wpaz13.hot.corp.google.com (wpaz13.hot.corp.google.com [172.24.198.77]) by smtp-out.google.com with ESMTP id o5OKn6xG004334 for ; Thu, 24 Jun 2010 13:49:07 -0700 Received: from pvg4 (pvg4.prod.google.com [10.241.210.132]) by wpaz13.hot.corp.google.com with ESMTP id o5OKn55H005067 for ; Thu, 24 Jun 2010 13:49:05 -0700 Received: by pvg4 with SMTP id 4so390544pvg.28 for ; Thu, 24 Jun 2010 13:49:05 -0700 (PDT) MIME-Version: 1.0 Received: by 10.115.39.21 with SMTP id r21mr10265488waj.155.1277412544907; Thu, 24 Jun 2010 13:49:04 -0700 (PDT) Received: by 10.220.201.66 with HTTP; Thu, 24 Jun 2010 13:49:04 -0700 (PDT) In-Reply-To: References: <4C20103B.1080906@redhat.com> <201006221045.23196.pedro@codesourcery.com> <4C20DA1A.10301@redhat.com> Date: Thu, 24 Jun 2010 20:49:00 -0000 Message-ID: Subject: Re: [RFA] c++/11734 From: Doug Evans To: tromey@redhat.com Cc: Keith Seitz , gdb-patches@sourceware.org, Pedro Alves Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-System-Of-Record: true 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: 2010-06/txt/msg00562.txt.bz2 On Thu, Jun 24, 2010 at 1:00 PM, Tom Tromey wrote: >>>>>> "Doug" =3D=3D Doug Evans writes: > > Doug> There may be more going on here though, I don't understand why > Doug> decode_line_1 has *both* is_quoted and is_quote_enclosed (is there > Doug> some cleanup that can be done here, or is there a technical reason > Doug> to handle " different from '?). > > Based on my last foray into decode_line_1, I would say that it is likely > that nobody knows the answer to this question. > > That code is seriously twisted and horrible. =A0E.g., see PR 8883, or > 11289, or 11614. =A0I think we should plan to rewrite it -- Keith was > talking about this a while ago... maybe he got scared off ;-) > > In the short run I am inclined to approve anything that fixes problems > and doesn't regress. IWBN to keep the code at least as coherent as it is now. Having decode_compound strip quotes doesn't do this for me. [This feels easy to fix, so I don't see this as asking too much, fwiw,] > Doug> There are several callers and rather than changing all of them to > Doug> strip method overloading of the name to search for, it seems > Doug> reasonable to handle it in lookup_partial_symbol. > > One thing I would like to know is the exact semantics required of these > lookup functions. =A0That is, if the implementation of one or more quick > functions is expected to strip the "(" stuff, then that ought to be > documented in symfile.h. I think that, for this particular case, it comes down to having a definition of what *is* recorded in a psymtab for overloaded methods (and documenting it of course). > Doug> IWBN if psymtabs didn't require that complexity and *only* recorded > Doug> the un-overloaded name. > > And then...? =A0I suppose we could expand all psymtabs with a matching > name, then let the symtab code be intelligent about picking out matches. My premise for saying that is that psymtabs, at least for the case at hand, *do* store the un-overloaded name. My point was IWBN to not have them sometimes store un-overloaded names and sometimes store overloaded names (i.e., don't be inconsistent). Of course, if they always *only* stored overloaded names then that's another way, but they don't do that today (and I *think* that's intentional). > It is pretty easy to add a pre-expansion call like this. =A0This is what > the index branch does -- it records names only, not symbol types, so > before a name lookup is done it reads all the CUs that define the name, > regardless of how it is defined there. > > FWIW, Sami is working on an approach like this for template functions. > I think it is a promising approach, though I'm concerned about how we > will handle different instantiations of the same unadorned name > appearing in multiple objfiles. > > Doug> AIUI, what you're doing here is having the lookup "fail" if an > Doug> overloaded-name is found, so that a subsequent search in the full > Doug> symtab will be done and, having expanded the psymtab here, that sea= rch > Doug> will succeed. =A0However psymtabs are searched *after* symtabs. =A0= This > Doug> patch works because it turns out that we will later do a search in = the > Doug> full symtab, but that's more by accident than design: > > Do you know what exact code path is taken here? Essentially we're in linespec.c:find_methods, given "foo()" (from "break c::foo()") and are looking up all the overloaded methods. Digging deeper, the caller of lookup_partial_symbol for the case at hand is lookup_symbol_aux_psymtabs, and it does call PSYMTAB_TO_SYMTAB (as do other callers, except one, but it doesn't need to). So we don't have to call PSYMTAB_TO_SYMTAB in lookup_partial_symbol and only need to return the match. And it's caller is lookup_symbol_aux_quick ... but we've returned NULL so it early exits. Changing lookup_partial_symbol to simply strip the overloading info before doing the comparison sounds good. diff -u -p -r1.5 psymtab.c --- psymtab.c 24 Jun 2010 20:17:52 -0000 1.5 +++ psymtab.c 24 Jun 2010 20:45:03 -0000 @@ -432,6 +432,7 @@ lookup_partial_symbol (struct partial_sy struct partial_symbol **top, **real_top, **bottom, **center; int length =3D (global ? pst->n_global_syms : pst->n_static_syms); int do_linear_search =3D 1; + char *simple_name =3D NULL, *paren; if (length =3D=3D 0) { @@ -441,6 +442,16 @@ lookup_partial_symbol (struct partial_sy pst->objfile->global_psymbols.list + pst->globals_offset : pst->objfile->static_psymbols.list + pst->statics_offset); + /* FIXME: What about "(anonymous namespace)". */ + paren =3D strchr (name, '('); + if (paren) + { + simple_name =3D alloca (strlen (name)); + memcpy (simple_name, name, paren - name); + simple_name[name - paren] =3D '\0'; + name =3D simple_name; + } + if (global) /* This means we can use a binary search. */ { do_linear_search =3D 0; I added the FIXME just as a reminder, I wasn't suggesting checking this in of course. > I think what is intended to happen is that a call to a quick function > may result in symtab expansion. =A0Then the caller is supposed to look in > this symtab for the answer. =A0E.g., lookup_symbol_aux_quick does this. I see that now. > Doug> This situation is not well solved by our normal psymtabs->symtabs l= azy > Doug> expansion design. > Doug> I don't have a specific proposal for a better fix at the moment. > Doug> Comments? > > We should probably also plan on a symtab overhaul. No disagreement here. > Right now gdb's behavior is dependent on the order of psymtab expansion. > This can result in weird results for users, even for pretty ordinary > code. =A0I don't think this is very good. =A0Pre-expansion would fix this, > though maybe at a cost; and maybe that alone would introduce new > problems as well. > > Also I think we have some bugs that we could fix if symtab lookups > returned all the symbols for a given name -- not just the first one > matching. =A0Perhaps we would even need to extend this to work across > objfiles; I imagine we have plenty of lurking bugs in this scenario. > > We may also want to consider the multi-inferior objfile reuse problem > when doing this. =A0Perhaps a new symtab API could return relocated > symbols or something along those lines. > > I'm very interested to learn about other cases where we have trouble. > > Tom >