This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: The future of dwarf2_physname
Jan> I have concerns about regressions for gdb-7.3, I would like to fix
Jan> regressions gdb-7.1 (pre-physname) -> gdb-7.2 (physname) but
Jan> possibly not introduce regressions gdb-7.2 (physname) -> gdb-7.3
Jan> (?).
I don't see how that is really possible.
I am willing to accept some minor regressions for 7.3 in exchange for
doing better in the long run.
Jan> I was trying to propose DMGL_RET_POSTFIX but it has other drawbacks
Jan> and it is in fact a new feature (and not a regression fixup)
Jan> because the trailing type was not present in gdb-7.2 anyway.
I think we should not take this approach, for the simple reason that it
will be weird to C++ users. I think it would be better to just require
quotes in this case for 7.3, like "break 'int f<int> ()'", then see
about fixing up linespec, or whatever, in 7.4+. Also for later would be
making some kind of symbol alias for this, so that in the normal case
users can omit the return type.
Jan> (gdb) complete p 'f(
Jan> p 'f(int)
Jan> p 'f(t)
Jan> - I do not find it clear this is right. There should be
Jan> definitely the `f(int)' - demangled linkage name - variant. But
Jan> whether to show the "convenient" aliases as SYMBOL_SEARCH_NAME I
Jan> do not find clear, it may be better but it also complicates the
Jan> list of all the available overloads for a function name, one
Jan> should ask practical C++ programmer for it IMO.
I think this one is ok, but I can see how that could go either way.
I find it a side issue at present.
Jan> Regarding the proposals to fix dwarf2_physname computation instead
Jan> of preferring DW_AT_linkage_name. ISTM we have never found the
Jan> agreement with Keith, when DW_AT_linkage_name is present it is
Jan> guaranteed to be correct and I do not know why not to use it.
I think we should move forward with it, on the basis that it is more
correct and probably has better performance to boot.
I suggest something like the appended, on top of Keith's patches and
your patch. The idea here is to prefer the fast path, but have a debug
setting so we can easily do physname checking.
WDYT? I didn't run it through the test suite or anything yet.
Jan> Keith proposes to fix dwarf2_physname instead of using
Jan> DW_AT_linkage_name.
We must do both in the medium term.
Tom
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 7530e3e..fff8e3b 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -119,6 +119,9 @@ _STATEMENT_PROLOGUE;
/* When non-zero, dump DIEs after they are read in. */
static int dwarf2_die_debug = 0;
+/* When non-zero, cross-check physname against demangler. */
+static int check_physname = 0;
+
static int pagesize;
/* When set, the file that we're processing is known to have debugging
@@ -5142,71 +5145,71 @@ dwarf2_full_name (char *name, struct die_info *die, struct dwarf2_cu *cu)
static const char *
dwarf2_physname (char *name, struct die_info *die, struct dwarf2_cu *cu)
{
- const char *physname = dwarf2_compute_name (name, die, cu, NAME_KIND_PHYS);
+ const char *physname;
struct attribute *attr;
- const char *mangled, *retval;
- char *demangled, *canon;
- struct cleanup *back_to;
-
- /* Do not check PHYSNAME if it never will be needed by GDB. GDB does not
- need to support something it has no use for. */
- if (!die_needs_namespace (die, cu))
- return physname;
+ const char *retval, *mangled = NULL;
+ char *canon = NULL;
+ struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
+ int need_copy = 1;
attr = dwarf2_attr (die, DW_AT_linkage_name, cu);
if (!attr)
attr = dwarf2_attr (die, DW_AT_MIPS_linkage_name, cu);
- if (!attr || !DW_STRING (attr))
+ /* DW_AT_linkage_name is missing in some cases - depend on what GDB
+ has computed. */
+ if (attr && DW_STRING (attr))
{
- /* DW_AT_linkage_name is missing in some cases - depend on what GDB has
- computed. */
-
- return physname;
- }
- mangled = DW_STRING (attr);
-
- /* As both DW_AT_linkage_name (MANGLED) and computed PHYSNAME are present
- cross-check them. */
-
- back_to = make_cleanup (null_cleanup, 0);
+ char *demangled;
- demangled = cplus_demangle (mangled, DMGL_PARAMS | DMGL_ANSI);
- if (demangled)
- make_cleanup (xfree, demangled);
- else
- demangled = (char *) mangled;
+ mangled = DW_STRING (attr);
- if (cu->language == language_cplus)
- {
- canon = cp_canonicalize_string (demangled);
- if (canon != NULL)
- make_cleanup (xfree, canon);
+ demangled = cplus_demangle (mangled, (DMGL_PARAMS | DMGL_ANSI
+ | DMGL_VERBOSE));
+ if (demangled)
+ {
+ make_cleanup (xfree, demangled);
+ canon = demangled;
+ }
else
- canon = demangled;
+ {
+ canon = (char *) mangled;
+ need_copy = 0;
+ }
}
- else
- canon = demangled;
- if (strcmp (physname, canon) != 0)
+ if (canon == NULL || check_physname)
{
- /* It may not mean a bug in GDB. The compiler could also compute
- DW_AT_linkage_name incorrectly. But in such case GDB would need to be
- bug-to-bug compatible. */
+ physname = dwarf2_compute_name (name, die, cu, NAME_KIND_PHYS);
- complaint (&symfile_complaints,
- _("Computed physname <%s> does not match demangled <%s> "
- "(from linkage <%s>) - DIE at 0x%x [in module %s]"),
- physname, canon, mangled, die->offset, cu->objfile->name);
+ if (canon != NULL && strcmp (physname, canon) != 0)
+ {
+ /* It may not mean a bug in GDB. The compiler could also
+ compute DW_AT_linkage_name incorrectly. But in such case
+ GDB would need to be bug-to-bug compatible. */
+
+ complaint (&symfile_complaints,
+ _("Computed physname <%s> does not match demangled <%s> "
+ "(from linkage <%s>) - DIE at 0x%x [in module %s]"),
+ physname, canon, mangled, die->offset, cu->objfile->name);
- /* Prefer DW_AT_linkage_name (in the CANON form) - when it is available
- here - over computed PHYSNAME. It is safer against both buggy GDB and
- buggy compilers. */
+ /* Prefer DW_AT_linkage_name (in the CANON form) - when it
+ is available here - over computed PHYSNAME. It is safer
+ against both buggy GDB and buggy compilers. */
- retval = obsavestring (canon, strlen (canon),
- &cu->objfile->objfile_obstack);
+ retval = canon;
+ }
+ else
+ {
+ retval = physname;
+ need_copy = 0;
+ }
}
else
- retval = physname;
+ retval = canon;
+
+ if (need_copy)
+ retval = obsavestring (retval, strlen (retval),
+ &cu->objfile->objfile_obstack);
do_cleanups (back_to);
return retval;
@@ -16247,6 +16250,15 @@ show_dwarf2_always_disassemble (struct ui_file *file, int from_tty,
value);
}
+static void
+show_check_physname (struct ui_file *file, int from_tty,
+ struct cmd_list_element *c, const char *value)
+{
+ fprintf_filtered (file,
+ _("Whether to check \"physname\" is %s.\n"),
+ value);
+}
+
void _initialize_dwarf2_read (void);
void
@@ -16302,6 +16314,14 @@ The value is the maximum depth to print."),
NULL,
&setdebuglist, &showdebuglist);
+ add_setshow_boolean_cmd ("check-physname", no_class, &check_physname, _("\
+Set cross-checking of \"physname\" code against demangler."), _("\
+Show cross-checking of \"physname\" code against demangler."), _("\
+When enabled, GDB's internal \"physname\" code is checked against\n\
+the demangler."),
+ NULL, show_check_physname,
+ &setdebuglist, &showdebuglist);
+
c = add_cmd ("gdb-index", class_files, save_gdb_index_command,
_("\
Save a gdb-index file.\n\