[RFC] breakpoints/19474 [was Re: RFC: branching for GDB 7.11 soon? (possibly Wed)]

[Keith Seitz <keiths at redhat dot com>]

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

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.  */

 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;
     }