This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On 05/24/2012 08:26 PM, Jeff Kenton wrote:
On 05/23/2012 01:33 PM, Pedro Alves wrote:Hi Jeff,
On 05/21/2012 03:23 PM, Jeff Kenton wrote:
- Comments applied. - SIGTRAMP frame handling improved. - Patch attached.
For the future, it's easier for the reviewer if you reply to the issues and questions raised. For example, in patch 2, I'm left wondering if the ps_get_thread_area addition fixed any testsuite failures, indicating it is correct. I'm also left wondering what does "improved" mean (probably fixed something in the testsuite?).
diff -r -u -N /home/packages/gdb-7.4x/gdb/tilegx-linux-tdep.c ./gdb/tilegx-linux-tdep.c --- /home/packages/gdb-7.4x/gdb/tilegx-linux-tdep.c 1969-12-31 19:00:00.000000000 -0500 +++ ./gdb/tilegx-linux-tdep.c 2012-05-24 13:43:30.089276000 -0400 +/* Supply raw registers from REGCACHE to REGS. */ + +/* TILE-Gx has 56 general purpose registers, + plus 8 special general purpose registers (network and ZERO), + plus 1 magic register (PC == 64). + We only need the following 2 enums here. */ + +enum { + E_NUM_EASY_REGS = 56, + E_PC_REGNUM = 64 +}; + +enum { tilegx_reg_size = 8 };
I didn't pay close attention to what these two enums were before. The standard thing to do instead of defining tilegx_reg_size twice, and hacking up that short enum for the register numbers, is to move the register numbers enum from tilegx-tdep.c to and tilegx-tdep.h file, and include that where necessary, so we only have one place with the register numbers. 'E_' isn't a great prefix (does it mean 'enum'?), so please replace it with something like TILEGX_PC_REGNUM, etc. along with the move.
+/* OS specific initialization of gdbarch. */ + +static void +tilegx_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) +{ + int arch_size = 64; + + /* Handle arch_size == 32 or 64. Default to 64. */ + if (info.abfd) + arch_size = bfd_get_arch_size (info.abfd); + + if (arch_size != 32) + arch_size = 64;
Why is this 'if' necessary? Does bfd_get_arch_size ever return anything not 32 or 64? In any case, you can just use gdbarch_addr_bit here, as tilegx_gdbarch_init has already done the similar bfd_get_arch_size bit.
+/* This is the implementation of gdbarch method breakpoint_from_pc. */ + +static const unsigned char * +tilegx_breakpoint_from_pc (struct gdbarch *gdbarch, + CORE_ADDR *pcptr, int *lenptr) +{ + /* 64-bit pattern for a { bpt ; nop } bundle. */ + static unsigned char breakpoint[] =
static const.
+ { 0x00, 0x50, 0x48, 0x51, 0xae, 0x44, 0x6a, 0x28 }; + + *lenptr = sizeof (breakpoint); + return breakpoint; +} +I think we're getting close. If Joel or anyone else has further comments, the next iteration should be about ready to go in.
Attachment:
patch_1
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |