Fix "list ambiguous_variable"

The "list" command allows specifying the name of variables as
argument, not just functions, so that users can type "list
a_global_variable".

That support is a broken when it comes to ambiguous locations though.

If there's more than one such global variable in the program, e.g.,
static globals in different compilation units, GDB ends up listing the
source of the first variable it finds, only.

linespec.c does find both symbol and minsym locations for all the
globals.  The problem is that it ends up merging all the resulting
sals into one, because they all have address, zero.  I.e., all sals
end up with sal.pc == 0, so maybe_add_address returns false for all
but the first.

The zero addresses appear because:

- in the minsyms case, linespec.c:minsym_found incorrectly treats all
  minsyms as if they were function/text symbols.  In list mode we can
  end up with data symbols there, and we shouldn't be using
  find_pc_sect_line on data symbols.

- in the debug symbols case, symbol_to_sal misses recording an address
  (sal.pc) for non-block, non-label symbols.

gdb/ChangeLog:
2017-09-20  Pedro Alves  <palves@redhat.com>

	* linespec.c (minsym_found): Handle non-text minsyms.
	(symbol_to_sal): Record a sal.pc for non-block, non-label symbols.

gdb/testsuite/ChangeLog:
2017-09-20  Pedro Alves  <palves@redhat.com>

	* gdb.base/list-ambiguous.exp (test_list_ambiguous_function):
	Rename to ...
	(test_list_ambiguous_symbol): ... this and add a symbol name
	parameter.  Adjust.
	(test_list_ambiguous_function): Reimplement on top of
	test_list_ambiguous_symbol and also test listing ambiguous
	variables.
	* gdb.base/list-ambiguous0.c (ambiguous): Rename to ...
	(ambiguous_fun): ... this.
	(ambiguous_var): New.
	* gdb.base/list-ambiguous1.c (ambiguous): Rename to ...
	(ambiguous_fun): ... this.
	(ambiguous_var): New.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a9a6c58512a65969ec8af58dac1bd712c7ff86fb..1e0ae7db6a8c7ce43152733a613055680920434b 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2017-09-20  Pedro Alves  <palves@redhat.com>
+
+       * linespec.c (minsym_found): Handle non-text minsyms.
+       (symbol_to_sal): Record a sal.pc for non-block, non-label symbols.
+
 2017-09-20  Walfred Tedeschi  <walfred.tedeschi@intel.com>
 
        * features/Makefile (i386-avx-mpx-avx512-pku.dat): Add missing

diff --git a/gdb/linespec.c b/gdb/linespec.c
index b69ab629eec86f4507cafffbecea1478ff934ebd..35ef15983653017455ec7c24d0fa0c84cf354d10 100644 (file)
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -4313,35 +4313,46 @@ minsym_found (struct linespec_state *self, struct objfile *objfile,
   CORE_ADDR pc;
   struct symtab_and_line sal;
 
-  sal = find_pc_sect_line (MSYMBOL_VALUE_ADDRESS (objfile, msymbol),
-                          (struct obj_section *) 0, 0);
-  sal.section = MSYMBOL_OBJ_SECTION (objfile, msymbol);
-
-  /* The minimal symbol might point to a function descriptor;
-     resolve it to the actual code address instead.  */
-  pc = gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc, &current_target);
-  if (pc != sal.pc)
-    sal = find_pc_sect_line (pc, NULL, 0);
-
-  if (self->funfirstline)
+  if (msymbol_is_text (msymbol))
     {
-      if (sal.symtab != NULL
-         && (COMPUNIT_LOCATIONS_VALID (SYMTAB_COMPUNIT (sal.symtab))
-             || SYMTAB_LANGUAGE (sal.symtab) == language_asm))
+      sal = find_pc_sect_line (MSYMBOL_VALUE_ADDRESS (objfile, msymbol),
+                              (struct obj_section *) 0, 0);
+      sal.section = MSYMBOL_OBJ_SECTION (objfile, msymbol);
+
+      /* The minimal symbol might point to a function descriptor;
+        resolve it to the actual code address instead.  */
+      pc = gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc,
+                                              &current_target);
+      if (pc != sal.pc)
+       sal = find_pc_sect_line (pc, NULL, 0);
+
+      if (self->funfirstline)
        {
-         /* If gdbarch_convert_from_func_ptr_addr does not apply then
-            sal.SECTION, sal.LINE&co. will stay correct from above.
-            If gdbarch_convert_from_func_ptr_addr applies then
-            sal.SECTION is cleared from above and sal.LINE&co. will
-            stay correct from the last find_pc_sect_line above.  */
-         sal.pc = MSYMBOL_VALUE_ADDRESS (objfile, msymbol);
-         sal.pc = gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc,
-                                                      &current_target);
-         if (gdbarch_skip_entrypoint_p (gdbarch))
-           sal.pc = gdbarch_skip_entrypoint (gdbarch, sal.pc);
+         if (sal.symtab != NULL
+             && (COMPUNIT_LOCATIONS_VALID (SYMTAB_COMPUNIT (sal.symtab))
+                 || SYMTAB_LANGUAGE (sal.symtab) == language_asm))
+           {
+             /* If gdbarch_convert_from_func_ptr_addr does not apply then
+                sal.SECTION, sal.LINE&co. will stay correct from above.
+                If gdbarch_convert_from_func_ptr_addr applies then
+                sal.SECTION is cleared from above and sal.LINE&co. will
+                stay correct from the last find_pc_sect_line above.  */
+             sal.pc = MSYMBOL_VALUE_ADDRESS (objfile, msymbol);
+             sal.pc = gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc,
+                                                          &current_target);
+             if (gdbarch_skip_entrypoint_p (gdbarch))
+               sal.pc = gdbarch_skip_entrypoint (gdbarch, sal.pc);
+           }
+         else
+           skip_prologue_sal (&sal);
        }
