From patchwork Thu Feb 4 00:45:11 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Keith Seitz X-Patchwork-Id: 10730 Received: (qmail 5108 invoked by alias); 4 Feb 2016 00:45:15 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 5094 invoked by uid 89); 4 Feb 2016 00:45:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=sals, ascertain, Filter, reconstruction X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 04 Feb 2016 00:45:14 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 99365C00230B; Thu, 4 Feb 2016 00:45:12 +0000 (UTC) Received: from valrhona.uglyboxes.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u140jBhr030431 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 3 Feb 2016 19:45:12 -0500 Subject: [RFC] breakpoints/19474 [was Re: RFC: branching for GDB 7.11 soon? (possibly Wed)] To: "gdb-patches@sourceware.org ml" References: <20160201030638.GG4008@adacore.com> <56AFB750.3030702@redhat.com> From: Keith Seitz X-Enigmail-Draft-Status: N1110 Cc: Yao Qi Message-ID: <56B29F17.6020603@redhat.com> Date: Wed, 3 Feb 2016 16:45:11 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <56AFB750.3030702@redhat.com> X-IsSubscribed: yes On 02/01/2016 11:51 AM, Keith Seitz wrote: > I started looking at this last week, and I have an analysis of the > problems. It all basically boils down to: we cannot (reliably) win in > this situation. Well, it turns out I'm wrong. Or at least as not-wrong as the test suite demonstrates. > Aside: It seems to me that instead of doing basename searches and using > CWD, this whole thing should really do/support directory mappings, e.g., > the user can say, "whenever you see /home/keiths/tmp/foo, look instead > in /home/keiths/tmp/bar." But I don't think that really addresses the > problem at hand. And the astute reader knows that we already have this! "set substitute-path" Does the trick. > The only solution I've come up with is changing the whole API so that in > this specific case (searching for a known symtab), we reject searching > the source path. As you can imagine, that could open up an entirely new > "can of worms." [And that change would be quite intrusive.] At Pedro's urging, I found another, simpler solution. The big problem with just about all solutions I was attempting to implement is [p]symtab_to_fululname/find_and_open_source/openp. These functions are just completely unaware of what the caller is attempting to do. So first I want to point out that this "bug" has existed for quite some time. I've only test back to 7.7 (earlier releases do not build on my machine without non-trivial intervention), but all suffer from this problem. Having said that, the scheme I've come up with restricts this "hack" to create_sals_line_offset, where the problem is observed. It is also here where we know we are attempting to fetch a list of symtabs for the default source file, meaning all symtabs with the "same" file name as default symtab. So the simple (but apparently working?) algorithm simply compares the default symtab's fullname vs a reconstruction of the SYMTAB_DIRNAME and basename of the "collected" symtab. [It also considers substitute-path.] I would consider this fairly risky for inclusion into a release without sufficient testing in HEAD, but when it comes to releases, I tend to be quite conservative. I've appended the patch below. This patch causes no regressions in the test suite, and it fixes 19474. Before: $ ./gdb ~/tmp/f Reading symbols from /home/keiths/tmp/f...done. (gdb) cd ~/tmp Working directory /home/keiths/tmp. (gdb) start Temporary breakpoint 1 at 0x4006e6: file f.c, line 6. Starting program: /home/keiths/tmp/f Temporary breakpoint 1, main () at f.c:6 6 return foo () + bar_in_main (); (gdb) b 12 Breakpoint 2 at 0x4006ff: /home/keiths/tmp/f.c:12. (2 locations) After: $ ./gdb ~/tmp/f Reading symbols from /home/keiths/tmp/f...done. (gdb) cd ~/tmp Working directory /home/keiths/tmp. (gdb) start Temporary breakpoint 1 at 0x4006e6: file f.c, line 6. Starting program: /home/keiths/tmp/f Temporary breakpoint 1, main () at f.c:6 6 return foo () + bar_in_main (); (gdb) b 12 Breakpoint 2 at 0x4006ff: file f.c, line 12. I'm sure there are corner cases and a whole bunch of other problems with this approach, but at least it is isolated to one place (for better or worse). Anyone have a better idea? Keith static struct symtabs_and_lines @@ -1851,6 +1911,7 @@ create_sals_line_offset (struct linespec_state *self, VEC_free (symtab_ptr, ls->file_symtabs); ls->file_symtabs = collect_symtabs_from_filename (fullname, self->search_pspace); + ls->file_symtabs = filter_default_symtabs (fullname, ls->file_symtabs); use_default = 1; } diff --git a/gdb/linespec.c b/gdb/linespec.c index 2360cc1..c0f7020 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -1816,6 +1816,66 @@ canonicalize_linespec (struct linespec_state *state, const linespec_p ls) } } +/* Filter the list of SYMTABS with the name of the default symtab given + by FULLNAME. + + When collecting symtabs from the fullname of the default symtab, + it is possible for openp to get it wrong (because it often searches + the current working directory using the basename of the file) if files + with the same base name exist. + + This function attempts to ascertain if symtabs in the given list + really do match the given fullname of the default symtab. */ + +static VEC (symtab_ptr) * +filter_default_symtabs (const char *fullname, VEC (symtab_ptr) *symtabs) +{ + int ix; + struct symtab *symtab; + VEC (symtab_ptr) *filtered_symtabs = NULL; + struct cleanup *back_to = make_cleanup (null_cleanup, NULL); + + /* Iterate through the symtabs, searching for matches to FULLNAME. */ + for (ix = 0; VEC_iterate (symtab_ptr, symtabs, ix, symtab); ++ix) + { + const char *basename = lbasename (fullname); + char *symtab_with_dir; + + if (SYMTAB_DIRNAME (symtab) == NULL) + continue; + + symtab_with_dir = concat (SYMTAB_DIRNAME (symtab), SLASH_STRING, + basename, NULL); + make_cleanup (xfree, symtab_with_dir); + if (streq (fullname, symtab_with_dir)) + VEC_safe_push (symtab_ptr, filtered_symtabs, symtab); + else + { + /* Now try any path substitution rules. */ + symtab_with_dir = rewrite_source_path (symtab_with_dir); + if (symtab_with_dir != NULL) + { + make_cleanup (xfree, symtab_with_dir); + if (streq (fullname, symtab_with_dir)) + VEC_safe_push (symtab_ptr, filtered_symtabs, symtab); + } + } + } + + /* If we found no matches, use whatever symtabs were originally + "collected." */ + if (filtered_symtabs == NULL) + { + /* Found no exact matches -- use the original list. */ + filtered_symtabs = symtabs; + } + else + VEC_free (symtab_ptr, symtabs); + + do_cleanups (back_to); + return filtered_symtabs; +} + /* Given a line offset in LS, construct the relevant SALs. */