RFC for: "Re: Regression for gdb.fortran/library-module.exp [Re: [RFA] choose symbol from given block's objfile first.]"

[Joel Brobecker <brobecker@adacore.com>]

[-- Attachment #1: Type: text/plain, Size: 1050 bytes --]

Hello everyone,

Here is something that implements a compromise between the previous
behavior, and the new behavior in terms of objfile search order.

The gist of the patch is that it introduces an iterator that can
take a "context" objfile into account for the iteration order.
The rest are small adjustements to use the iterator. Long term,
I think we are opening the door for more arch-dependency by
adjusting the body of the iterator. I'll leave that part to someone
else who's motivated (I think it's a can of worms).

This patch assumes that my other patch has been reverted.  This way,
it allows us to start from the initial code, and see how I am
changing it this time.

It also includes a C testcase, which hopefully reproduces the same
issue as gdb.fortran/library-module.exp, but in C, to widen the
number of people testing this feature.

For now, it's only a prototype, so I'm seeking comments. I will
document the code when we're happy with the approach.

Tested on x86_64-linux, no regression. Fixes the new testcase.

Thanks,
-- 
Joel

[-- Attachment #2: 0001-WIP.patch --]
[-- Type: text/x-diff, Size: 12550 bytes --]

From da06bff34e77513ee10b664e009cd0c871ad5f82 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Wed, 16 May 2012 11:33:26 -0700
Subject: [PATCH] WIP.

---
 gdb/findvar.c                         |   29 +++++++++-
 gdb/objfiles.c                        |   46 ++++++++++++++
 gdb/objfiles.h                        |    8 +++
 gdb/symtab.c                          |  104 ++++++++++++++++++++++----------
 gdb/testsuite/gdb.base/cp-rel-lib.c   |   29 +++++++++
 gdb/testsuite/gdb.base/cp-rel-var.c   |   36 +++++++++++
 gdb/testsuite/gdb.base/cp-rel-var.exp |   48 +++++++++++++++
 7 files changed, 266 insertions(+), 34 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/cp-rel-lib.c
 create mode 100644 gdb/testsuite/gdb.base/cp-rel-var.c
 create mode 100644 gdb/testsuite/gdb.base/cp-rel-var.exp

diff --git a/gdb/findvar.c b/gdb/findvar.c
index 9009e6f..2f6fb29 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -406,6 +406,25 @@ symbol_read_needs_frame (struct symbol *sym)
   return 1;
 }
 
+struct minsym_lookup_data
+{
+  const char *name;
+  struct minimal_symbol *result;
+};
+
+static int
+minsym_lookup_iterator_cb (struct objfile *objfile, void *cb_data)
+{
+  struct minsym_lookup_data *data = (struct minsym_lookup_data *)cb_data;
+
+  gdb_assert (data->result == NULL);
+
+  data->result = lookup_minimal_symbol (data->name, NULL, objfile);
+
+  /* The iterator should stop iff a match was found.  */
+  return (data->result != NULL);
+}
+
 /* A default implementation for the "la_read_var_value" hook in
    the language vector which should work in most situations.  */
 
@@ -559,10 +578,18 @@ default_read_var_value (struct symbol *var, struct frame_info *frame)
 
     case LOC_UNRESOLVED:
       {
+	struct minsym_lookup_data lookup_data;
 	struct minimal_symbol *msym;
 	struct obj_section *obj_section;
 
-	msym = lookup_minimal_symbol (SYMBOL_LINKAGE_NAME (var), NULL, NULL);
+	memset (&lookup_data, 0, sizeof (lookup_data));
+	lookup_data.name = SYMBOL_LINKAGE_NAME (var);
+
+	iterate_over_objfiles_in_search_order (minsym_lookup_iterator_cb,
+					       &lookup_data,
+					       SYMBOL_SYMTAB (var)->objfile);
+	msym = lookup_data.result;
+
 	if (msym == NULL)
 	  error (_("No global symbol \"%s\"."), SYMBOL_LINKAGE_NAME (var));
 	if (overlay_debugging)
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 8d9f8a5..8dffac5 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -1525,6 +1525,52 @@ gdb_bfd_unref (struct bfd *abfd)
   xfree (name);
 }
 
