From 2dc80cf8a5799120fd4e00199688f721e7de2a62 Mon Sep 17 00:00:00 2001 From: Kevin Buettner Date: Wed, 3 Jul 2019 17:35:21 -0700 Subject: [PATCH] Restrict use of minsym names when printing addresses in disassembled code build_address_symbolic contains some code which causes it to prefer the minsym over the the function symbol in certain cases. The cases where this occurs are the same as the "certain pathological cases" that used to exist in find_frame_funname(). This commit largely disables that code; it will only prefer the minsym when the address of minsym is identical to that of the address under consideration AND the function address for the symbtab sym is not the same as the address under consideration. So, without this change, when using the dw2-ranges-func-lo-cold executable from the gdb.dwarf2/dw2-ranges-func.exp test, GDB exhibits the following behavior: (gdb) x/5i foo_cold 0x40110d : push %rbp 0x40110e : mov %rsp,%rbp 0x401111 : callq 0x401106 0x401116 : nop 0x401117 : pop %rbp On the other hand, still without this change, using the dw2-ranges-func-hi-cold executable from the same test, GDB does this instead: (gdb) x/5i foo_cold 0x401128 : push %rbp 0x401129 : mov %rsp,%rbp 0x40112c : callq 0x401134 0x401131 : nop 0x401132 : pop %rbp This is inconsistent behavior. When foo_cold is at a lower address than the function's entry point, the symtab symbol (foo) is displayed along with a large positive offset which would wrap around the address space if the address space were only 32 bits wide. (A later patch fixes this problem by displaying negative offsets.) This commit makes the behavior uniform for both the "lo-cold" and "hi-cold" cases: lo-cold: (gdb) x/5i foo_cold 0x40110d : push %rbp 0x40110e : mov %rsp,%rbp 0x401111 : callq 0x401106 0x401116 : nop 0x401117 : pop %rbp hi-cold: (gdb) x/5i foo_cold 0x401128 : push %rbp 0x401129 : mov %rsp,%rbp 0x40112c : callq 0x401134 0x401131 : nop 0x401132 : pop %rbp In both cases, the symbol shown for the address at which foo_cold resides is shown as . Subsequent offsets are shown as either negative or positive offsets from the entry pc for foo. When disassembling a function, care must be taken to NOT display <+0> as the offset for the second range. For this reason, I found it necessary to add the "prefer_sym_over_minsym" parameter to build_address_symbolic. The type of this flag is a bool; do_demangle ought to be a bool also, so I made this change at the same time. gdb/ChangeLog: * valprint.h (build_address_symbolic): Add "prefer_sym_over_minsym" parameter. Change type of "do_demangle" to bool. * disasm.c (gdb_pretty_print_disassembler::pretty_print_insn): Pass suitable "prefer_sym_over_minsym" flag to build_address_symbolic(). Don't output "+" for negative offsets. * printcmd.c (print_address_symbolic): Update invocation of build_address_symbolic to include a "prefer_sym_over_minsym" flag. (build_address_symbolic): Add "prefer_sym_over_minsym" parameter. Restrict cases in which use of minimal symbol is preferred to that of a found symbol. Update comments. --- gdb/ChangeLog | 12 ++++++++++++ gdb/disasm.c | 12 ++++++++---- gdb/printcmd.c | 29 +++++++++++++++++++++-------- gdb/valprint.h | 13 +++++++++---- 4 files changed, 50 insertions(+), 16 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 7d6ef1aae2..82ef5038f1 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -3,6 +3,18 @@ * stack.c (find_frame_funname): Remove code which preferred minsym over symtab sym in "certain pathological cases". + * valprint.h (build_address_symbolic): Add "prefer_sym_over_minsym" + parameter. Change type of "do_demangle" to bool. + * disasm.c (gdb_pretty_print_disassembler::pretty_print_insn): + Pass suitable "prefer_sym_over_minsym" flag to + build_address_symbolic(). Don't output "+" for negative offsets. + * printcmd.c (print_address_symbolic): Update invocation of + build_address_symbolic to include a "prefer_sym_over_minsym" + flag. + (build_address_symbolic): Add "prefer_sym_over_minsym" parameter. + Restrict cases in which use of minimal symbol is preferred to that + of a found symbol. Update comments. + 2019-07-26 Brian Callahan PR gdb/24839: diff --git a/gdb/disasm.c b/gdb/disasm.c index e2b4fd6391..0d4c9733a0 100644 --- a/gdb/disasm.c +++ b/gdb/disasm.c @@ -237,16 +237,20 @@ gdb_pretty_print_disassembler::pretty_print_insn (struct ui_out *uiout, uiout->field_core_addr ("address", gdbarch, pc); std::string name, filename; - if (!build_address_symbolic (gdbarch, pc, 0, &name, &offset, &filename, - &line, &unmapped)) + bool omit_fname = ((flags & DISASSEMBLY_OMIT_FNAME) != 0); + if (!build_address_symbolic (gdbarch, pc, false, omit_fname, &name, + &offset, &filename, &line, &unmapped)) { /* We don't care now about line, filename and unmapped. But we might in the future. */ uiout->text (" <"); - if ((flags & DISASSEMBLY_OMIT_FNAME) == 0) + if (!omit_fname) uiout->field_string ("func-name", name.c_str (), ui_out_style_kind::FUNCTION); - uiout->text ("+"); + /* For negative offsets, avoid displaying them as +-N; the sign of + the offset takes the place of the "+" here. */ + if (offset >= 0) + uiout->text ("+"); uiout->field_signed ("offset", offset); uiout->text (">:\t"); } diff --git a/gdb/printcmd.c b/gdb/printcmd.c index 2081704bc4..efe6874d39 100644 --- a/gdb/printcmd.c +++ b/gdb/printcmd.c @@ -529,8 +529,8 @@ print_address_symbolic (struct gdbarch *gdbarch, CORE_ADDR addr, int offset = 0; int line = 0; - if (build_address_symbolic (gdbarch, addr, do_demangle, &name, &offset, - &filename, &line, &unmapped)) + if (build_address_symbolic (gdbarch, addr, do_demangle, false, &name, + &offset, &filename, &line, &unmapped)) return 0; fputs_filtered (leadin, stream); @@ -564,7 +564,8 @@ print_address_symbolic (struct gdbarch *gdbarch, CORE_ADDR addr, int build_address_symbolic (struct gdbarch *gdbarch, CORE_ADDR addr, /* IN */ - int do_demangle, /* IN */ + bool do_demangle, /* IN */ + bool prefer_sym_over_minsym, /* IN */ std::string *name, /* OUT */ int *offset, /* OUT */ std::string *filename, /* OUT */ @@ -592,8 +593,10 @@ build_address_symbolic (struct gdbarch *gdbarch, } } - /* First try to find the address in the symbol table, then - in the minsyms. Take the closest one. */ + /* Try to find the address in both the symbol table and the minsyms. + In most cases, we'll prefer to use the symbol instead of the + minsym. However, there are cases (see below) where we'll choose + to use the minsym instead. */ /* This is defective in the sense that it only finds text symbols. So really this is kind of pointless--we should make sure that the @@ -630,7 +633,19 @@ build_address_symbolic (struct gdbarch *gdbarch, if (msymbol.minsym != NULL) { - if (BMSYMBOL_VALUE_ADDRESS (msymbol) > name_location || symbol == NULL) + /* Use the minsym if no symbol is found. + + Additionally, use the minsym instead of a (found) symbol if + the following conditions all hold: + 1) The prefer_sym_over_minsym flag is false. + 2) The minsym address is identical to that of the address under + consideration. + 3) The symbol address is not identical to that of the address + under consideration. */ + if (symbol == NULL || + (!prefer_sym_over_minsym + && BMSYMBOL_VALUE_ADDRESS (msymbol) == addr + && name_location != addr)) { /* If this is a function (i.e. a code address), strip out any non-address bits. For instance, display a pointer to the @@ -643,8 +658,6 @@ build_address_symbolic (struct gdbarch *gdbarch, || MSYMBOL_TYPE (msymbol.minsym) == mst_solib_trampoline) addr = gdbarch_addr_bits_remove (gdbarch, addr); - /* The msymbol is closer to the address than the symbol; - use the msymbol instead. */ symbol = 0; name_location = BMSYMBOL_VALUE_ADDRESS (msymbol); if (do_demangle || asm_demangle) diff --git a/gdb/valprint.h b/gdb/valprint.h index 987c534eaf..07014c11b9 100644 --- a/gdb/valprint.h +++ b/gdb/valprint.h @@ -255,13 +255,18 @@ extern void print_command_completer (struct cmd_list_element *ignore, /* Given an address ADDR return all the elements needed to print the address in a symbolic form. NAME can be mangled or not depending on DO_DEMANGLE (and also on the asm_demangle global variable, - manipulated via ''set print asm-demangle''). Return 0 in case of - success, when all the info in the OUT paramters is valid. Return 1 - otherwise. */ + manipulated via ''set print asm-demangle''). When + PREFER_SYM_OVER_MINSYM is true, names (and offsets) from minimal + symbols won't be used except in instances where no symbol was + found; otherwise, a minsym might be used in some instances (mostly + involving function with non-contiguous address ranges). Return + 0 in case of success, when all the info in the OUT paramters is + valid. Return 1 otherwise. */ extern int build_address_symbolic (struct gdbarch *, CORE_ADDR addr, - int do_demangle, + bool do_demangle, + bool prefer_sym_over_minsym, std::string *name, int *offset, std::string *filename, -- 2.34.1