From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19732 invoked by alias); 31 Jan 2008 17:58:37 -0000 Received: (qmail 19719 invoked by uid 22791); 31 Jan 2008 17:58:35 -0000 X-Spam-Check-By: sourceware.org Received: from dmz.mips-uk.com (HELO dmz.mips-uk.com) (194.74.144.194) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 31 Jan 2008 17:58:05 +0000 Received: from internal-mx1 ([192.168.192.240] helo=ukservices1.mips.com) by dmz.mips-uk.com with esmtp (Exim 3.35 #1 (Debian)) id 1JKdfm-0004Pu-00; Thu, 31 Jan 2008 17:58:02 +0000 Received: from perivale.mips.com ([192.168.192.200]) by ukservices1.mips.com with esmtp (Exim 3.36 #1 (Debian)) id 1JKdfh-0004KC-00; Thu, 31 Jan 2008 17:57:57 +0000 Received: from macro (helo=localhost) by perivale.mips.com with local-esmtp (Exim 4.63) (envelope-from ) id 1JKdfh-00037j-ET; Thu, 31 Jan 2008 17:57:57 +0000 Date: Thu, 31 Jan 2008 18:14:00 -0000 From: "Maciej W. Rozycki" To: gdb-patches@sourceware.org cc: "Maciej W. Rozycki" Subject: MIPS: Handle manual calls of MIPS16 functions with a call stub Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-MIPS-Technologies-UK-MailScanner: Found to be clean X-MIPS-Technologies-UK-MailScanner-From: macro@mips.com Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-01/txt/msg00860.txt.bz2 Hello, There is a problem with GDB calling some of MIPS16 functions manually. It happens when such a function has a call stub (which is needed due to an ABI difference when passing floating-point arguments when called from standard MIPS code) and DWARF-2 information is available for the function. In this case the stub, which is prepended to the function itself, is made a part of the function's DWARF-2 subprogram block. The PC range of the block is determined by two labels that are put at the beginning and the end of the block each. GDB applies the least significant bit (LSB) of the start address of a DWARF-2 block to its internal block structure associated with a function and uses it to determine whether the function is MIPS16 code (when set) or standard MIPS code (if clear). The bit in the start address is set in the DWARF-2 record by BFD at the link time based on the STO_MIPS16 annotation of the symbol at the same address in the ELF symbol table. If the start address is the same as the regular entry point of the function, which is the case when no call stub has been generated for the function in question, then the DWARF-2 record gets updated accordingly and the bit is correctly set. However, when the call stub indeed is there, the function's entry point is at a different location, and the stub's entry point point is standard MIPS code and therefore bearing no STO_MIPS16 annotation. In this case the bit in the DWARF-2 record remains clear. This leads to incorrect operation and indeed a couple of failures in the GDB regression test suite. This is because MIPS16 functions are called as if they were standard MIPS code and therefore a different (and effectively random) stream of instructions is executed than one that has been intended. Here is a patch that fixes the problem in a way that I attempted to make generic. The change introduces an additional architecture-specific request that is done when the GDB's internal block structure is being constructed for a function. This is an opportunity for the target-specific backend to perform an adjustment of the block. The default is to do nothing. With this change the MIPS backend applies a fix-up to the start address of the block. It makes use of the observation, that the real entry point of a MIPS16 function is always correctly annotated (with STO_MIPS16) in the ELF symbol table. This annotation is carried forward to GDB's minimal symbol table and available from there as a "special symbol" flag. When called for a block being built the backend performs lookup in the minimal symbol table and checks whether it has been marked as special there. If so, it "marks the symbol as special" in the GDB's regular symbol table as well, which in this case means setting the LSB in the start address. Only the DWARF-2 parser is addressed with this change, though other debugging formats used for MIPS might be affected as well. I have no easy way to test them at the moment though and we all want to use DWARF-2 anyway, don't we? I have regression tested the change using the mipsisa32-sde-elf target with the following boards: 1. mips-sim-sde32/-EB/-march=mips32r2 -- no regressions, 2. mips-sim-sde32/-EL/-march=mips32r2 -- no regressions, 3. mips-sim-sde32/-EB/-march=mips32r2/-mips16/-Wa,-O0 -- a number of cases, namely: gdb.base/call-ar-st.exp gdb.base/call-sc.exp gdb.base/callfuncs.exp do not crash prematurely, and there are a couple of failures gone elsewhere too, ultimately yielding the following improvement in the final results: -# of expected passes 9947 -# of unexpected failures 340 +# of expected passes 10020 +# of unexpected failures 270 4. mips-sim-sde32/-EL/-march=mips32r2/-mips16/-Wa,-O0 -- similar effects as #3 above, with final results as follow: -# of expected passes 9945 -# of unexpected failures 342 +# of expected passes 10032 +# of unexpected failures 258 gdb/: 2008-01-31 Maciej W. Rozycki * arch-utils.c (default_make_symbol_special): New function. * arch-utils.h (default_make_symbol_special): New declaration. * Makefile.in (arch-utils.o): Update dependencies. * mips-tdep.c (mips_make_symbol_special): New function. (mips_gdbarch_init): Install it. * gdbarch.sh (make_symbol_special): New architecture method. * gdbarch.c, gdbarch.h: Regenerate. * dwarf2read.c (read_func_scope): Call gdbarch_make_symbol_special(). gdb/testsuite/: 2008-01-31 Maciej W. Rozycki * gdb.cp/cp-relocate.exp: Qualify caller() being looked up. The change to the test suite is required, because caller() is a MIPS16 function with the boards #3 and #4 above and with this fix not at 0x0 anymore, but rather 0x1. Therefore it has be properly qualified as the name will actually appear in the output from GDB in this configuration and the test case may not rely on it being swallowed and pass random rubbish as the matching pattern. OK to apply? Maciej gdb-make_symbol_special.diff Index: binutils-quilt/src/gdb/Makefile.in =================================================================== --- binutils-quilt.orig/src/gdb/Makefile.in 2008-01-30 13:17:11.000000000 +0000 +++ binutils-quilt/src/gdb/Makefile.in 2008-01-30 13:17:18.000000000 +0000 @@ -1912,8 +1912,8 @@ $(gdbtypes_h) $(breakpoint_h) arch-utils.o: arch-utils.c $(defs_h) $(arch_utils_h) $(buildsym_h) \ $(gdbcmd_h) $(inferior_h) $(gdb_string_h) $(regcache_h) \ - $(gdb_assert_h) $(sim_regno_h) $(gdbcore_h) $(osabi_h) $(version_h) \ - $(floatformat_h) $(target_descriptions_h) + $(gdb_assert_h) $(sim_regno_h) $(gdbcore_h) $(osabi_h) $(objfiles_h) \ + $(symtab_h) $(version_h) $(floatformat_h) $(target_descriptions_h) arm-linux-nat.o: arm-linux-nat.c $(defs_h) $(inferior_h) $(gdbcore_h) \ $(gdb_string_h) $(regcache_h) $(arm_tdep_h) $(gregset_h) \ $(target_h) $(linux_nat_h) $(gdb_proc_service_h) $(arm_linux_tdep_h) \ Index: binutils-quilt/src/gdb/arch-utils.c =================================================================== --- binutils-quilt.orig/src/gdb/arch-utils.c 2008-01-30 13:17:11.000000000 +0000 +++ binutils-quilt/src/gdb/arch-utils.c 2008-01-30 13:17:18.000000000 +0000 @@ -31,6 +31,8 @@ #include "gdbcore.h" #include "osabi.h" #include "target-descriptions.h" +#include "objfiles.h" +#include "symtab.h" #include "version.h" @@ -124,6 +126,12 @@ return; } +void +default_make_symbol_special (struct symbol *sym, struct objfile *objfile) +{ + return; +} + int cannot_register_not (struct gdbarch *gdbarch, int regnum) { Index: binutils-quilt/src/gdb/arch-utils.h =================================================================== --- binutils-quilt.orig/src/gdb/arch-utils.h 2008-01-30 13:17:11.000000000 +0000 +++ binutils-quilt/src/gdb/arch-utils.h 2008-01-30 13:17:18.000000000 +0000 @@ -51,6 +51,10 @@ void default_coff_make_msymbol_special (int val, struct minimal_symbol *msym); +/* Do nothing version of make_symbol_special. */ + +void default_make_symbol_special (struct symbol *, struct objfile *); + /* Version of cannot_fetch_register() / cannot_store_register() that always fails. */ Index: binutils-quilt/src/gdb/dwarf2read.c =================================================================== --- binutils-quilt.orig/src/gdb/dwarf2read.c 2008-01-30 13:17:11.000000000 +0000 +++ binutils-quilt/src/gdb/dwarf2read.c 2008-01-30 13:17:18.000000000 +0000 @@ -3011,7 +3011,9 @@ /* If we have address ranges, record them. */ dwarf2_record_block_ranges (die, block, baseaddr, cu); - + + gdbarch_make_symbol_special (current_gdbarch, new->name, objfile); + /* In C++, we can have functions nested inside functions (e.g., when a function declares a class that has methods). This means that when we finish processing a function scope, we may need to go Index: binutils-quilt/src/gdb/gdbarch.c =================================================================== --- binutils-quilt.orig/src/gdb/gdbarch.c 2008-01-30 13:17:11.000000000 +0000 +++ binutils-quilt/src/gdb/gdbarch.c 2008-01-30 13:17:18.000000000 +0000 @@ -48,6 +48,8 @@ #include "reggroups.h" #include "osabi.h" #include "gdb_obstack.h" +#include "objfiles.h" +#include "symtab.h" /* Static function declarations */ @@ -213,6 +215,7 @@ gdbarch_construct_inferior_arguments_ftype *construct_inferior_arguments; gdbarch_elf_make_msymbol_special_ftype *elf_make_msymbol_special; gdbarch_coff_make_msymbol_special_ftype *coff_make_msymbol_special; + gdbarch_make_symbol_special_ftype *make_symbol_special; const char * name_of_malloc; int cannot_step_breakpoint; int have_nonsteppable_watchpoint; @@ -335,6 +338,7 @@ construct_inferior_arguments, /* construct_inferior_arguments */ 0, /* elf_make_msymbol_special */ 0, /* coff_make_msymbol_special */ + 0, /* make_symbol_special */ "malloc", /* name_of_malloc */ 0, /* cannot_step_breakpoint */ 0, /* have_nonsteppable_watchpoint */ @@ -429,6 +433,7 @@ gdbarch->construct_inferior_arguments = construct_inferior_arguments; gdbarch->elf_make_msymbol_special = default_elf_make_msymbol_special; gdbarch->coff_make_msymbol_special = default_coff_make_msymbol_special; + gdbarch->make_symbol_special = default_make_symbol_special; gdbarch->name_of_malloc = "malloc"; gdbarch->register_reggroup_p = default_register_reggroup_p; /* gdbarch_alloc() */ @@ -573,6 +578,7 @@ /* Skip verify of construct_inferior_arguments, invalid_p == 0 */ /* Skip verify of elf_make_msymbol_special, invalid_p == 0 */ /* Skip verify of coff_make_msymbol_special, invalid_p == 0 */ + /* Skip verify of make_symbol_special, invalid_p == 0 */ /* Skip verify of name_of_malloc, invalid_p == 0 */ /* Skip verify of cannot_step_breakpoint, invalid_p == 0 */ /* Skip verify of have_nonsteppable_watchpoint, invalid_p == 0 */ @@ -805,6 +811,9 @@ "gdbarch_dump: long_long_bit = %s\n", paddr_d (gdbarch->long_long_bit)); fprintf_unfiltered (file, + "gdbarch_dump: make_symbol_special = <0x%lx>\n", + (long) gdbarch->make_symbol_special); + fprintf_unfiltered (file, "gdbarch_dump: memory_insert_breakpoint = <0x%lx>\n", (long) gdbarch->memory_insert_breakpoint); fprintf_unfiltered (file, @@ -2622,6 +2631,23 @@ gdbarch->coff_make_msymbol_special = coff_make_msymbol_special; } +void +gdbarch_make_symbol_special (struct gdbarch *gdbarch, struct symbol *sym, struct objfile *objfile) +{ + gdb_assert (gdbarch != NULL); + gdb_assert (gdbarch->make_symbol_special != NULL); + if (gdbarch_debug >= 2) + fprintf_unfiltered (gdb_stdlog, "gdbarch_make_symbol_special called\n"); + gdbarch->make_symbol_special (sym, objfile); +} + +void +set_gdbarch_make_symbol_special (struct gdbarch *gdbarch, + gdbarch_make_symbol_special_ftype make_symbol_special) +{ + gdbarch->make_symbol_special = make_symbol_special; +} + const char * gdbarch_name_of_malloc (struct gdbarch *gdbarch) { Index: binutils-quilt/src/gdb/gdbarch.h =================================================================== --- binutils-quilt.orig/src/gdb/gdbarch.h 2008-01-30 13:17:11.000000000 +0000 +++ binutils-quilt/src/gdb/gdbarch.h 2008-01-30 13:17:18.000000000 +0000 @@ -50,6 +50,8 @@ struct obstack; struct bp_target_info; struct target_desc; +struct objfile; +struct symbol; extern struct gdbarch *current_gdbarch; @@ -576,6 +578,10 @@ extern void gdbarch_coff_make_msymbol_special (struct gdbarch *gdbarch, int val, struct minimal_symbol *msym); extern void set_gdbarch_coff_make_msymbol_special (struct gdbarch *gdbarch, gdbarch_coff_make_msymbol_special_ftype *coff_make_msymbol_special); +typedef void (gdbarch_make_symbol_special_ftype) (struct symbol *sym, struct objfile *objfile); +extern void gdbarch_make_symbol_special (struct gdbarch *gdbarch, struct symbol *sym, struct objfile *objfile); +extern void set_gdbarch_make_symbol_special (struct gdbarch *gdbarch, gdbarch_make_symbol_special_ftype *make_symbol_special); + extern const char * gdbarch_name_of_malloc (struct gdbarch *gdbarch); extern void set_gdbarch_name_of_malloc (struct gdbarch *gdbarch, const char * name_of_malloc); Index: binutils-quilt/src/gdb/gdbarch.sh =================================================================== --- binutils-quilt.orig/src/gdb/gdbarch.sh 2008-01-30 13:17:11.000000000 +0000 +++ binutils-quilt/src/gdb/gdbarch.sh 2008-01-30 13:17:18.000000000 +0000 @@ -579,6 +579,7 @@ m:char *:construct_inferior_arguments:int argc, char **argv:argc, argv::construct_inferior_arguments::0 f:void:elf_make_msymbol_special:asymbol *sym, struct minimal_symbol *msym:sym, msym::default_elf_make_msymbol_special::0 f:void:coff_make_msymbol_special:int val, struct minimal_symbol *msym:val, msym::default_coff_make_msymbol_special::0 +f:void:make_symbol_special:struct symbol *sym, struct objfile *objfile:sym, objfile::default_make_symbol_special::0 v:const char *:name_of_malloc:::"malloc":"malloc"::0:gdbarch->name_of_malloc v:int:cannot_step_breakpoint:::0:0::0 v:int:have_nonsteppable_watchpoint:::0:0::0 @@ -729,6 +730,8 @@ struct obstack; struct bp_target_info; struct target_desc; +struct objfile; +struct symbol; extern struct gdbarch *current_gdbarch; EOF Index: binutils-quilt/src/gdb/mips-tdep.c =================================================================== --- binutils-quilt.orig/src/gdb/mips-tdep.c 2008-01-30 13:17:11.000000000 +0000 +++ binutils-quilt/src/gdb/mips-tdep.c 2008-01-30 13:17:18.000000000 +0000 @@ -333,6 +333,24 @@ return (((long) MSYMBOL_INFO (msym) & 0x80000000) != 0); } +/* macro/2008-01-29: Some MIPS16 functions have a call stub prepended + that is standard MIPS code and as a result the begin-of-the-block + label that DWARF2 records as emitted by GCC use does not have its LSB + set. Consequently the whole block is marked as standard MIPS code. + Hack the problem around by retrieving the MIPS16 annotation from the + corresponding minimal symbol, which always carries correct + information, and adjusting the addresses as necessary. */ + +static void +mips_make_symbol_special (struct symbol *sym, struct objfile *objfile) +{ + struct minimal_symbol *msym; + + msym = lookup_minimal_symbol (DEPRECATED_SYMBOL_NAME (sym), NULL, objfile); + if (msym && msymbol_is_special (msym) && SYMBOL_CLASS (sym) == LOC_BLOCK) + BLOCK_START (SYMBOL_BLOCK_VALUE (sym)) |= 1; +} + /* XFER a value from the big/little/left end of the register. Depending on the size of the value it might occupy the entire register or just part of it. Make an allowance for this, aligning @@ -5730,6 +5748,7 @@ set_gdbarch_elf_make_msymbol_special (gdbarch, mips_elf_make_msymbol_special); + set_gdbarch_make_symbol_special (gdbarch, mips_make_symbol_special); regnum = GDBARCH_OBSTACK_ZALLOC (gdbarch, struct mips_regnum); *regnum = mips_regnum; Index: binutils-quilt/src/gdb/testsuite/gdb.cp/cp-relocate.exp =================================================================== --- binutils-quilt.orig/src/gdb/testsuite/gdb.cp/cp-relocate.exp 2008-01-07 14:29:21.000000000 +0000 +++ binutils-quilt/src/gdb/testsuite/gdb.cp/cp-relocate.exp 2008-01-30 13:17:18.000000000 +0000 @@ -78,7 +78,7 @@ # Check that all the functions have different addresses. set func1_addr [get_func_address "$func1_name"] set func2_addr [get_func_address "$func2_name"] -set caller_addr [get_func_address "caller"] +set caller_addr [get_func_address "caller()"] if { "${func1_addr}" == "${func2_addr}" || "${func1_addr}" == "${func2_addr}"