+/* FIXME: Document.  */
+
+void
+iterate_over_objfiles_in_search_order
+  (iterate_over_objfiles_in_search_order_cb cb,
+   void *cb_data,
+   struct objfile *context_objfile)
+{
+  int stop = 0;
+  struct objfile *objfile;
+
+  if (context_objfile && context_objfile->flags & OBJF_MAINLINE)
+    {
+      stop = cb (context_objfile, cb_data);
+      if (stop)
+	return;
+    }
+
+  ALL_OBJFILES (objfile)
+    {
+      if (objfile->flags & OBJF_MAINLINE && objfile != context_objfile)
+	{
+	  stop = cb (objfile, cb_data);
+	  if (stop)
+	    return;
+	}
+    }
+
+  if (context_objfile && !(context_objfile->flags & OBJF_MAINLINE))
+    {
+      stop = cb (context_objfile, cb_data);
+      if (stop)
+	return;
+    }
+
+  ALL_OBJFILES (objfile)
+    {
+      if (!(objfile->flags & OBJF_MAINLINE) && objfile != context_objfile)
+	{
+	  stop = cb (objfile, cb_data);
+	  if (stop)
+	    return;
+	}
+    }
+}
+
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 extern initialize_file_ftype _initialize_objfiles;
 
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index d5c807f..ca204db 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -525,6 +525,14 @@ extern void *objfile_data (struct objfile *objfile,
 extern struct bfd *gdb_bfd_ref (struct bfd *abfd);
 extern void gdb_bfd_unref (struct bfd *abfd);
 extern int gdb_bfd_close_or_warn (struct bfd *abfd);
+
+typedef int (*iterate_over_objfiles_in_search_order_cb)
+  (struct objfile *objfile, void *cb_data);
+
+extern void iterate_over_objfiles_in_search_order
+  (iterate_over_objfiles_in_search_order_cb cb,
+   void *cb_data,
+   struct objfile *context_objfile);
 \f
 
 /* Traverse all object files in the current program space.
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 0ed42fd..2486c9a 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -1500,40 +1500,55 @@ lookup_global_symbol_from_objfile (const struct objfile *main_objfile,
   return NULL;
 }
 
-/* Check to see if the symbol is defined in one of the symtabs.
-   BLOCK_INDEX should be either GLOBAL_BLOCK or STATIC_BLOCK,
+/* Check to see if the symbol is defined in one of the OBJFILE's
+   symtabs.  BLOCK_INDEX should be either GLOBAL_BLOCK or STATIC_BLOCK,
    depending on whether or not we want to search global symbols or
    static symbols.  */
 
 static struct symbol *
+lookup_symbol_aux_objfile (struct objfile *objfile, int block_index,
+			   const char *name, const domain_enum domain)
+{
+  struct symbol *sym = NULL;
+  struct blockvector *bv;
+  const struct block *block;
+  struct symtab *s;
+
+  if (objfile->sf)
+    objfile->sf->qf->pre_expand_symtabs_matching (objfile, block_index,
+						  name, domain);
+
+  ALL_OBJFILE_SYMTABS (objfile, s)
+    if (s->primary)
+      {
+	bv = BLOCKVECTOR (s);
+	block = BLOCKVECTOR_BLOCK (bv, block_index);
+	sym = lookup_block_symbol (block, name, domain);
+	if (sym)
+	  {
+	    block_found = block;
+	    return fixup_symbol_section (sym, objfile);
+	  }
+      }
+
+  return NULL;
+}
+
+/* Same as lookup_symbol_aux_objfile, except that it searches all
+   objfiles.  Return the first match found.  */
+
+static struct symbol *
 lookup_symbol_aux_symtabs (int block_index, const char *name,
 			   const domain_enum domain)
 {
   struct symbol *sym;
   struct objfile *objfile;
-  struct blockvector *bv;
-  const struct block *block;
-  struct symtab *s;
 
   ALL_OBJFILES (objfile)
   {
-    if (objfile->sf)
-      objfile->sf->qf->pre_expand_symtabs_matching (objfile,
-						    block_index,
-						    name, domain);
-
-    ALL_OBJFILE_SYMTABS (objfile, s)
-      if (s->primary)
-	{
-	  bv = BLOCKVECTOR (s);
-	  block = BLOCKVECTOR_BLOCK (bv, block_index);
-	  sym = lookup_block_symbol (block, name, domain);
-	  if (sym)
-	    {
-	      block_found = block;
-	      return fixup_symbol_section (sym, objfile);
-	    }
-	}
+    sym = lookup_symbol_aux_objfile (objfile, block_index, name, domain);
+    if (sym)
+      return sym;
   }
 
   return NULL;
@@ -1650,6 +1665,33 @@ lookup_symbol_static (const char *name,
     return NULL;
 }
 
+struct global_sym_lookup_data
+{
+  const char *name;
+  domain_enum domain;
+  struct symbol *result;
+};
+
+static int
+lookup_symbol_global_iterator_cb (struct objfile *objfile,
+				  void *cb_data)
+{
+  struct global_sym_lookup_data *data = 
+    (struct global_sym_lookup_data *) cb_data;
+
+  gdb_assert (data->result == NULL);
+
+  data->result = lookup_symbol_aux_objfile (objfile, GLOBAL_BLOCK,
+					    data->name, data->domain);
+  if (data->result == NULL)
+    data->result = lookup_symbol_aux_quick (objfile, GLOBAL_BLOCK,
+					    data->name, data->domain);
+
+  /* If we found a match, tell the iterator to stop.  Otherwise,
+     keep going.  */
+  return (data->result != NULL);
+}
+
 /* Lookup a symbol in all files' global blocks (searching psymtabs if
    necessary).  */
 
@@ -1660,6 +1702,7 @@ lookup_symbol_global (const char *name,
 {
   struct symbol *sym = NULL;
   struct objfile *objfile = NULL;
+  struct global_sym_lookup_data lookup_data;
 
   /* Call library-specific lookup procedure.  */
   objfile = lookup_objfile_from_block (block);
@@ -1668,18 +1711,13 @@ lookup_symbol_global (const char *name,
   if (sym != NULL)
     return sym;
 
-  sym = lookup_symbol_aux_symtabs (GLOBAL_BLOCK, name, domain);
-  if (sym != NULL)
-    return sym;
+  memset (&lookup_data, 0, sizeof (lookup_data));
+  lookup_data.name = name;
+  lookup_data.domain = domain;
+  iterate_over_objfiles_in_search_order (lookup_symbol_global_iterator_cb,
+					 &lookup_data, objfile);
 
-  ALL_OBJFILES (objfile)
-  {
-    sym = lookup_symbol_aux_quick (objfile, GLOBAL_BLOCK, name, domain);
-    if (sym)
-      return sym;
-  }
-
-  return NULL;
+  return lookup_data.result;
 }
 
 int
diff --git a/gdb/testsuite/gdb.base/cp-rel-lib.c b/gdb/testsuite/gdb.base/cp-rel-lib.c
new file mode 100644
index 0000000..d2c6984
--- /dev/null
+++ b/gdb/testsuite/gdb.base/cp-rel-lib.c
@@ -0,0 +1,29 @@
+/* This testcase is part of GDB, the GNU debugger.
+   Copyright 2012 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+
+int cp_rel_var = 1;
+
+int
+get_cp_rel_var (void)
+{
+  if (cp_rel_var != 1)
+    abort ();
+
+  cp_rel_var = 2;
+  return cp_rel_var;  /* LIB_STOP */
+}
diff --git a/gdb/testsuite/gdb.base/cp-rel-var.c b/gdb/testsuite/gdb.base/cp-rel-var.c
new file mode 100644
index 0000000..fd7f751
--- /dev/null
+++ b/gdb/testsuite/gdb.base/cp-rel-var.c
@@ -0,0 +1,36 @@
+/* This testcase is part of GDB, the GNU debugger.
+   Copyright 2012 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+
+int cp_rel_var;
+
+extern int get_cp_rel_var (void);
+
+int
+main (void)
+{
+  int tmp;
+
+  if (cp_rel_var != 1)
+    abort ();
+
+  tmp = get_cp_rel_var ();
+  if (cp_rel_var != 2)  /* VAR_STOP */
+    abort ();
+
+  return (tmp != cp_rel_var);
+}
diff --git a/gdb/testsuite/gdb.base/cp-rel-var.exp b/gdb/testsuite/gdb.base/cp-rel-var.exp
new file mode 100644
index 0000000..aa99379
--- /dev/null
+++ b/gdb/testsuite/gdb.base/cp-rel-var.exp
@@ -0,0 +1,48 @@
+# Copyright 2012 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+set executable cp-rel-var
+
+set libsrc "${srcdir}/${subdir}/cp-rel-lib.c"
+set libobj "${objdir}/${subdir}/libcp-rel-lib.so"
+set libobj_opts { debug additional_flags=-fPIC }
+
+if { [gdb_compile_shlib $libsrc $libobj $libobj_opts ] != "" } {
+    return -1
+}
+
+if { [gdb_compile "${srcdir}/${subdir}/${executable}.c" \
+                  "${objdir}/${subdir}/${executable}" \
+                  executable \
+                  [list debug shlib=${libobj}]]
+     != ""} {
+    return -1
+}
+
+clean_restart $executable
+
+if ![runto_main] {
+    untested "could not run to main"
+    return -1
+}
+
+gdb_breakpoint $libsrc:[gdb_get_line_number "STOP" $libsrc]
+gdb_continue_to_breakpoint "LIB_STOP" ".*LIB_STOP.*"
+gdb_test "print cp_rel_var" " = 2" "print cp_rel_var from lib"
+
+gdb_breakpoint ${executable}.c:[gdb_get_line_number "STOP" ${executable}.c]
+gdb_continue_to_breakpoint "VAR_STOP" ".*VAR_STOP.*"
+gdb_test "print cp_rel_var" " = 2" "print cp_rel_var from main"
+
-- 
1.7.1