Fix target architecture address size inside gdbarch structure

Message ID CAF3nGXdMOH5ynBPRNQ0XxrrFYZz7ujGqCVrp3=h+MSsTByNBUw@mail.gmail.com
State New, archived
Headers

Commit Message

Denis Dmitriev Oct. 16, 2018, 12:03 p.m. UTC
  Hi all,
GDB can't get information about instructions when debugging mips:octeon2
architecture.
The problem is that the gdbarch structure contains the wrong address size
for target architecture (gdbarch->addr_bit, 32).
However, in the same structure there is data settings for the target
architecture (mips:octeon2) which indicate the correct address size
(gdbarch->bfd_arch_info->bits_per_address, 64).
This patch fixes creation gdbarch structure. Now the address size is taken
directly from the settings (gdbarch->bfd_arch_info).

   if (gdbarch->char_signed == -1)
  

Comments

Simon Marchi Oct. 16, 2018, 9:13 p.m. UTC | #1
On 2018-10-16 08:03, Denis Dmitriev wrote:
> Hi all,
> GDB can't get information about instructions when debugging 
> mips:octeon2
> architecture.
> The problem is that the gdbarch structure contains the wrong address 
> size
> for target architecture (gdbarch->addr_bit, 32).
> However, in the same structure there is data settings for the target
> architecture (mips:octeon2) which indicate the correct address size
> (gdbarch->bfd_arch_info->bits_per_address, 64).
> This patch fixes creation gdbarch structure. Now the address size is 
> taken
> directly from the settings (gdbarch->bfd_arch_info).

Hi Denis,

This file (gdbarch.c) is generated from gdbarch.sh, so it can't be 
modified directly.  What you see here:

   gdbarch->addr_bit = gdbarch_ptr_bit (gdbarch);

is the result from the default value of the addr_bit property in 
gdbarch.sh:

402 v;int;addr_bit;;;8 * sizeof (void*);0;gdbarch_ptr_bit (gdbarch);

So if any modification needs to be done here, it needs to be in 
gdbarch.sh.

However, I tried generating an executable with:

