| Message ID | 51AF1444.5080800@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 06/05/2013 11:34 AM, Will Newton wrote: > > In build_address_symbolic we call gdbarch_addr_bits_remove for > symbols in the symbol table but not for minimal symbols. This > causes a failure in gdb.cp/virtfunc.exp on ARM, as the address > of the virtual thunk is given an offset of 1 when in Thumb mode. > > gdb/ChangeLog: > > 2013-06-05 Will Newton <will.newton@linaro.org> > > * printcmd.c (build_address_symbolic): Call > gdbarch_addr_bits_remove for text minimal symbols. > --- > gdb/printcmd.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/gdb/printcmd.c b/gdb/printcmd.c > index 7beb334..619e684 100644 > --- a/gdb/printcmd.c > +++ b/gdb/printcmd.c > @@ -689,6 +689,16 @@ build_address_symbolic (struct gdbarch *gdbarch, > { > if (SYMBOL_VALUE_ADDRESS (msymbol) > name_location || symbol == NULL) > { > + /* If this is a function (i.e. a code address), strip out any > + non-address bits. For instance, display a pointer to the > + first instruction of a Thumb function as <function>; the > + second instruction will be <function+2>, even though the > + pointer is <function+3>. This matches the ISA behavior. */ > + if (MSYMBOL_TYPE (msymbol) == mst_text > + || MSYMBOL_TYPE (msymbol) == mst_text_gnu_ifunc > + || MSYMBOL_TYPE (msymbol) == mst_file_text) > + addr = gdbarch_addr_bits_remove (gdbarch, addr); Shouldn't we do this for all text symbols, and thus for mst_solib_trampoline too? Not sure whether the if above that seems to be checking for text symbols is missing it too: if (msymbol != NULL && MSYMBOL_HAS_SIZE (msymbol) && MSYMBOL_SIZE (msymbol) == 0 && MSYMBOL_TYPE (msymbol) != mst_text && MSYMBOL_TYPE (msymbol) != mst_text_gnu_ifunc && MSYMBOL_TYPE (msymbol) != mst_file_text) msymbol = NULL; (I wish this bit had a comment mentioning what's it about.) > + > /* The msymbol is closer to the address than the symbol; > use the msymbol instead. */ > symbol = 0;
On 5 June 2013 16:21, Pedro Alves <palves@redhat.com> wrote: > On 06/05/2013 11:34 AM, Will Newton wrote: >> >> In build_address_symbolic we call gdbarch_addr_bits_remove for >> symbols in the symbol table but not for minimal symbols. This >> causes a failure in gdb.cp/virtfunc.exp on ARM, as the address >> of the virtual thunk is given an offset of 1 when in Thumb mode. >> >> gdb/ChangeLog: >> >> 2013-06-05 Will Newton <will.newton@linaro.org> >> >> * printcmd.c (build_address_symbolic): Call >> gdbarch_addr_bits_remove for text minimal symbols. >> --- >> gdb/printcmd.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/gdb/printcmd.c b/gdb/printcmd.c >> index 7beb334..619e684 100644 >> --- a/gdb/printcmd.c >> +++ b/gdb/printcmd.c >> @@ -689,6 +689,16 @@ build_address_symbolic (struct gdbarch *gdbarch, >> { >> if (SYMBOL_VALUE_ADDRESS (msymbol) > name_location || symbol == NULL) >> { >> + /* If this is a function (i.e. a code address), strip out any >> + non-address bits. For instance, display a pointer to the >> + first instruction of a Thumb function as <function>; the >> + second instruction will be <function+2>, even though the >> + pointer is <function+3>. This matches the ISA behavior. */ >> + if (MSYMBOL_TYPE (msymbol) == mst_text >> + || MSYMBOL_TYPE (msymbol) == mst_text_gnu_ifunc >> + || MSYMBOL_TYPE (msymbol) == mst_file_text) >> + addr = gdbarch_addr_bits_remove (gdbarch, addr); > > Shouldn't we do this for all text symbols, and thus for > mst_solib_trampoline too? Sounds sensible. I'll respin the patch and retest. -- Will Newton Toolchain Working Group, Linaro
diff --git a/gdb/printcmd.c b/gdb/printcmd.c index 7beb334..619e684 100644 --- a/gdb/printcmd.c +++ b/gdb/printcmd.c @@ -689,6 +689,16 @@ build_address_symbolic (struct gdbarch *gdbarch, { if (SYMBOL_VALUE_ADDRESS (msymbol) > name_location || symbol == NULL) { + /* If this is a function (i.e. a code address), strip out any + non-address bits. For instance, display a pointer to the + first instruction of a Thumb function as <function>; the + second instruction will be <function+2>, even though the + pointer is <function+3>. This matches the ISA behavior. */ + if (MSYMBOL_TYPE (msymbol) == mst_text + || MSYMBOL_TYPE (msymbol) == mst_text_gnu_ifunc + || MSYMBOL_TYPE (msymbol) == mst_file_text) + addr = gdbarch_addr_bits_remove (gdbarch, addr); + /* The msymbol is closer to the address than the symbol; use the msymbol instead. */ symbol = 0;