diff mbox

Include minimal symbol flavour name in LOC_UNRESOLVED error message.

Message ID m3bndpel90.fsf@sspiff.org
State New
Headers show

Commit Message

Doug Evans Aug. 29, 2015, 11:19 p.m. UTC
Hi.

The error message that is printed if a LOC_UNRESOLVED minsym isn't
found can be a bit confusing. The goal here is to clarify what the
term "symbol" means in the error message.

$ gdb testsuite/gdb.dwarf2/dw2-bad-unresolved
(gdb) start
(gdb) ptype var
type = bool
(gdb) p var
No global symbol "var".

E.g., it's odd to see gdb claiming there's no global symbol "var" when
it can plainly see its type.

The term "minsym" is an internal gdb term so I didn't want to expose that.
IOW, I discarded `No global minimal symbol "var".'

One thing I don't have a good answer for (beyond coming up with a
term for "minsym" that we can use in error messages), is that
one can imagine a program built from multiple file types
(maybe not today but at least in theory - e.g.,
in an embedded session I can imagine using add-symbol-file from
more than one file format). That could leave as being ambiguous
what flavour to use in the error message - using the flavour of
the objfile experiencing the error works for me.

So whereas today an ELF user would see `No global symbol "var".'
now they'll see `Missing ELF symbol "var".'

2015-08-29  Doug Evans  <xdje42@gmail.com>

	bfd/
	* targets.c (enum bfd_flavour): Add comment.
	(bfd_flavour_name): New function.
	* bfd-in2.h: Regenerate.

	gdb/
	* findvar.c (default_read_var_value) <LOC_UNRESOLVED>: Include the
	kind of minimal symbol in the error message.
	* objfiles.c (objfile_flavour_name): New function.
	* objfiles.h (objfile_flavour_name): Declare.

	gdb/testsuite/
	* gdb.dwarf2/dw2-bad-unresolved.c: New file.
	* gdb.dwarf2/dw2-bad-unresolved.exp: New file.

Comments

