From 4024cf2b8d864279ff87af1a2ade77ab6d710d50 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Wed, 29 Nov 2017 19:25:58 +0000 Subject: Fix setting-breakpoints regression on PPC64 (function descriptors) The recent-ish commit e5f25bc5d6db ('Fix "list ambiguous_variable"') caused a serious regression on PPC64. See . Basically, after that patch, GDB sets breakpoints in function descriptors instead of where the descriptors point to, which is incorrect. The problem is that GDB now only runs a minsym's address through gdbarch_convert_from_func_ptr_addr if msymbol_is_text returns true. However, if the symbol points to a function descriptor, msymbol_is_text is false since function descriptors are in fact outside the text section. The fix is to also run a non-text address through gdbarch_convert_from_func_ptr_addr, and if that detects that it was indeed a function descriptor, treat the resulting address as a function. While implementing that directly in linespec.c:minsym_found (where the bad msymbol_is_text check is) fixes the issue, I noticed that linespec.c:add_minsym has some code that also basically needs to do the same checks, however it's implemented differently. Also, add_minsym is calling find_pc_sect_line on non-function symbols, which also doesn't look right. So I introduced msymbol_is_function, so that we have a simple place to consider minsyms and function descriptors. And then, the only other use of msymbol_is_text is in find_function_alias_target, which turns out to also be incorrect. Changing that one to use msymbol_is_function, i.e., to consider function descriptors too fixes (on PPC64): -FAIL: gdb.base/symbol-alias.exp: p func_alias -FAIL: gdb.base/symbol-alias.exp: p *func_alias() +PASS: gdb.base/symbol-alias.exp: p func_alias +PASS: gdb.base/symbol-alias.exp: p *func_alias() And then after that, msymbol_is_text is no longer used anywhere, so it can be removed. Tested on x86_64 GNU/Linux, no regressions. Tested on PPC64 GNU/Linux and results compared to a testrun of e5f25bc5d6db^ (before the offending commit), also no regressions. (there's a couple new FAILs and some new symbol name matching unit tests are crashing, but that looks unrelated). gdb/ChangeLog: 2017-11-29 Pedro Alves * linespec.c (minsym_found, add_minsym): Use msymbol_is_function. * minsyms.c (msymbol_is_text): Delete. (msymbol_is_function): New function. * minsyms.h (msymbol_is_text): Delete. (msymbol_is_function): New declaration. * symtab.c (find_function_alias_target): Use msymbol_is_function. --- gdb/minsyms.h | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'gdb/minsyms.h') diff --git a/gdb/minsyms.h b/gdb/minsyms.h index 5c0dde4..baa87f0 100644 --- a/gdb/minsyms.h +++ b/gdb/minsyms.h @@ -159,9 +159,14 @@ void terminate_minimal_symbol_table (struct objfile *objfile); -/* Return whether MSYMBOL is a function/method. */ - -bool msymbol_is_text (minimal_symbol *msymbol); +/* Return whether MSYMBOL is a function/method. If FUNC_ADDRESS_P is + non-NULL, and the MSYMBOL is a function, then *FUNC_ADDRESS_P is + set to the function's address, already resolved if MINSYM points to + a function descriptor. */ + +bool msymbol_is_function (struct objfile *objfile, + minimal_symbol *minsym, + CORE_ADDR *func_address_p = NULL); /* Compute a hash code for the string argument. */ -- cgit v1.1