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

Maciej W. Rozycki
2008-01-31T17:58:37Z

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