-      else
-       skip_prologue_sal (&sal);
+    }
+  else
+    {
+      sal.objfile = objfile;
+      sal.pc = MSYMBOL_VALUE_ADDRESS (objfile, msymbol);
+      sal.pspace = current_program_space;
+      sal.section = MSYMBOL_OBJ_SECTION (objfile, msymbol);
     }
 
   if (maybe_add_address (self->addr_set, objfile->pspace, sal.pc))
@@ -4628,6 +4639,7 @@ symbol_to_sal (struct symtab_and_line *result,
          *result = {};
          result->symtab = symbol_symtab (sym);
          result->line = SYMBOL_LINE (sym);
+         result->pc = SYMBOL_VALUE_ADDRESS (sym);
          result->pspace = SYMTAB_PSPACE (result->symtab);
          return 1;
        }

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index d68072a258486e399eb74012180349705cd11155..7ab92a2c0c77286341c4ec92554fcc21403e7289 100644 (file)
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,19 @@
+2017-09-20  Pedro Alves  <palves@redhat.com>
+
+       * gdb.base/list-ambiguous.exp (test_list_ambiguous_function):
+       Rename to ...
+       (test_list_ambiguous_symbol): ... this and add a symbol name
+       parameter.  Adjust.
+       (test_list_ambiguous_function): Reimplement on top of
+       test_list_ambiguous_symbol and also test listing ambiguous
+       variables.
+       * gdb.base/list-ambiguous0.c (ambiguous): Rename to ...
+       (ambiguous_fun): ... this.
+       (ambiguous_var): New.
+       * gdb.base/list-ambiguous1.c (ambiguous): Rename to ...
+       (ambiguous_fun): ... this.
+       (ambiguous_var): New.
+
 2017-09-19  John Baldwin  <jhb@FreeBSD.org>
 
        * gdb.base/starti.c: New file.

