This is the mail archive of the
mailing list for the GDB project.
Re: [PATCH 1/4] Prefer symtab symbol over minsym for function names in non-contiguous blocks
On 6/8/19 8:54 PM, Kevin Buettner wrote:
> The discussion on gdb-patches which led to this patch may be found
> Here's a brief synopsis/analysis:
> Eli Zaretskii, while debugging a Windows emacs executable, found
> that functions comprised of more than one (non-contiguous)
> address range were not being displayed correctly in a backtrace. This
> is the example that Eli provided:
> (gdb) bt
> #0 0x76a63227 in KERNELBASE!DebugBreak ()
> from C:\Windows\syswow64\KernelBase.dll
> #1 0x012e7b89 in emacs_abort () at w32fns.c:10768
> #2 0x012e1f3b in print_vectorlike.cold () at print.c:1824
> #3 0x011d2dec in print_object (obj=<optimized out>, printcharfun=XIL(0),
> escapeflag=true) at print.c:2150
> The function print_vectorlike consists of two address ranges, one of
> which contains "cold" code which is expected to not execute very often.
> There is a minimal symbol, print_vectorlike.cold.65, which is the address
> of the "cold" range.
> GDB is prefering this minsym over the the name provided by the
> DWARF info due to some really old code in GDB which handles
> "certain pathological cases". See the first big block comment
> in find_frame_funname for more information.
> I considered removing the code for this corner case entirely, but it
> seems as though it might still be useful, so I left it intact.
Yeah, I'd be inclined to try removing it too. The comment
smells of a.out or stabs limitations. But I'm not 100% sure,
and I'm sympathetic with forward incremental progress.
> That code is already disabled for inline functions. I added a
> condition which disables it for non-contiguous functions as well.
> The change to find_frame_funname in stack.c fixes this problem for
> stack traces. A similar change to print_address_symbolic in
> printcmd.c fixes this problem for the "x" command and other commands
> which use print_address_symbolic().
> * stack.c (find_frame_funname): Disable use of minsym for function
> name in functions comprised of non-contiguous blocks.
> * printcmd.c (print_address_symbolic): Likewise.
> gdb/printcmd.c | 4 +++-
> gdb/stack.c | 15 ++++++++++++---
> 2 files changed, 15 insertions(+), 4 deletions(-)
> diff --git a/gdb/printcmd.c b/gdb/printcmd.c
> index 9e84594fe6..e00a9c671a 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -627,7 +627,9 @@ build_address_symbolic (struct gdbarch *gdbarch,
> if (msymbol.minsym != NULL)
> - if (BMSYMBOL_VALUE_ADDRESS (msymbol) > name_location || symbol == NULL)
> + if (symbol == NULL
> + || (BLOCK_CONTIGUOUS_P (SYMBOL_BLOCK_VALUE (symbol))
> + && BMSYMBOL_VALUE_ADDRESS (msymbol) > name_location))
I think this warrants a comment somewhere. Maybe a bit higher up,
where it reads:
/* First try to find the address in the symbol table, then
in the minsyms. Take the closest one. */
Or better yet, abstract out the relevant bits in
build_address_symbolic and find_frame_funname to a separate
function, and then use that function in both places?
IIUC, in both cases, we're looking to see if there's a minimal
symbol that would be better than the debug symbol we start with.
> @@ -1067,9 +1067,18 @@ find_frame_funname (struct frame_info *frame, enum language *funlang,
> struct bound_minimal_symbol msymbol;
> - /* Don't attempt to do this for inlined functions, which do not
> - have a corresponding minimal symbol. */
> - if (!block_inlined_p (SYMBOL_BLOCK_VALUE (func)))
> + /* Don't attempt to do this for two cases:
> + 1) Inlined functions, which do not have a corresponding minimal
> + symbol.
> + 2) Functions which are comprised of non-contiguous blocks.
> + Such functions often contain a minimal symbol for a
> + "cold" range, i.e. code which is not expected to execute
> + very often. It is incorrect to use the minimal symbol
> + associated with this range. */
I don't find this "incorrect" here very useful -- why is it incorrect
is my immediate question when I read this.
Maybe that old comment:
So look in the minimal symbol tables as well, and if it comes
up with a larger address for the function use that instead.
I don't think this can ever cause any problems; there
shouldn't be any minimal symbols in the middle of a function;
should be updated.