Doug Evans Aug. 29, 2015, 11:35 p.m. UTC | #1
Doug Evans <xdje42@gmail.com> writes:
> Hi.
>
> The error message that is printed if a LOC_UNRESOLVED minsym isn't
> found can be a bit confusing. The goal here is to clarify what the
> term "symbol" means in the error message.
>...
> 2015-08-29  Doug Evans  <xdje42@gmail.com>
>
> 	bfd/
> 	* targets.c (enum bfd_flavour): Add comment.
> 	(bfd_flavour_name): New function.
> 	* bfd-in2.h: Regenerate.
>...
> +const char *
> +bfd_flavour_name (bfd *abfd)
> +{
> +  switch (abfd->xvec->flavour)

Btw, a better way to do this would be to take the flavour enum
as the parameter instead of the bfd. Why does the caller
have to have a bfd in order to get the print form of the
enum? I'll do that assuming the patch is otherwise ok.

Callers with a bfd could then just do
bfd_flavour_name (bfd_get_flavour (abfd)).
Nick Clifton Sept. 3, 2015, 3:39 p.m. UTC | #2
Hi Doug,

> 2015-08-29  Doug Evans  <xdje42@gmail.com>
>
> 	bfd/
> 	* targets.c (enum bfd_flavour): Add comment.
> 	(bfd_flavour_name): New function.
> 	* bfd-in2.h: Regenerate.

This part of the patch is approved.

Cheers
   Nick
diff mbox

Patch

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 82bf269..9d3277e 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -6978,6 +6978,7 @@  bfd_boolean generic_core_file_matches_executable_p
 
 enum bfd_flavour
 {
+  /* N.B. Update bfd_flavour_name if you change this.  */
   bfd_target_unknown_flavour,
   bfd_target_aout_flavour,
   bfd_target_coff_flavour,
@@ -7397,6 +7398,8 @@  const bfd_target *bfd_search_for_target
    (int (*search_func) (const bfd_target *, void *),
     void *);
 
+const char *bfd_flavour_name (bfd *abfd);
+
 /* Extracted from format.c.  */
 bfd_boolean bfd_check_format (bfd *abfd, bfd_format format);
 
diff --git a/bfd/targets.c b/bfd/targets.c
index 1ada260..50ea6f8 100644
--- a/bfd/targets.c
+++ b/bfd/targets.c
@@ -144,6 +144,7 @@  DESCRIPTION
 
 .enum bfd_flavour
 .{
+.  {* N.B. Update bfd_flavour_name if you change this.  *}
 .  bfd_target_unknown_flavour,
 .  bfd_target_aout_flavour,
 .  bfd_target_coff_flavour,
@@ -1829,3 +1830,50 @@  bfd_search_for_target (int (*search_func) (const bfd_target *, void *),
 
   return NULL;
 }
+
+/*
+FUNCTION
+	bfd_flavour_name
+
+SYNOPSIS
+	const char *bfd_flavour_name (bfd *abfd);
+
+DESCRIPTION
+	Return the file format flavour of @var{abfd}.
+*/
+
+const char *
+bfd_flavour_name (bfd *abfd)
+{
+  switch (abfd->xvec->flavour)
+    {
+    case bfd_target_unknown_flavour: return "unknown file format";
+    case bfd_target_aout_flavour: return "a.out";
+    case bfd_target_coff_flavour: return "COFF";
+    case bfd_target_ecoff_flavour: return "ECOFF";
+    case bfd_target_xcoff_flavour: return "XCOFF";
+    case bfd_target_elf_flavour: return "ELF";
+    case bfd_target_ieee_flavour: return "IEEE";
+    case bfd_target_nlm_flavour: return "NLM";
+    case bfd_target_oasys_flavour: return "Oasys";
+    case bfd_target_tekhex_flavour: return "Tekhex";
+    case bfd_target_srec_flavour: return "Srec";
+    case bfd_target_verilog_flavour: return "Verilog";
+    case bfd_target_ihex_flavour: return "Ihex";
+    case bfd_target_som_flavour: return "SOM";
+    case bfd_target_os9k_flavour: return "OS9K";
+    case bfd_target_versados_flavour: return "Versados";
+    case bfd_target_msdos_flavour: return "MSDOS";
+    case bfd_target_ovax_flavour: return "Ovax";
+    case bfd_target_evax_flavour: return "Evax";
+    case bfd_target_mmo_flavour: return "MMO";
+    case bfd_target_mach_o_flavour: return "MACH_O";
+    case bfd_target_pef_flavour: return "PEF";
+    case bfd_target_pef_xlib_flavour: return "PEF_XLIB";
+    case bfd_target_sym_flavour: return "SYM";
+    /* There is no "default" case here so that -Wswitch (part of -Wall)
+       catches missing entries.  */
+    }
+
+  abort ();
+}
diff --git a/gdb/findvar.c b/gdb/findvar.c
index 1c077f7..022d78d 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -737,8 +737,20 @@  default_read_var_value (struct symbol *var, const struct block *var_block,
 	   symbol_objfile (var));
 	msym = lookup_data.result.minsym;
 
+	/* If we can't find the minsym there's a problem in the symbol info.
+	   The symbol exists in the debug info, but it's missing in the minsym
+	   table.  */
 	if (msym == NULL)
-	  error (_("No global symbol \"%s\"."), SYMBOL_LINKAGE_NAME (var));
+	  {
+	    const char *flavour_name
+	      = objfile_flavour_name (symbol_objfile (var));
+
+	    /* We can't get here unless we've opened the file, so flavour_name
+	       can't be NULL.  */
+	    gdb_assert (flavour_name != NULL);
+	    error (_("Missing %s symbol \"%s\"."),
+		   flavour_name, SYMBOL_LINKAGE_NAME (var));
+	  }
 	if (overlay_debugging)
 	  addr = symbol_overlayed_address (BMSYMBOL_VALUE_ADDRESS (lookup_data.result),
 					   MSYMBOL_OBJ_SECTION (lookup_data.result.objfile,
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 907126c..fa2c8fb 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -1614,6 +1614,16 @@  objfile_debug_name (const struct objfile *objfile)
   return lbasename (objfile->original_name);
 }
 
+/* See objfiles.h.  */
+
+const char *
+objfile_flavour_name (struct objfile *objfile)
+{
+  if (objfile->obfd != NULL)
+    return bfd_flavour_name (objfile->obfd);
+  return NULL;
+}
+
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 extern initialize_file_ftype _initialize_objfiles;
 
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index be2a5ef..da5ac1a 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -744,6 +744,11 @@  const char *objfile_filename (const struct objfile *objfile);
 
 extern const char *objfile_debug_name (const struct objfile *objfile);
 
+/* Return the name of the file format of OBJFILE if the file has been opened,
+   otherwise return NULL.  */
+
+const char *objfile_flavour_name (struct objfile *objfile);
+
 /* Set the objfile's notion of the "main" name and language.  */
 
 extern void set_objfile_main_name (struct objfile *objfile,
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-bad-unresolved.c b/gdb/testsuite/gdb.dwarf2/dw2-bad-unresolved.c
new file mode 100644
index 0000000..1c35510
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-bad-unresolved.c
@@ -0,0 +1,24 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 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/>.  */
+
+/* Dummy main function.  */
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-bad-unresolved.exp b/gdb/testsuite/gdb.dwarf2/dw2-bad-unresolved.exp
new file mode 100644
index 0000000..33d7a77
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-bad-unresolved.exp
@@ -0,0 +1,58 @@ 
+# Copyright 2015 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/>.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0  
+}
+
+standard_testfile dw2-bad-unresolved.c dw2-bad-unresolved-2.S
+
+# Set up the DWARF for the test.
+# The key part of this is the definition of variable "var" having no location.
+# That will cause it to have LOC_UNRESOLVED.
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    cu {} {
+	DW_TAG_compile_unit {
+	    {DW_AT_language @DW_LANG_C}
+	    {DW_AT_name     dw2-bad-unresolved-2.c}
+	    {DW_AT_comp_dir /tmp}
+	} {
+	    declare_labels b_l
+
+	    b_l: DW_TAG_base_type {
+		{DW_AT_byte_size 1 DW_FORM_sdata}
+		{DW_AT_encoding  @DW_ATE_signed}
+		{DW_AT_name      bool}
+	    }
+	    DW_TAG_variable {
+		{name var}
+		{type :$b_l}
+		{external 1 flag}
+	    }
+	}
+    }
+}
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+gdb_test "print var" "Missing .* symbol \"var\"."