diff --git a/gdb/testsuite/gdb.base/list-ambiguous.exp b/gdb/testsuite/gdb.base/list-ambiguous.exp
index db9d028453489a0824719a733de9ea12d66f0911..dd473caf62f985ae36d81291c7f465c90ab61173 100644 (file)
--- a/gdb/testsuite/gdb.base/list-ambiguous.exp
+++ b/gdb/testsuite/gdb.base/list-ambiguous.exp
@@ -39,36 +39,41 @@ proc line_range_pattern { range_start range_end } {
 # Test the "list" command with linespecs that expand to multiple
 # locations.
 
-proc test_list_ambiguous_function {} {
+proc test_list_ambiguous_symbol {symbol_line symbol} {
     global srcfile srcfile2
 
-    set lineno0 [gdb_get_line_number "ambiguous (void)" $srcfile]
-    set lineno1 [gdb_get_line_number "ambiguous (void)" $srcfile2]
+    set lineno0 [gdb_get_line_number $symbol_line $srcfile]
+    set lineno1 [gdb_get_line_number $symbol_line $srcfile2]
     set lines0_re [line_range_pattern [expr $lineno0 - 5] [expr $lineno0 + 4]]
     set lines1_re [line_range_pattern [expr $lineno1 - 5] [expr $lineno1 + 4]]
 
     set any "\[^\r\n\]*"
     set h0_re "file: \"${any}list-ambiguous0.c\", line number: $lineno0"
     set h1_re "file: \"${any}list-ambiguous1.c\", line number: $lineno1"
-    gdb_test "list ambiguous" "${h0_re}${lines0_re}\r\n${h1_re}${lines1_re}"
-
-    gdb_test "list main,ambiguous" \
-       "Specified last line 'ambiguous' is ambiguous:\r\n${h0_re}\r\n${h1_re}"
-    gdb_test "list ,ambiguous" \
-       "Specified last line 'ambiguous' is ambiguous:\r\n${h0_re}\r\n${h1_re}"
-    gdb_test "list ambiguous,main" \
-       "Specified first line 'ambiguous' is ambiguous:\r\n${h0_re}\r\n${h1_re}"
-    gdb_test "list ambiguous,ambiguous" \
-       "Specified first line 'ambiguous' is ambiguous:\r\n${h0_re}\r\n${h1_re}"
-    gdb_test "list ambiguous," \
-       "Specified first line 'ambiguous' is ambiguous:\r\n${h0_re}\r\n${h1_re}"
+    gdb_test "list $symbol" "${h0_re}${lines0_re}\r\n${h1_re}${lines1_re}"
+
+    gdb_test "list main,$symbol" \
+       "Specified last line '$symbol' is ambiguous:\r\n${h0_re}\r\n${h1_re}"
+    gdb_test "list ,$symbol" \
+       "Specified last line '$symbol' is ambiguous:\r\n${h0_re}\r\n${h1_re}"
+    gdb_test "list $symbol,main" \
+       "Specified first line '$symbol' is ambiguous:\r\n${h0_re}\r\n${h1_re}"
+    gdb_test "list $symbol,$symbol" \
+       "Specified first line '$symbol' is ambiguous:\r\n${h0_re}\r\n${h1_re}"
+    gdb_test "list $symbol," \
+       "Specified first line '$symbol' is ambiguous:\r\n${h0_re}\r\n${h1_re}"
 
     # While at it, test the "edit" command as well, since it shares
     # code with "list".
-    gdb_test "edit ambiguous" \
+    gdb_test "edit $symbol" \
        "Specified line is ambiguous:\r\n${h0_re}\r\n${h1_re}"
 }
 
+proc test_list_ambiguous_function {} {
+    test_list_ambiguous_symbol "ambiguous_fun (void)" "ambiguous_fun"
+    test_list_ambiguous_symbol "ambiguous_var" "ambiguous_var"
+}
+
 gdb_test_no_output "set listsize 10"
 
 test_list_ambiguous_function

diff --git a/gdb/testsuite/gdb.base/list-ambiguous0.c b/gdb/testsuite/gdb.base/list-ambiguous0.c
index 91f7fe968cdcb048dffd3e03dd8b4b4b309058c8..afd945745fd99ad79cd7d40fdf73c798f1e0cd7d 100644 (file)
--- a/gdb/testsuite/gdb.base/list-ambiguous0.c
+++ b/gdb/testsuite/gdb.base/list-ambiguous0.c
@@ -19,12 +19,13 @@
 
 
 
-/* This function is defined in both
+/* These symbols are defined in both
    list-ambiguous0.c/list-ambiguous1.c files, in order to test
    "list"'s behavior with ambiguous linespecs.  */
 
-static void __attribute__ ((used)) ambiguous (void) {}
+static void __attribute__ ((used)) ambiguous_fun (void) {}
 
+static int ambiguous_var;
 
 
 

diff --git a/gdb/testsuite/gdb.base/list-ambiguous1.c b/gdb/testsuite/gdb.base/list-ambiguous1.c
index 024299aa943a5a82bea4dc3baa86e956b3c1004e..69db11e045bd4c98647595a899ccb751baa89634 100644 (file)
--- a/gdb/testsuite/gdb.base/list-ambiguous1.c
+++ b/gdb/testsuite/gdb.base/list-ambiguous1.c
@@ -23,11 +23,12 @@
 
 
 
-/* This function is defined in both
+/* These symbols are defined in both
    list-ambiguous0.c/list-ambiguous1.c files, in order to test
    "list"'s behavior with ambiguous linespecs.  */
-static void __attribute__ ((used)) ambiguous (void) {}
+static void __attribute__ ((used)) ambiguous_fun (void) {}
 
+static int ambiguous_var;