MIPS: Handle manual calls of MIPS16 functions with a call stub

["Maciej W. Rozycki" <macro at mips dot com>]

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  <macro@mips.com>

	* 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  <macro@mips.com>

	* 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}"