[PATCH 1/3] gdb/printcmd.c: Fix printing of Thumb minimal symbols.

[PATCH 1/3] gdb/printcmd.c: Fix printing of Thumb minimal symbols.

[Will Newton]

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);
+
 	  /* The msymbol is closer to the address than the symbol;
 	     use the msymbol instead.  */
 	  symbol = 0;
-- 
1.8.1.4

Re: [PATCH 1/3] gdb/printcmd.c: Fix printing of Thumb minimal symbols.

[Pedro Alves]

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;
-- 
Pedro Alves

Re: [PATCH 1/3] gdb/printcmd.c: Fix printing of Thumb minimal symbols.

[Will Newton]

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