$ mips64el-linux-gnuabi64-gcc --versio
mips64el-linux-gnuabi64-gcc (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 
20160609
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is 
NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR 
PURPOSE.
$ mips64el-linux-gnuabi64-gcc test.c -o test

And the resulting addr_bit and ptr_bit in gdb are both 64.  From you 
explanations, I get that the value you get for ptr_bit is 32.  Is this 
value the right one, or it should be 64?

Could you share how you generate you executable, with what toolchain, 
etc?

Simon
  
Denis Dmitriev Oct. 17, 2018, 11:33 a.m. UTC | #2
hi!

gcc --version
gcc (Ubuntu 5.4.0-6ubuntu1~16.04.10) 5.4.0 20160609
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

uname -a
Linux heroboecPC 4.15.0-34-generic #37~16.04.1-Ubuntu SMP Tue Aug 28
10:44:06 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux


I took gdb 8.2.50.20180906-git (hash:
739ab2e92e1840c9285f3cfce1f1236c0fa68730).
./configure --host=x86_64-linux --target=mips64-linux-uclibc
make
After that, I start the debugger, install the architecture for debugging.
set arch mips:octeon2
tar rem :1234 (i have qemu with mips to connect)
disas $pc, $pc+20
0xbfc00000 in ?? ()
(gdb) disas $pc, $pc+20
Dump of assembler code from 0xbfc00000 to 0xbfc00014:
=> 0xbfc00000:  Cannot access memory at address 0xbfc00000

Address size is 32 bit instead of 64.

In the cpu-mips.c file says that the address should be 64 bit size.
160: N (64, 64, bfd_mach_mips_octeon2,"mips:octeon2",  FALSE,
NN(I_mipsocteon2)),

ср, 17 окт. 2018 г. в 0:13, Simon Marchi <simon.marchi@polymtl.ca>:

> On 2018-10-16 08:03, Denis Dmitriev wrote:
> > Hi all,
> > GDB can't get information about instructions when debugging
> > mips:octeon2
> > architecture.
> > The problem is that the gdbarch structure contains the wrong address
> > size
> > for target architecture (gdbarch->addr_bit, 32).
> > However, in the same structure there is data settings for the target
> > architecture (mips:octeon2) which indicate the correct address size
> > (gdbarch->bfd_arch_info->bits_per_address, 64).
> > This patch fixes creation gdbarch structure. Now the address size is
> > taken
> > directly from the settings (gdbarch->bfd_arch_info).
>
> Hi Denis,
>
> This file (gdbarch.c) is generated from gdbarch.sh, so it can't be
> modified directly.  What you see here:
>
>    gdbarch->addr_bit = gdbarch_ptr_bit (gdbarch);
>
> is the result from the default value of the addr_bit property in
> gdbarch.sh:
>
> 402 v;int;addr_bit;;;8 * sizeof (void*);0;gdbarch_ptr_bit (gdbarch);
>
> So if any modification needs to be done here, it needs to be in
> gdbarch.sh.
>
> However, I tried generating an executable with:
>
> $ mips64el-linux-gnuabi64-gcc --versio
> mips64el-linux-gnuabi64-gcc (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0
> 20160609
> Copyright (C) 2015 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is
> NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
> PURPOSE.
> $ mips64el-linux-gnuabi64-gcc test.c -o test
>
> And the resulting addr_bit and ptr_bit in gdb are both 64.  From you
> explanations, I get that the value you get for ptr_bit is 32.  Is this
> value the right one, or it should be 64?
>
> Could you share how you generate you executable, with what toolchain,
> etc?
>
> Simon
>
  
Simon Marchi Oct. 17, 2018, 1:02 p.m. UTC | #3
On 2018-10-17 07:33, Denis Dmitriev wrote:
> hi!
> 
> gcc --version
> gcc (Ubuntu 5.4.0-6ubuntu1~16.04.10) 5.4.0 20160609
> Copyright (C) 2015 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is 
> NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR 
> PURPOSE.
> 
> uname -a
> Linux heroboecPC 4.15.0-34-generic #37~16.04.1-Ubuntu SMP Tue Aug 28
> 10:44:06 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
> 
> 
> I took gdb 8.2.50.20180906-git (hash:
> 739ab2e92e1840c9285f3cfce1f1236c0fa68730).
> ./configure --host=x86_64-linux --target=mips64-linux-uclibc
> make
> After that, I start the debugger, install the architecture for 
> debugging.
> set arch mips:octeon2
> tar rem :1234 (i have qemu with mips to connect)
> disas $pc, $pc+20
> 0xbfc00000 in ?? ()
> (gdb) disas $pc, $pc+20
> Dump of assembler code from 0xbfc00000 to 0xbfc00014:
> => 0xbfc00000:  Cannot access memory at address 0xbfc00000
> 
> Address size is 32 bit instead of 64.

Looking at the code, there is the mips ABI setting that comes into play, 
that's where the ptr size (and consequently the addr size):

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/mips-tdep.c;h=bf44c52f5d2ad73605cdfaabe1ac1d2bde822ef9;hb=HEAD#l8541

When loading a binary in GDB, it auto detects the ABI:

$ ./gdb -q --data-directory=data-directory test
Reading symbols from test...
(No debugging symbols found in test)
(gdb) show architecture
The target architecture is set automatically (currently mips:isa64r2)
(gdb) show mips abi
The MIPS ABI is set automatically (currently "n64").


If you don't provide a binary to gdb, then you should probably set this 
to the right value:

(gdb) set architecture mips:octeon2
The target architecture is assumed to be mips:octeon2
(gdb) show mips abi
The MIPS ABI is set automatically (currently "o32").
(gdb) p sizeof(void*)
$1 = 4
(gdb) set mips abi n64
(gdb) p sizeof(void*)
$2 = 8

I don't know if it makes sense to have the architecture "mips:octeon2" 
along with a 32-bits ABI, I don't really know much about MIPS.  If it 
doesn't, perhaps it would be possible to select a more sensible ABI by 
default when selecting that architecture.  But for now, you have to 
select it by hand.  Or if you can provide the binary to gdb, that's the 
ideal.

Simon
  
Denis Dmitriev Oct. 17, 2018, 2:35 p.m. UTC | #4
This behavior is correct for the case when there is a specific elf file.
But I think in the case when using remote debugging and manual adjustment
of the architecture, it's possible to choose the ABI type without an elf
file.
All necessary information is already available in the structure of the
gdbarch->bfd_arch_info.


For example mips-tdep.c

8067 static struct gdbarch *
8068 mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list
*arches)
8069 {
8070   struct gdbarch *gdbarch;
8071   struct gdbarch_tdep *tdep;
8072   int elf_flags;
8073   enum mips_abi mips_abi, found_abi, wanted_abi;
8074   int i, num_regs;
8075   enum mips_fpu_type fpu_type;
8076   struct tdesc_arch_data *tdesc_data = NULL;
8077   int elf_fpu_type = Val_GNU_MIPS_ABI_FP_ANY;
8078   const char **reg_names;
8079   struct mips_regnum mips_regnum, *regnum;
8080   enum mips_isa mips_isa;
8081   int dspacc;
8082   int dspctl;
8083
8084   /* First of all, extract the elf_flags, if available.  */
8085   if (info.abfd && bfd_get_flavour (info.abfd) ==
bfd_target_elf_flavour)
8086     elf_flags = elf_elfheader (info.abfd)->e_flags;
8087   else if (arches != NULL)
8088     elf_flags = gdbarch_tdep (arches->gdbarch)->elf_flags;
// in the case when using remote debugging and manual adjustment of the
architecture value of elf_flags = 0
// Here we can check the existence of a gdbarch->bfd_arch_info structure
and adjust the elf_flag as if it was read from a real elf file.
8089   else
8090     elf_flags = 0;
8091   if (gdbarch_debug)
8092     fprintf_unfiltered (gdb_stdlog,
8093             "mips_gdbarch_init: elf_flags = 0x%08x\n", elf_flags);

Further, on the basis of elf_flag, we get the correct type of ABI. In this
case, this solution is for mips only.



ср, 17 окт. 2018 г. в 16:02, Simon Marchi <simon.marchi@polymtl.ca>:

> On 2018-10-17 07:33, Denis Dmitriev wrote:
> > hi!
> >
> > gcc --version
> > gcc (Ubuntu 5.4.0-6ubuntu1~16.04.10) 5.4.0 20160609
> > Copyright (C) 2015 Free Software Foundation, Inc.
> > This is free software; see the source for copying conditions.  There is
> > NO
> > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
> > PURPOSE.
> >
> > uname -a
> > Linux heroboecPC 4.15.0-34-generic #37~16.04.1-Ubuntu SMP Tue Aug 28
> > 10:44:06 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
> >
> >
> > I took gdb 8.2.50.20180906-git (hash:
> > 739ab2e92e1840c9285f3cfce1f1236c0fa68730).
> > ./configure --host=x86_64-linux --target=mips64-linux-uclibc
> > make
> > After that, I start the debugger, install the architecture for
> > debugging.
> > set arch mips:octeon2
> > tar rem :1234 (i have qemu with mips to connect)
> > disas $pc, $pc+20
> > 0xbfc00000 in ?? ()
> > (gdb) disas $pc, $pc+20
> > Dump of assembler code from 0xbfc00000 to 0xbfc00014:
> > => 0xbfc00000:  Cannot access memory at address 0xbfc00000
> >
> > Address size is 32 bit instead of 64.
>
> Looking at the code, there is the mips ABI setting that comes into play,
> that's where the ptr size (and consequently the addr size):
>
>
> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/mips-tdep.c;h=bf44c52f5d2ad73605cdfaabe1ac1d2bde822ef9;hb=HEAD#l8541
>
> When loading a binary in GDB, it auto detects the ABI:
>
> $ ./gdb -q --data-directory=data-directory test
> Reading symbols from test...
> (No debugging symbols found in test)
> (gdb) show architecture
> The target architecture is set automatically (currently mips:isa64r2)
> (gdb) show mips abi
> The MIPS ABI is set automatically (currently "n64").
>
>
> If you don't provide a binary to gdb, then you should probably set this
> to the right value:
>
> (gdb) set architecture mips:octeon2
> The target architecture is assumed to be mips:octeon2
> (gdb) show mips abi
> The MIPS ABI is set automatically (currently "o32").
> (gdb) p sizeof(void*)
> $1 = 4
> (gdb) set mips abi n64
> (gdb) p sizeof(void*)
> $2 = 8
>
> I don't know if it makes sense to have the architecture "mips:octeon2"
> along with a 32-bits ABI, I don't really know much about MIPS.  If it
> doesn't, perhaps it would be possible to select a more sensible ABI by
> default when selecting that architecture.  But for now, you have to
> select it by hand.  Or if you can provide the binary to gdb, that's the
> ideal.
>
> Simon
>
  

Patch

diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index e2abf26..9d130a8 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -542,7 +542,12 @@  verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of floatformat_for_type, invalid_p == 0 */
   /* Skip verify of ptr_bit, invalid_p == 0 */
   if (gdbarch->addr_bit == 0)
-    gdbarch->addr_bit = gdbarch_ptr_bit (gdbarch);
+  {
+    if (gdbarch->bfd_arch_info)
+      gdbarch->addr_bit = gdbarch->bfd_arch_info->bits_per_address;
+    else
+      gdbarch->addr_bit = gdbarch_ptr_bit(gdbarch);
+  }
   if (gdbarch->dwarf2_addr_size == 0)
     gdbarch->dwarf2_addr_size = gdbarch_ptr_bit (gdbarch) /
TARGET_CHAR_BIT;