Re: [rfc] physname cross-check #2

Jan Kratochvil
2011-05-19T23:04:30Z

On Tue, 17 May 2011 20:15:18 +0200, Keith Seitz wrote:
> For me, what really matters is what is best for users. Is reverting
> dwarf2_physname better or worse than DW_AT_MIPS_linkage_name?

This patch has no regressions on {x86_64,x86_64-m32,i686}-fedora15-linux-gnu.
And it prefers DW_AT_linkage_name when available to solve the DWARF<->ELF
symbols discrepancies.

DMGL_VERBOSE was suggested by Keith.

The libiberty/ part needs more review and a post to GCC but it seems as
a correct fix idea anyway.

Thanks,
Jan

--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -5142,7 +5142,77 @@ 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)
{
- return dwarf2_compute_name (name, die, cu, NAME_KIND_PHYS);
+ const char *physname = dwarf2_compute_name (name, die, cu, NAME_KIND_PHYS);
+ 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;
+
+ 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. */
+
+ 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);
+
+ demangled = cplus_demangle (mangled,
+ (DMGL_PARAMS | DMGL_VERBOSE | DMGL_RET_POSTFIX
+ | DMGL_ANSI | (cu->language == language_java
+ ? DMGL_JAVA : 0)));
+ if (demangled)
+ make_cleanup (xfree, demangled);
+ else
+ demangled = (char *) mangled;
+
+ if (cu->language == language_cplus)
+ {
+ canon = cp_canonicalize_string (demangled);
+ if (canon != NULL)
+ make_cleanup (xfree, canon);
+ else
+ canon = demangled;
+ }
+ else
+ canon = demangled;
+
+ if (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]\n"),
+ 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. */
+
+ retval = obsavestring (canon, strlen (canon),
+ &cu->objfile->objfile_obstack);
+ }
+ else
+ retval = physname;
+
+ do_cleanups (back_to);
+ return retval;
}

static const char *
--- a/gdb/testsuite/gdb.base/break-interp.exp
+++ b/gdb/testsuite/gdb.base/break-interp.exp
@@ -140,14 +140,14 @@ proc reach_1 {func command displacement} {
}
exp_continue
}
- -re "Breakpoint \[0-9\]+, \\.?$func \\(.*\\) at .*:\[0-9\]+\r\n.*$gdb_prompt $" {
+ -re "Breakpoint \[0-9\]+, \\.?(__GI_)?$func \\(.*\\) at .*:\[0-9\]+\r\n.*$gdb_prompt $" {
if {$func == "_dl_debug_state"} {
fail $test
} else {
pass $test
}
}
- -re "Breakpoint \[0-9\]+, \[0-9xa-f\]+ in \\.?$func \\(\\).*\r\n$gdb_prompt $" {
+ -re "Breakpoint \[0-9\]+, \[0-9xa-f\]+ in \\.?(__GI_)?$func \\(\\).*\r\n$gdb_prompt $" {
if {$func == "_dl_debug_state"} {
fail $test
} else {
@@ -403,7 +403,7 @@ proc test_ld {file ifmain trynosym displacement} {

reach "_dl_debug_state" "run" $displacement

- gdb_test "bt" "#0 +\[^\r\n\]*\\m_dl_debug_state\\M.*" "dl bt"
+ gdb_test "bt" "#0 +\[^\r\n\]*\\m(__GI_)?_dl_debug_state\\M.*" "dl bt"

if $ifmain {
reach "main" continue "NONE"
--- a/include/demangle.h
+++ b/include/demangle.h
@@ -45,7 +45,8 @@ extern "C" {
#define DMGL_VERBOSE (1 << 3) /* Include implementation details. */
#define DMGL_TYPES (1 << 4) /* Also try to demangle type encodings. */
#define DMGL_RET_POSTFIX (1 << 5) /* Print function return types (when
- present) after function signature */
+ present) after function signature.
+ It applies only for toplevel types. */

#define DMGL_AUTO (1 << 8)
#define DMGL_GNU (1 << 9)
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -3557,6 +3557,8 @@ static void
d_print_comp (struct d_print_info *dpi,
const struct demangle_component *dc)
{
+ int ret_postfix;
+
if (dc == NULL)
{
d_print_error (dpi);
@@ -3565,6 +3567,9 @@ d_print_comp (struct d_print_info *dpi,
if (d_print_saw_error (dpi))
return;

+ ret_postfix = dpi->options & DMGL_RET_POSTFIX;
+ dpi->options &= ~DMGL_RET_POSTFIX;
+
switch (dc->type)
{
case DEMANGLE_COMPONENT_NAME:
@@ -3576,6 +3581,7 @@ d_print_comp (struct d_print_info *dpi,

case DEMANGLE_COMPONENT_QUAL_NAME:
case DEMANGLE_COMPONENT_LOCAL_NAME:
+ dpi->options |= ret_postfix;
d_print_comp (dpi, d_left (dc));
if ((dpi->options & DMGL_JAVA) == 0)
d_append_string (dpi, "::");
@@ -3597,6 +3603,7 @@ d_print_comp (struct d_print_info *dpi,
any CV-qualifiers, which apply to the this parameter. */
hold_modifiers = dpi->modifiers;
dpi->modifiers = 0;
+ dpi->options |= ret_postfix;
i = 0;
typed_name = d_left (dc);
while (typed_name != NULL)
@@ -3919,7 +3926,7 @@ d_print_comp (struct d_print_info *dpi,

case DEMANGLE_COMPONENT_FUNCTION_TYPE:
{
- if ((dpi->options & DMGL_RET_POSTFIX) != 0)
+ if (ret_postfix)
d_print_function_type (dpi, dc, dpi->modifiers);

/* Print return type if present */
@@ -3944,11 +3951,11 @@ d_print_comp (struct d_print_info *dpi,

/* In standard prefix notation, there is a space between the
return type and the function signature. */
- if ((dpi->options & DMGL_RET_POSTFIX) == 0)
+ if (!ret_postfix)
d_append_char (dpi, ' ');
}

- if ((dpi->options & DMGL_RET_POSTFIX) == 0)
+ if (!ret_postfix)
d_print_function_type (dpi, dc, dpi->modifiers);

return;