Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* MIPS: Handle manual calls of MIPS16 functions with a call stub
@ 2008-01-31 18:14 Maciej W. Rozycki
  2008-01-31 22:08 ` Daniel Jacobowitz
  0 siblings, 1 reply; 21+ messages in thread
From: Maciej W. Rozycki @ 2008-01-31 18:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: Maciej W. Rozycki

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: MIPS: Handle manual calls of MIPS16 functions with a call stub
  2008-01-31 18:14 MIPS: Handle manual calls of MIPS16 functions with a call stub Maciej W. Rozycki
@ 2008-01-31 22:08 ` Daniel Jacobowitz
  2008-02-01 10:27   ` Maciej W. Rozycki
  0 siblings, 1 reply; 21+ messages in thread
From: Daniel Jacobowitz @ 2008-01-31 22:08 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: gdb-patches, Maciej W. Rozycki

On Thu, Jan 31, 2008 at 05:57:57PM +0000, Maciej W. Rozycki wrote:
> 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.

Does this mean the DWARF block describes the MIPS16 parts, but the
function's minimal symbol points to the call stub, which is
elsewhere)?  Maybe mips_write_pc should use mips_pc_is_mips16; that's
how Thumb works, by always consulting the minimal symbol table to find
out whether an address should be called as MIPS16 or MIPS32.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: MIPS: Handle manual calls of MIPS16 functions with a call stub
  2008-01-31 22:08 ` Daniel Jacobowitz
@ 2008-02-01 10:27   ` Maciej W. Rozycki
  2008-02-01 14:19     ` Daniel Jacobowitz
  2008-02-04 16:14     ` Maciej W. Rozycki
  0 siblings, 2 replies; 21+ messages in thread
From: Maciej W. Rozycki @ 2008-02-01 10:27 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches, Maciej W. Rozycki

On Thu, 31 Jan 2008, Daniel Jacobowitz wrote:

> >  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.
> 
> Does this mean the DWARF block describes the MIPS16 parts, but the
> function's minimal symbol points to the call stub, which is

 The other way round -- the minimal symbol points to the actual entry 
point, but the stub precedes it and is included in the DWARF-2 block 
together with the MIPS16 function body.  Here's an example that triggers a 
failure in the test suite (generated from gdb.base/call-ar-st.c by GCC 
4.2.2):

	.text
	.align	2
	.globl	print_ten_doubles
.LFB20:
	.loc 1 664 0
	# Stub function for print_ten_doubles (double, double)
	.set	nomips16
	.section	.mips16.fn.print_ten_doubles,"ax",@progbits
	.align	2
	.ent	__fn_stub_print_ten_doubles
__fn_stub_print_ten_doubles:
	.set	noreorder
	mfc1	$4,$f13
	mfc1	$5,$f12
	mfc1	$6,$f15
	mfc1	$7,$f14
	.set	noat
	la	$1,print_ten_doubles
	jr	$1
	.set	at
	nop
	.set	reorder
	.end	__fn_stub_print_ten_doubles
	.text
	.set	mips16
	.ent	print_ten_doubles
print_ten_doubles:
	.frame	$17,8,$31		# vars= 0, regs= 2/0, args= 24, gp= 0
	.mask	0x80020000,-4
	.fmask	0x00000000,0
	save	32,$17,$31
.LCFI36:
	addiu	$17,$sp,24
.LCFI37:
	sw	$5,12($17)
	sw	$4,8($17)
	sw	$7,20($17)
	sw	$6,16($17)
	.loc 1 666 0
	lw	$4,.L134
	lw	$7,12($17)
	lw	$6,8($17)
	lw	$3,20($17)
	lw	$2,16($17)
	sw	$3,20($sp)
	sw	$2,16($sp)
	jal	printf
	.loc 1 667 0
	lw	$4,.L134
	lw	$7,28($17)
	lw	$6,24($17)
	lw	$3,36($17)
	lw	$2,32($17)
	sw	$3,20($sp)
	sw	$2,16($sp)
	jal	printf
	.loc 1 668 0
	lw	$4,.L134
	lw	$7,44($17)
	lw	$6,40($17)
	lw	$3,52($17)
	lw	$2,48($17)
	sw	$3,20($sp)
	sw	$2,16($sp)
	jal	printf
	.loc 1 669 0
	lw	$4,.L134
	lw	$7,60($17)
	lw	$6,56($17)
	lw	$3,68($17)
	lw	$2,64($17)
	sw	$3,20($sp)
	sw	$2,16($sp)
	jal	printf
	.loc 1 670 0
	lw	$4,.L134
	lw	$7,76($17)
	lw	$6,72($17)
	lw	$3,84($17)
	lw	$2,80($17)
	sw	$3,20($sp)
	sw	$2,16($sp)
	jal	printf
	.loc 1 671 0
	move	$sp,$17
	restore	8,$17,$31
	j	$31
	.align	2
.L134:
	.word	.LC75
	.end	print_ten_doubles
.LFE20:
	.size	print_ten_doubles, .-print_ten_doubles

The .LFB20 and .LFE20 labels are used as the boundaries of the function in 
the DWARF-2 block.

> elsewhere)?  Maybe mips_write_pc should use mips_pc_is_mips16; that's
> how Thumb works, by always consulting the minimal symbol table to find
> out whether an address should be called as MIPS16 or MIPS32.

 The ABI is different, so it is not enough to set the PC correctly -- more 
about it in the patch that is going through my regression testing at the 
moment.  Though perhaps the other places could use mips_pc_is_mips16() 
too.  On the other hand I feel setting the block's start address correctly 
is the right way to make the handling consistent throughout -- by using 
mips_pc_is_mips16() here and there some places may be omitted by accident.  
Hmm...

  Maciej


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: MIPS: Handle manual calls of MIPS16 functions with a call stub
  2008-02-01 10:27   ` Maciej W. Rozycki
@ 2008-02-01 14:19     ` Daniel Jacobowitz
  2008-02-01 15:34       ` Maciej W. Rozycki
  2008-02-04 16:14     ` Maciej W. Rozycki
  1 sibling, 1 reply; 21+ messages in thread
From: Daniel Jacobowitz @ 2008-02-01 14:19 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: gdb-patches, Maciej W. Rozycki

On Fri, Feb 01, 2008 at 10:26:54AM +0000, Maciej W. Rozycki wrote:
> On Thu, 31 Jan 2008, Daniel Jacobowitz wrote:
> 
> > >  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.
> > 
> > Does this mean the DWARF block describes the MIPS16 parts, but the
> > function's minimal symbol points to the call stub, which is
> 
>  The other way round -- the minimal symbol points to the actual entry 
> point, but the stub precedes it and is included in the DWARF-2 block 
> together with the MIPS16 function body.  Here's an example that triggers a 
> failure in the test suite (generated from gdb.base/call-ar-st.c by GCC 
> 4.2.2):

Then why aren't we calling the instruction at the start of the block,
i.e. the stub?  In which case not using the MIPS16 convention is
correct.  I don't see why you'd want to call
__fn_stub_print_ten_doubles as a MIPS16 function.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: MIPS: Handle manual calls of MIPS16 functions with a call stub
  2008-02-01 14:19     ` Daniel Jacobowitz
@ 2008-02-01 15:34       ` Maciej W. Rozycki
  2008-02-01 16:58         ` Daniel Jacobowitz
  0 siblings, 1 reply; 21+ messages in thread
From: Maciej W. Rozycki @ 2008-02-01 15:34 UTC (permalink / raw)
  To: Daniel Jacobowitz, Thiemo Seufer; +Cc: gdb-patches, Maciej W. Rozycki

On Fri, 1 Feb 2008, Daniel Jacobowitz wrote:

> >  The other way round -- the minimal symbol points to the actual entry 
> > point, but the stub precedes it and is included in the DWARF-2 block 
> > together with the MIPS16 function body.  Here's an example that triggers a 
> > failure in the test suite (generated from gdb.base/call-ar-st.c by GCC 
> > 4.2.2):
> 
> Then why aren't we calling the instruction at the start of the block,
> i.e. the stub?  In which case not using the MIPS16 convention is
> correct.  I don't see why you'd want to call
> __fn_stub_print_ten_doubles as a MIPS16 function.

 Well, GDB does not ever seem to call the stub.  I have not written code 
responsible for this, but I can see two possible reasons:

1. Simplicity -- depending on the callers of the function in question 
   there may be no stub.  If there are no standard MIPS callers, then the 
   stub is stripped out by the linker.

2. Performance -- the stub is a couple of additional instructions to 
   execute which buy you nothing when called from GDB as it may load the 
   correct argument registers according to the ABI in the first place.

And as I wrote the block associated with print_ten_doubles() does not span 
__fn_stub_print_ten_doubles() -- I may have not been clear enough about 
this being the case for the DWARF-2 record.

 This is what GDB has to say about the function (with the fix applied):

(gdb) print print_ten_doubles
$1 = {void (double, double, double, double, double, double, double, double,
    double, double)} 0x80020a91 <print_ten_doubles>
(gdb) print __fn_stub_print_ten_doubles
$2 = {<text variable, no debug info>} 0x800283d0 <__fn_stub_print_ten_doubles>

 And this is what the relevant DWARF-2 record holds:

 <1><de6>: Abbrev Number: 16 (DW_TAG_subprogram)
  <de7>     DW_AT_external    : 1
  <de8>     DW_AT_name        : print_ten_doubles
  <dfa>     DW_AT_decl_file   : 1
  <dfb>     DW_AT_decl_line   : 664
  <dfd>     DW_AT_low_pc      : 0x80020a90
  <e01>     DW_AT_high_pc     : 0x80020b00
  <e05>     DW_AT_frame_base  : 0x306   (location list)
  <e09>     DW_AT_sibling     : <e9d>

 I have done a little more research of this matter now and it looks like 
the reason this is happening is a likely bug somewhere in GAS.  For 
comparison, here are the unrelocated DWARF-2 records for 
print_ten_doubles() and a nearby function that has no stub:

 <1><bab>: Abbrev Number: 16 (DW_TAG_subprogram)
    <bac>   DW_AT_external    : 1
    <bad>   DW_AT_name        : init_small_structs
    <bc0>   DW_AT_decl_file   : 1
    <bc1>   DW_AT_decl_line   : 613
    <bc3>   DW_AT_low_pc      : 0x790
    <bc7>   DW_AT_high_pc     : 0x900
    <bcb>   DW_AT_frame_base  : 0x2db   (location list)
    <bcf>   DW_AT_sibling     : <0xcaf>

 <1><caf>: Abbrev Number: 16 (DW_TAG_subprogram)
    <cb0>   DW_AT_external    : 1
    <cb1>   DW_AT_name        : print_ten_doubles
    <cc3>   DW_AT_decl_file   : 1
    <cc4>   DW_AT_decl_line   : 664
    <cc6>   DW_AT_low_pc      : 0x900
    <cca>   DW_AT_high_pc     : 0x97c
    <cce>   DW_AT_frame_base  : 0x306   (location list)
    <cd2>   DW_AT_sibling     : <0xd66>

And here are the relevant relocation records:

00000bc3  00003c02 R_MIPS_32         00000790   .LFB23
00000bc7  00000202 R_MIPS_32         00000000   .text

00000cc6  00000202 R_MIPS_32         00000000   .text
00000cca  00000202 R_MIPS_32         00000000   .text

Notice that the DWARF-2 record at 0xbc3 is relocated against .LFB23 and 
one at 0xcc6 -- against .text, rather than .LFB20 as it should be.  I 
presume this is because of the section switch happening inbetween.  Or 
could it be because of ".set nomips16" actually preceding the section 
switch?  Thiemo, can you perhaps make any comments about this?

 I do not know how long this bug has been there in GAS, but it may still 
be worth handling broken binaries people may have.  Then again -- maybe 
not.  But we have no fix for GAS as yet.  Regardless I have not made a 
strong opinion either way.

  Maciej


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: MIPS: Handle manual calls of MIPS16 functions with a call stub
  2008-02-01 15:34       ` Maciej W. Rozycki
@ 2008-02-01 16:58         ` Daniel Jacobowitz
  2008-02-01 17:07           ` Maciej W. Rozycki
  0 siblings, 1 reply; 21+ messages in thread
From: Daniel Jacobowitz @ 2008-02-01 16:58 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: Thiemo Seufer, gdb-patches, Maciej W. Rozycki

On Fri, Feb 01, 2008 at 03:34:04PM +0000, Maciej W. Rozycki wrote:
> On Fri, 1 Feb 2008, Daniel Jacobowitz wrote:
> 
> > >  The other way round -- the minimal symbol points to the actual entry 
> > > point, but the stub precedes it and is included in the DWARF-2 block 
> > > together with the MIPS16 function body.  Here's an example that triggers a 
> > > failure in the test suite (generated from gdb.base/call-ar-st.c by GCC 
> > > 4.2.2):
> > 
> > Then why aren't we calling the instruction at the start of the block,
> > i.e. the stub?  In which case not using the MIPS16 convention is
> > correct.  I don't see why you'd want to call
> > __fn_stub_print_ten_doubles as a MIPS16 function.
> 
>  Well, GDB does not ever seem to call the stub.

That's because I didn't notice the section switch in your example.
The stub is not part of the block described in the DWARF-2 table;
it's physically between the labels in the assembly file, but the
labels are in the text section and the stub isn't.  So that's why we
call the real function.  One confusion solved.

>  <1><de6>: Abbrev Number: 16 (DW_TAG_subprogram)
>   <de7>     DW_AT_external    : 1
>   <de8>     DW_AT_name        : print_ten_doubles
>   <dfa>     DW_AT_decl_file   : 1
>   <dfb>     DW_AT_decl_line   : 664
>   <dfd>     DW_AT_low_pc      : 0x80020a90
>   <e01>     DW_AT_high_pc     : 0x80020b00
>   <e05>     DW_AT_frame_base  : 0x306   (location list)
>   <e09>     DW_AT_sibling     : <e9d>
> 
>  I have done a little more research of this matter now and it looks like 
> the reason this is happening is a likely bug somewhere in GAS.  For 
> comparison, here are the unrelocated DWARF-2 records for 
> print_ten_doubles() and a nearby function that has no stub:
> 
>  <1><bab>: Abbrev Number: 16 (DW_TAG_subprogram)
>     <bac>   DW_AT_external    : 1
>     <bad>   DW_AT_name        : init_small_structs
>     <bc0>   DW_AT_decl_file   : 1
>     <bc1>   DW_AT_decl_line   : 613
>     <bc3>   DW_AT_low_pc      : 0x790
>     <bc7>   DW_AT_high_pc     : 0x900
>     <bcb>   DW_AT_frame_base  : 0x2db   (location list)
>     <bcf>   DW_AT_sibling     : <0xcaf>
> 
>  <1><caf>: Abbrev Number: 16 (DW_TAG_subprogram)
>     <cb0>   DW_AT_external    : 1
>     <cb1>   DW_AT_name        : print_ten_doubles
>     <cc3>   DW_AT_decl_file   : 1
>     <cc4>   DW_AT_decl_line   : 664
>     <cc6>   DW_AT_low_pc      : 0x900
>     <cca>   DW_AT_high_pc     : 0x97c
>     <cce>   DW_AT_frame_base  : 0x306   (location list)
>     <cd2>   DW_AT_sibling     : <0xd66>
> 
> And here are the relevant relocation records:
> 
> 00000bc3  00003c02 R_MIPS_32         00000790   .LFB23
> 00000bc7  00000202 R_MIPS_32         00000000   .text
> 
> 00000cc6  00000202 R_MIPS_32         00000000   .text
> 00000cca  00000202 R_MIPS_32         00000000   .text
> 
> Notice that the DWARF-2 record at 0xbc3 is relocated against .LFB23 and 
> one at 0xcc6 -- against .text, rather than .LFB20 as it should be.  I 
> presume this is because of the section switch happening inbetween.  Or 
> could it be because of ".set nomips16" actually preceding the section 
> switch?  Thiemo, can you perhaps make any comments about this?

Does the low bit end up set in the DWARF for init_small_structs?  From
what you're saying, I think it does.  My experience with Thumb makes
me think that's the actual GAS bug here.  Those are code addresses,
not symbols, and I don't think they should encode MIPS16-ness -
see the recent discussion of values in .debug_line, which we agreed
should not have assorted flag bits set either.  The code starts at
0x900, not 0x901.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: MIPS: Handle manual calls of MIPS16 functions with a call stub
  2008-02-01 16:58         ` Daniel Jacobowitz
@ 2008-02-01 17:07           ` Maciej W. Rozycki
  2008-02-01 17:15             ` Daniel Jacobowitz
  0 siblings, 1 reply; 21+ messages in thread
From: Maciej W. Rozycki @ 2008-02-01 17:07 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Thiemo Seufer, gdb-patches, Maciej W. Rozycki

On Fri, 1 Feb 2008, Daniel Jacobowitz wrote:

> Does the low bit end up set in the DWARF for init_small_structs?  From
> what you're saying, I think it does.  My experience with Thumb makes
> me think that's the actual GAS bug here.  Those are code addresses,
> not symbols, and I don't think they should encode MIPS16-ness -
> see the recent discussion of values in .debug_line, which we agreed
> should not have assorted flag bits set either.  The code starts at
> 0x900, not 0x901.

 It does.  Here is the final DWARF-2 record for init_small_structs():

 <1><ce2>: Abbrev Number: 16 (DW_TAG_subprogram)
  <ce3>     DW_AT_external    : 1
  <ce4>     DW_AT_name        : init_small_structs
  <cf7>     DW_AT_decl_file   : 1
  <cf8>     DW_AT_decl_line   : 613
  <cfa>     DW_AT_low_pc      : 0x8002093d
  <cfe>     DW_AT_high_pc     : 0x80020a90
  <d02>     DW_AT_frame_base  : 0x2db   (location list)
  <d06>     DW_AT_sibling     : <de6>

I cannot seem to locate the discussion you are referring to easily enough 
-- would you care to provide a link?

  Maciej


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: MIPS: Handle manual calls of MIPS16 functions with a call stub
  2008-02-01 17:07           ` Maciej W. Rozycki
@ 2008-02-01 17:15             ` Daniel Jacobowitz
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel Jacobowitz @ 2008-02-01 17:15 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: Thiemo Seufer, gdb-patches, Maciej W. Rozycki

On Fri, Feb 01, 2008 at 05:07:01PM +0000, Maciej W. Rozycki wrote:
> I cannot seem to locate the discussion you are referring to easily enough 
> -- would you care to provide a link?

Certainly: http://sourceware.org/ml/gdb-patches/2008-01/msg00532.html

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: MIPS: Handle manual calls of MIPS16 functions with a call stub
  2008-02-01 10:27   ` Maciej W. Rozycki
  2008-02-01 14:19     ` Daniel Jacobowitz
@ 2008-02-04 16:14     ` Maciej W. Rozycki
  2008-02-04 16:39       ` Daniel Jacobowitz
  1 sibling, 1 reply; 21+ messages in thread
From: Maciej W. Rozycki @ 2008-02-04 16:14 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches, Maciej W. Rozycki

On Fri, 1 Feb 2008, Maciej W. Rozycki wrote:

> > elsewhere)?  Maybe mips_write_pc should use mips_pc_is_mips16; that's
> > how Thumb works, by always consulting the minimal symbol table to find
> > out whether an address should be called as MIPS16 or MIPS32.
> 
>  The ABI is different, so it is not enough to set the PC correctly -- more 
> about it in the patch that is going through my regression testing at the 
> moment.  Though perhaps the other places could use mips_pc_is_mips16() 
> too.  On the other hand I feel setting the block's start address correctly 
> is the right way to make the handling consistent throughout -- by using 
> mips_pc_is_mips16() here and there some places may be omitted by accident.  
> Hmm...

 I have tried your suggestion of using mips_pc_is_mips16() in 
mips_write_pc(), but it only removes some of the regressions.  I think 
your proposal could work if the LSB of MIPS16 addresses was explicitly 
cleared throughout processing within GDB as all the DWARF-2 records 
referring to such addresses are currently meant to have it set.  This is 
obviously because the R_MIPS_32 relocation is used for them as well as 
where an address of a function is taken for the purpose of making a call, 
so all these places are treated the same by the linker and GDB is prepared 
to it.

 Obviously the problem in the first place is in the way the stub is 
annotated with debugging information.  Moving it outside the block 
describing the associated actual function breaks backtracing and 
single-stepping quite badly if I recall correctly.  The DWARF-3 spec 
actually provides for such special code fragments by the means of the 
DW_AT_trampoline attribute, but GCC does not really have any infrastruture 
for handling it yet, except from recognising the name of the attribute.  
Until that is adopted I think we should have means to handle stubs 
regardless and even once GCC has been updated I think older binaries 
should be kept supported by reasonable means.

 Please note that for MIPS16 we have return stubs too. ;-)

  Maciej


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: MIPS: Handle manual calls of MIPS16 functions with a call stub
  2008-02-04 16:14     ` Maciej W. Rozycki
@ 2008-02-04 16:39       ` Daniel Jacobowitz
  2008-02-08 14:23         ` Maciej W. Rozycki
  0 siblings, 1 reply; 21+ messages in thread
From: Daniel Jacobowitz @ 2008-02-04 16:39 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: gdb-patches, Maciej W. Rozycki

On Mon, Feb 04, 2008 at 04:13:48PM +0000, Maciej W. Rozycki wrote:
>  I have tried your suggestion of using mips_pc_is_mips16() in 
> mips_write_pc(), but it only removes some of the regressions.  I think 
> your proposal could work if the LSB of MIPS16 addresses was explicitly 
> cleared throughout processing within GDB as all the DWARF-2 records 
> referring to such addresses are currently meant to have it set.  This is 
> obviously because the R_MIPS_32 relocation is used for them as well as 
> where an address of a function is taken for the purpose of making a call, 
> so all these places are treated the same by the linker and GDB is prepared 
> to it.

This is not how any other target that I know of handles extra PC bits.
I think it's going to cause a whole lot of other similar problems.  I
am reluctant to add new hooks to support storing additional bits in
addresses, when at the same time we have hooks that other targets use
- called from overlapping places - to remove extra bits from addresses.

Honestly, I'm not just trying to be difficult.  But having two targets
solve the same problem in opposite ways makes the core of GDB a mess.
"Do line table entries include extra non-address bits" is not a
question that should have different answers on different targets.
Similarly for function block addresses.

I think that using mips_pc_is_mips16 can be made to work, by analogy
to ARM.  I'd look at this myself, but I don't think I'm set up to run
mips16 tests (yet).  Should I be able to do this with just the GDB
simulator and a board file?

>  Obviously the problem in the first place is in the way the stub is 
> annotated with debugging information.  Moving it outside the block 
> describing the associated actual function breaks backtracing and 
> single-stepping quite badly if I recall correctly.

I don't understand.  The stub is not annotated with debug information
in the example you posted earlier in the thread.  It's only "inside
the block" physically in the assembly file and for the purposes of
confusing gas (it probably puts the symbol and first instruction in
different frags, the first of which is zero length, breaking whatever
gas uses to annotate the symbol value).  It's not covered by the range
[.LFB20, .LEB20] because those labels are in the text section.

> The DWARF-3 spec actually provides for such special code fragments
> by the means of the DW_AT_trampoline attribute, but GCC does not
> really have any infrastruture for handling it yet, except from
> recognising the name of the attribute.  Until that is adopted I
> think we should have means to handle stubs regardless and even once
> GCC has been updated I think older binaries should be kept supported
> by reasonable means.

I'm not suggesting that we shouldn't.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: MIPS: Handle manual calls of MIPS16 functions with a call stub
  2008-02-04 16:39       ` Daniel Jacobowitz
@ 2008-02-08 14:23         ` Maciej W. Rozycki
  2008-02-08 14:57           ` Daniel Jacobowitz
                             ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Maciej W. Rozycki @ 2008-02-08 14:23 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches, Maciej W. Rozycki

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3418 bytes --]

On Mon, 4 Feb 2008, Daniel Jacobowitz wrote:

> This is not how any other target that I know of handles extra PC bits.
> I think it's going to cause a whole lot of other similar problems.  I
> am reluctant to add new hooks to support storing additional bits in
> addresses, when at the same time we have hooks that other targets use
> - called from overlapping places - to remove extra bits from addresses.

 Well, it is certainly the smallest change that fits the way the MIPS 
target is currently handled throughout the toolchain.  Not necessarily the 
best.

> Honestly, I'm not just trying to be difficult.  But having two targets

 Of course -- I would not have doubted.  I have been around for long 
enough.

> solve the same problem in opposite ways makes the core of GDB a mess.
> "Do line table entries include extra non-address bits" is not a
> question that should have different answers on different targets.
> Similarly for function block addresses.

 Your proposal sounds clean enough for me to investigate it further.  In 
fact I have done so to some extent already.

> I think that using mips_pc_is_mips16 can be made to work, by analogy
> to ARM.  I'd look at this myself, but I don't think I'm set up to run

 It is much more than that, but I think it can be done with some 
adjustments to pointer_to_address(), address_to_pointer() and 
integer_to_address() methods.  If DWARF-2 records could be treated as 
pointers (which they are given how the linker processes them) rather than 
addresses then such a setup should work.  That should be done above the 
level of the DWARF-2 interpreter, as losing the LSB from relative data 
often contained in records would result in an accumulative error.

 Of course further adjustments might be needed to methods like read_pc(), 
write_pc() and unwind_pc() (I am not absolutely sure at this point yet) 
and perhaps elsewhere.  Just as a proof of concept, with some hacks to the 
DWARF-2 parser and elsewhere, including but not limited to functions 
mentioned, I got down to just three regressions compared to results with 
my proposal.  Now the question is whether a similar result may be achieved 
using properly architected code.  I'll have a look at it soon.

> mips16 tests (yet).  Should I be able to do this with just the GDB
> simulator and a board file?

 I have attached the "mips-sim-sde32" board description file I use and the 
necessary linker script.  You should be able to use it, though there may 
be pitfalls.  When running tests you need -Wa,-O0 to disable branch 
swapping as it makes MIPS16 code inconsistent with DWARF-2 information in 
a fatal way.

> I don't understand.  The stub is not annotated with debug information
> in the example you posted earlier in the thread.  It's only "inside
> the block" physically in the assembly file and for the purposes of
> confusing gas (it probably puts the symbol and first instruction in
> different frags, the first of which is zero length, breaking whatever
> gas uses to annotate the symbol value).  It's not covered by the range
> [.LFB20, .LEB20] because those labels are in the text section.

 It is still covered by the .loc directive and therefore recognised to be 
a part of the code corresponding to the first line of the function.  It 
makes single-stepping through it possible -- including correct frame 
discovery as required by `nexti'/`step'/`next' (not `stepi' though).

  Maciej

[-- Attachment #2: Type: TEXT/PLAIN, Size: 10844 bytes --]

2005-03-17  Nigel Stephens  <nigel@mips.com>

	[mti-libgloss-linkonce]
	* mips/mdi32.ld: Append appending final '.' to .gnu.linkonce.x
	input section names, and add missing .gnu.linkonce.b. section.
	* mips/mdi64.ld: Similarly. 
	* mips/sde32.ld: Similarly. 
	* mips/sde64.ld: Similarly. 

2004-11-26  Nigel Stephens  <nigel@mips.com>

	* mips/sde32.ld: New linker script for SDE newlib configurations,
	which uses elf32-trad*mips.

	* mips/sde64.ld: Change object formats from elf32-n*mips to elf32-ntrad*mips.

	* mips/mdi32.ld: Change object formats from elf32-*mips to elf32-trad*mips.
	
	* mips/mdi64.ld: Change object formats from elf32-n*mips to elf32-ntrad*mips.

	* mips/configure.in (mips*-sde-elf*): Change "idt32" in script_list
	to "sde32". 

	* mips/configure: Rebuilt.

2004-10-04  Nigel Stephens  <nigel@mips.com>

	* mips/idt32.ld (.sdeosabi): Added this new section to the script,
	so that we can tell gdb that whether this program uses "SDE"
	exception handling or not. 
	* mips/sde64.ld: Similarly.

2004-08-24  Nigel Stephens  <nigel@mips.com>

	* mips/mdi32.ld: MDI/MIPSsim link script for 32-bit ABIs.
	* mips/mdi64.ld: MDI/MIPSsim link script for N32 ABI.


Index: binutils/src/libgloss/mips/sde32.ld
===================================================================
--- /dev/null
+++ binutils/src/libgloss/mips/sde32.ld
@@ -0,0 +1,175 @@
+/* The following TEXT start address leaves space for the monitor
+   workspace.  This linker script links isa32 programs for use with the
+   simulator.  */
+
+ENTRY(_start)
+OUTPUT_ARCH("mips:isa32")
+OUTPUT_FORMAT("elf32-tradlittlemips", "elf32-tradbigmips", "elf32-tradlittlemips")
+GROUP(-lc -lidt -lgcc)
+SEARCH_DIR(.)
+__DYNAMIC  =  0;
+STARTUP(crt0.o)
+
+/*
+ * Allocate the stack to be at the top of memory, since the stack
+ * grows down
+ */
+PROVIDE (__stack = 0);
+/* PROVIDE (__global = 0); */
+
+/*
+ * Initalize some symbols to be zero so we can reference them in the
+ * crt0 without core dumping. These functions are all optional, but
+ * we do this so we can have our crt0 always use them if they exist.
+ * This is so BSPs work better when using the crt0 installed with gcc.
+ * We have to initalize them twice, so we multiple object file
+ * formats, as some prepend an underscore.
+ */
+PROVIDE (hardware_exit_hook = 0);
+PROVIDE (hardware_hazard_hook = 0);
+PROVIDE (hardware_init_hook = 0);
+PROVIDE (software_init_hook = 0);
+
+SECTIONS
+{
+  . = 0x80020000;
+  .text : {
+     _ftext = . ;
+    PROVIDE (eprol  =  .);
+    *(.text)
+    *(.text.*)
+    *(.gnu.linkonce.t.*)
+    *(.mips16.fn.*)
+    *(.mips16.call.*)
+  }
+  .init : {
+    KEEP(*(.init))
+  }
+  .fini : {
+    KEEP(*(.fini))
+  }
+  .rel.sdata : {
+    PROVIDE (__runtime_reloc_start = .);
+    *(.rel.sdata)
+    PROVIDE (__runtime_reloc_stop = .);
+  }
+  PROVIDE (etext  =  .);
+  _etext  =  .;
+
+  .eh_frame_hdr : { *(.eh_frame_hdr) }
+  .eh_frame : { KEEP (*(.eh_frame)) }
+  .gcc_except_table : { *(.gcc_except_table) }
+  .jcr : { KEEP (*(.jcr)) }
+
+  .ctors    :
+  {
+    /* gcc uses crtbegin.o to find the start of
+       the constructors, so we make sure it is
+       first.  Because this is a wildcard, it
+       doesn't matter if the user does not
+       actually link against crtbegin.o; the
+       linker won't look for a file to match a
+       wildcard.  The wildcard also means that it
+       doesn't matter which directory crtbegin.o
+       is in.  */
+
+    KEEP (*crtbegin.o(.ctors))
+
+    /* We don't want to include the .ctor section from
+       from the crtend.o file until after the sorted ctors.
+       The .ctor section from the crtend file contains the
+       end of ctors marker and it must be last */
+
+    KEEP (*(EXCLUDE_FILE (*crtend.o) .ctors))
+    KEEP (*(SORT(.ctors.*)))
+    KEEP (*(.ctors))
+  }
+
+  .dtors    :
+  {
+    KEEP (*crtbegin.o(.dtors))
+    KEEP (*(EXCLUDE_FILE (*crtend.o) .dtors))
+    KEEP (*(SORT(.dtors.*)))
+    KEEP (*(.dtors))
+  }
+
+  . = .;
+  .rodata : {
+    *(.rdata)
+    *(.rodata)
+    *(.rodata.*)
+    *(.gnu.linkonce.r.*)
+  }
+  .sdeosabi : {
+     /* SDE OSABI indication for GDB */
+     *(.sdeosabi)
+  }
+   _fdata = ALIGN(16);
+  .data : {
+    *(.data)
+    *(.data.*)
+    *(.data1)
+    *(.gnu.linkonce.d.*)
+  }
+  . = ALIGN(8);
+  _gp = . + 0x8000;
+  __global = _gp;
+  .lit8 : {
+    *(.lit8)
+  }
+  .lit4 : {
+    *(.lit4)
+  }
+  .sdata : {
+    *(.sdata)
+    *(.sdata.*)
+    *(.gnu.linkonce.s.*)
+  }
+  . = ALIGN(4);
+  PROVIDE (edata  =  .);
+  _edata  =  .;
+  _fbss = .;
+  .sbss : {
+    *(.sbss .sbss.* .gnu.linkonce.sb.*)
+    *(.scommon)
+  }
+  .bss : {
+    _bss_start = . ;
+    *(.bss .bss.* .gnu.linkonce.b.*)
+    *(COMMON)
+  }
+
+   PROVIDE (end = .);
+   _end = .;
+
+  /* DWARF debug sections.
+     Symbols in the DWARF debugging sections are relative to
+     the beginning of the section so we begin them at 0.  */
+
+  /* DWARF 1 */
+  .debug          0 : { *(.debug) }
+  .line           0 : { *(.line) }
+
+  /* GNU DWARF 1 extensions */
+  .debug_srcinfo  0 : { *(.debug_srcinfo) }
+  .debug_sfnames  0 : { *(.debug_sfnames) }
+
+  /* DWARF 1.1 and DWARF 2 */
+  .debug_aranges  0 : { *(.debug_aranges) }
+  .debug_pubnames 0 : { *(.debug_pubnames) }
+
+  /* DWARF 2 */
+  .debug_info     0 : { *(.debug_info .gnu.linkonce.wi.*) }
+  .debug_abbrev   0 : { *(.debug_abbrev) }
+  .debug_line     0 : { *(.debug_line) }
+  .debug_frame    0 : { *(.debug_frame) }
+  .debug_str      0 : { *(.debug_str) }
+  .debug_loc      0 : { *(.debug_loc) }
+  .debug_macinfo  0 : { *(.debug_macinfo) }
+
+  /* SGI/MIPS DWARF 2 extensions */
+  .debug_weaknames 0 : { *(.debug_weaknames) }
+  .debug_funcnames 0 : { *(.debug_funcnames) }
+  .debug_typenames 0 : { *(.debug_typenames) }
+  .debug_varnames  0 : { *(.debug_varnames) }
+}
Index: binutils/src/libgloss/mips/sde64.ld
===================================================================
--- /dev/null
+++ binutils/src/libgloss/mips/sde64.ld
@@ -0,0 +1,176 @@
+/* The following TEXT start address leaves space for the monitor
+   workspace.  This linker script links isa32 programs for use with the
+   simulator.  */
+
+ENTRY(_start)
+OUTPUT_ARCH("mips:isa64")
+OUTPUT_FORMAT("elf32-ntradlittlemips", "elf32-ntradbigmips", "elf32-ntradlittlemips")
+GROUP(-lc -lidt -lgcc)
+SEARCH_DIR(.)
+__DYNAMIC  =  0;
+STARTUP(crt0.o)
+
+/*
+ * Allocate the stack to be at the top of memory, since the stack
+ * grows down
+ */
+PROVIDE (__stack = 0);
+/* PROVIDE (__global = 0); */
+
+/*
+ * Initalize some symbols to be zero so we can reference them in the
+ * crt0 without core dumping. These functions are all optional, but
+ * we do this so we can have our crt0 always use them if they exist.
+ * This is so BSPs work better when using the crt0 installed with gcc.
+ * We have to initalize them twice, so we multiple object file
+ * formats, as some prepend an underscore.
+ */
+PROVIDE (hardware_exit_hook = 0);
+PROVIDE (hardware_hazard_hook = 0);
+PROVIDE (hardware_init_hook = 0);
+PROVIDE (software_init_hook = 0);
+
+SECTIONS
+{
+  . = 0x80020000;
+  .text : {
+     _ftext = . ;
+    PROVIDE (eprol  =  .);
+    *(.text)
+    *(.text.*)
+    *(.gnu.linkonce.t.*)
+    *(.mips16.fn.*)
+    *(.mips16.call.*)
+  }
+  .init : {
+    KEEP(*(.init))
+  }
+  .fini : {
+    KEEP(*(.fini))
+  }
+  .rel.sdata : {
+    PROVIDE (__runtime_reloc_start = .);
+    *(.rel.sdata)
+    PROVIDE (__runtime_reloc_stop = .);
+  }
+  PROVIDE (etext  =  .);
+  _etext  =  .;
+
+
+  .eh_frame_hdr : { *(.eh_frame_hdr) }
+  .eh_frame : { KEEP (*(.eh_frame)) }
+  .gcc_except_table : { *(.gcc_except_table) }
+  .jcr : { KEEP (*(.jcr)) }
+
+  .ctors    :
+  {
+    /* gcc uses crtbegin.o to find the start of
+       the constructors, so we make sure it is
+       first.  Because this is a wildcard, it
+       doesn't matter if the user does not
+       actually link against crtbegin.o; the
+       linker won't look for a file to match a
+       wildcard.  The wildcard also means that it
+       doesn't matter which directory crtbegin.o
+       is in.  */
+
+    KEEP (*crtbegin.o(.ctors))
+
+    /* We don't want to include the .ctor section from
+       from the crtend.o file until after the sorted ctors.
+       The .ctor section from the crtend file contains the
+       end of ctors marker and it must be last */
+
+    KEEP (*(EXCLUDE_FILE (*crtend.o) .ctors))
+    KEEP (*(SORT(.ctors.*)))
+    KEEP (*(.ctors))
+  }
+
+  .dtors    :
+  {
+    KEEP (*crtbegin.o(.dtors))
+    KEEP (*(EXCLUDE_FILE (*crtend.o) .dtors))
+    KEEP (*(SORT(.dtors.*)))
+    KEEP (*(.dtors))
+  }
+
+  . = .;
+  .rodata : {
+    *(.rdata)
+    *(.rodata)
+    *(.rodata.*)
+    *(.gnu.linkonce.r.*)
+  }
+  .sdeosabi : {
+     /* SDE OSABI indication for GDB */
+     *(.sdeosabi)
+  }
+   _fdata = ALIGN(16);
+  .data : {
+    *(.data)
+    *(.data.*)
+    *(.data1)
+    *(.gnu.linkonce.d.*)
+  }
+  . = ALIGN(8);
+  _gp = . + 0x8000;
+  __global = _gp;
+  .lit8 : {
+    *(.lit8)
+  }
+  .lit4 : {
+    *(.lit4)
+  }
+  .sdata : {
+    *(.sdata)
+    *(.sdata.*)
+    *(.gnu.linkonce.s.*)
+  }
+  . = ALIGN(4);
+  PROVIDE (edata  =  .);
+  _edata  =  .;
+  _fbss = .;
+  .sbss : {
+    *(.sbss .sbss.* .gnu.linkonce.sb.*)
+    *(.scommon)
+  }
+  .bss : {
+    _bss_start = . ;
+    *(.bss .bss.* .gnu.linkonce.b.*)
+    *(COMMON)
+  }
+
+  PROVIDE (end = .);
+  _end = .;
+
+  /* DWARF debug sections.
+     Symbols in the DWARF debugging sections are relative to
+     the beginning of the section so we begin them at 0.  */
+
+  /* DWARF 1 */
+  .debug          0 : { *(.debug) }
+  .line           0 : { *(.line) }
+
+  /* GNU DWARF 1 extensions */
+  .debug_srcinfo  0 : { *(.debug_srcinfo) }
+  .debug_sfnames  0 : { *(.debug_sfnames) }
+
+  /* DWARF 1.1 and DWARF 2 */
+  .debug_aranges  0 : { *(.debug_aranges) }
+  .debug_pubnames 0 : { *(.debug_pubnames) }
+
+  /* DWARF 2 */
+  .debug_info     0 : { *(.debug_info .gnu.linkonce.wi.*) }
+  .debug_abbrev   0 : { *(.debug_abbrev) }
+  .debug_line     0 : { *(.debug_line) }
+  .debug_frame    0 : { *(.debug_frame) }
+  .debug_str      0 : { *(.debug_str) }
+  .debug_loc      0 : { *(.debug_loc) }
+  .debug_macinfo  0 : { *(.debug_macinfo) }
+
+  /* SGI/MIPS DWARF 2 extensions */
+  .debug_weaknames 0 : { *(.debug_weaknames) }
+  .debug_funcnames 0 : { *(.debug_funcnames) }
+  .debug_typenames 0 : { *(.debug_typenames) }
+  .debug_varnames  0 : { *(.debug_varnames) }
+}

[-- Attachment #3: Type: TEXT/PLAIN, Size: 3047 bytes --]

Index: dejagnu-quilt/dejagnu/baseboards/mips-sim-sde32.exp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ dejagnu-quilt/dejagnu/baseboards/mips-sim-sde32.exp	2007-05-24 16:25:43.000000000 +0100
@@ -0,0 +1,60 @@
+# Load the generic configuration for this board. This will define a basic
+# set of routines needed by the tool to communicate with the board.
+load_generic_config "sim";
+
+# basic-sim.exp is a basic description for the standard Cygnus simulator.
+load_base_board_description "basic-sim";
+
+# This tells it which directory to look in for the simulator.
+setup_sim mips;
+
+# No multilib flags are set by default.
+process_multilib_options "";
+
+# The compiler used to build for this board. This has *nothing* to do
+# with what compiler is tested if we're testing gcc.
+set_board_info compiler "[find_gcc]";
+#set_board_info needs_status_wrapper 1
+set_board_info gdb,nofileio 1
+
+set_board_info cflags "[libgloss_include_flags] [newlib_include_flags]";
+set_board_info ldflags "[libgloss_link_flags] [newlib_link_flags]";
+set_board_info ldscript "-Tsde32.ld";
+
+# And, it can't do arguments, and doesn't have real signals.
+set_board_info noargs 1;
+set_board_info gdb,nosignals 1;
+
+# For some big C++ benchmarks...
+set_board_info sim,options "--memory-size=0x1000000"
+
+# Skip huge test. The MIPS simulator reports only 
+# 2MB of memory.
+set_board_info gdb,skip_huge_test 1
+
+# Tell gdb to assume no fpu for -msoft-float compilation
+if {[string match "*soft-float*" [board_info $board multilib_flags]]} {
+  add_board_info gdb_load_commands "set mipsfpu none"
+}
+
+# Tell gdb to assume no fpu for -msoft-float compilation
+if {[string match "*mno-data-in-code*" [board_info $board multilib_flags]]} {
+  add_board_info sim,options "--spram=on"
+}
+
+#
+# Kludge g++_link_flags routine to explicitly add -lstdc++ to link flags
+# so that we can make sure the it comes *before* newlib and the libgloss
+# target libraries 
+#
+proc msim_g++_link_flags { args } {
+  set flags [ msim_orig_g++_link_flags "$args" ]
+  append flags "-lstdc++ "
+  return "$flags"
+}
+
+if { [info procs msim_orig_g++_link_flags ] == "" } {
+  verbose "installing msim_g++_link_flags" 1;
+  rename g++_link_flags msim_orig_g++_link_flags
+  rename msim_g++_link_flags g++_link_flags
+}
Index: dejagnu-quilt/dejagnu/lib/targetdb.exp
===================================================================
--- dejagnu-quilt.orig/dejagnu/lib/targetdb.exp	2007-05-24 16:25:04.000000000 +0100
+++ dejagnu-quilt/dejagnu/lib/targetdb.exp	2007-05-24 16:25:12.000000000 +0100
@@ -73,6 +73,15 @@
     }
 }
 
+#
+# Add VALUE to ENTRY for the current board being defined.
+#
+proc add_board_info { entry value } {
+    global board_info board;
+
+    lappend board_info($board,$entry) $value;
+}
+
 # Fill in ENTRY with VALUE for the current target.
 #
 proc set_currtarget_info { entry value } {

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: MIPS: Handle manual calls of MIPS16 functions with a call stub
  2008-02-08 14:23         ` Maciej W. Rozycki
@ 2008-02-08 14:57           ` Daniel Jacobowitz
  2008-02-08 18:06           ` Jim Blandy
  2008-02-08 18:08           ` Jim Blandy
  2 siblings, 0 replies; 21+ messages in thread
From: Daniel Jacobowitz @ 2008-02-08 14:57 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: gdb-patches, Maciej W. Rozycki

On Fri, Feb 08, 2008 at 02:23:03PM +0000, Maciej W. Rozycki wrote:
> > I think that using mips_pc_is_mips16 can be made to work, by analogy
> > to ARM.  I'd look at this myself, but I don't think I'm set up to run
> 
>  It is much more than that, but I think it can be done with some 
> adjustments to pointer_to_address(), address_to_pointer() and 
> integer_to_address() methods.  If DWARF-2 records could be treated as 
> pointers (which they are given how the linker processes them) rather than 
> addresses then such a setup should work.  That should be done above the 
> level of the DWARF-2 interpreter, as losing the LSB from relative data 
> often contained in records would result in an accumulative error.

Hmm.  This sounds believable, but it may react badly with other
platforms.  We'll have to experiment.

> > mips16 tests (yet).  Should I be able to do this with just the GDB
> > simulator and a board file?
> 
>  I have attached the "mips-sim-sde32" board description file I use and the 
> necessary linker script.  You should be able to use it, though there may 
> be pitfalls.  When running tests you need -Wa,-O0 to disable branch 
> swapping as it makes MIPS16 code inconsistent with DWARF-2 information in 
> a fatal way.

Thanks.

> > I don't understand.  The stub is not annotated with debug information
> > in the example you posted earlier in the thread.  It's only "inside
> > the block" physically in the assembly file and for the purposes of
> > confusing gas (it probably puts the symbol and first instruction in
> > different frags, the first of which is zero length, breaking whatever
> > gas uses to annotate the symbol value).  It's not covered by the range
> > [.LFB20, .LEB20] because those labels are in the text section.
> 
>  It is still covered by the .loc directive and therefore recognised to be 
> a part of the code corresponding to the first line of the function.  It 
> makes single-stepping through it possible -- including correct frame 
> discovery as required by `nexti'/`step'/`next' (not `stepi' though).

This makes more sense, but not quite... I see how the .loc covers it.
That should get it into .debug_line.  However it shouldn't affect the
symbol table, or the frame unwinders... I must be missing something,
but I'll figure it out eventually.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: MIPS: Handle manual calls of MIPS16 functions with a call stub
  2008-02-08 14:23         ` Maciej W. Rozycki
  2008-02-08 14:57           ` Daniel Jacobowitz
@ 2008-02-08 18:06           ` Jim Blandy
  2008-02-08 18:08           ` Jim Blandy
  2 siblings, 0 replies; 21+ messages in thread
From: Jim Blandy @ 2008-02-08 18:06 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: Daniel Jacobowitz, gdb-patches, Maciej W. Rozycki

On Feb 8, 2008 6:23 AM, Maciej W. Rozycki <macro@mips.com> wrote:
> On Mon, 4 Feb 2008, Daniel Jacobowitz wrote:
>
> > This is not how any other target that I know of handles extra PC bits.
> > I think it's going to cause a whole lot of other similar problems.  I
> > am reluctant to add new hooks to support storing additional bits in
> > addresses, when at the same time we have hooks that other targets use
> > - called from overlapping places - to remove extra bits from addresses.
>
>  Well, it is certainly the smallest change that fits the way the MIPS
> target is currently handled throughout the toolchain.  Not necessarily the
> best.
>
> > Honestly, I'm not just trying to be difficult.  But having two targets
>
>  Of course -- I would not have doubted.  I have been around for long
> enough.
>
> > solve the same problem in opposite ways makes the core of GDB a mess.
> > "Do line table entries include extra non-address bits" is not a
> > question that should have different answers on different targets.
> > Similarly for function block addresses.
>
>  Your proposal sounds clean enough for me to investigate it further.  In
> fact I have done so to some extent already.
>
> > I think that using mips_pc_is_mips16 can be made to work, by analogy
> > to ARM.  I'd look at this myself, but I don't think I'm set up to run
>
>  It is much more than that, but I think it can be done with some
> adjustments to pointer_to_address(), address_to_pointer() and
> integer_to_address() methods.  If DWARF-2 records could be treated as
> pointers (which they are given how the linker processes them) rather than
> addresses then such a setup should work.  That should be done above the
> level of the DWARF-2 interpreter, as losing the LSB from relative data
> often contained in records would result in an accumulative error.
>
>  Of course further adjustments might be needed to methods like read_pc(),
> write_pc() and unwind_pc() (I am not absolutely sure at this point yet)
> and perhaps elsewhere.  Just as a proof of concept, with some hacks to the
> DWARF-2 parser and elsewhere, including but not limited to functions
> mentioned, I got down to just three regressions compared to results with
> my proposal.  Now the question is whether a similar result may be achieved
> using properly architected code.  I'll have a look at it soon.
>
> > mips16 tests (yet).  Should I be able to do this with just the GDB
> > simulator and a board file?
>
>  I have attached the "mips-sim-sde32" board description file I use and the
> necessary linker script.  You should be able to use it, though there may
> be pitfalls.  When running tests you need -Wa,-O0 to disable branch
> swapping as it makes MIPS16 code inconsistent with DWARF-2 information in
> a fatal way.
>
> > I don't understand.  The stub is not annotated with debug information
> > in the example you posted earlier in the thread.  It's only "inside
> > the block" physically in the assembly file and for the purposes of
> > confusing gas (it probably puts the symbol and first instruction in
> > different frags, the first of which is zero length, breaking whatever
> > gas uses to annotate the symbol value).  It's not covered by the range
> > [.LFB20, .LEB20] because those labels are in the text section.
>
>  It is still covered by the .loc directive and therefore recognised to be
> a part of the code corresponding to the first line of the function.  It
> makes single-stepping through it possible -- including correct frame
> discovery as required by `nexti'/`step'/`next' (not `stepi' though).
>
>   Maciej


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: MIPS: Handle manual calls of MIPS16 functions with a call stub
  2008-02-08 14:23         ` Maciej W. Rozycki
  2008-02-08 14:57           ` Daniel Jacobowitz
  2008-02-08 18:06           ` Jim Blandy
@ 2008-02-08 18:08           ` Jim Blandy
  2008-02-13 18:28             ` Maciej W. Rozycki
  2 siblings, 1 reply; 21+ messages in thread
From: Jim Blandy @ 2008-02-08 18:08 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: Daniel Jacobowitz, gdb-patches, Maciej W. Rozycki

On Feb 8, 2008 6:23 AM, Maciej W. Rozycki <macro@mips.com> wrote:
>  It is much more than that, but I think it can be done with some
> adjustments to pointer_to_address(), address_to_pointer() and
> integer_to_address() methods.  If DWARF-2 records could be treated as
> pointers (which they are given how the linker processes them) rather than
> addresses then such a setup should work.  That should be done above the
> level of the DWARF-2 interpreter, as losing the LSB from relative data
> often contained in records would result in an accumulative error.

[that was weird; sorry]

If you're suggesting that we run the values in DWARF data through
pointer_to_address, I don't think that's right, either.  DWARF
specifies that those attributes' values are byte addresses of code.
Putting ISA information in low bits of DWARF attribute values isn't
the way we've decided to do things.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: MIPS: Handle manual calls of MIPS16 functions with a call stub
  2008-02-08 18:08           ` Jim Blandy
@ 2008-02-13 18:28             ` Maciej W. Rozycki
  2008-02-13 20:54               ` Jim Blandy
  0 siblings, 1 reply; 21+ messages in thread
From: Maciej W. Rozycki @ 2008-02-13 18:28 UTC (permalink / raw)
  To: Jim Blandy; +Cc: Daniel Jacobowitz, gdb-patches, Maciej W. Rozycki

On Fri, 8 Feb 2008, Jim Blandy wrote:

> On Feb 8, 2008 6:23 AM, Maciej W. Rozycki <macro@mips.com> wrote:
> >  It is much more than that, but I think it can be done with some
> > adjustments to pointer_to_address(), address_to_pointer() and
> > integer_to_address() methods.  If DWARF-2 records could be treated as
> > pointers (which they are given how the linker processes them) rather than
> > addresses then such a setup should work.  That should be done above the
> > level of the DWARF-2 interpreter, as losing the LSB from relative data
> > often contained in records would result in an accumulative error.
> 
> If you're suggesting that we run the values in DWARF data through
> pointer_to_address, I don't think that's right, either.  DWARF
> specifies that those attributes' values are byte addresses of code.

 You are right indeed -- this is what the spec says:

"address

   Represented as an object of appropriate size to hold an address on the 
   target machine (DW_FORM_addr).  The size is encoded in the compilation 
   unit header (see Section 7.5.1).  This address is relocatable in a 
   relocatable object file and is relocated inan executable file or shared 
   object."

> Putting ISA information in low bits of DWARF attribute values isn't
> the way we've decided to do things.

 Unfortunately this is what the current versions of the relevant tools do 
and I think it has been like this for a while.  The linker does not 
differentiate between relocations used for taking an address of a function 
for the purpose of making a call and for other uses and the same 
relocation types are used in both cases.

 Ultimately it is the linker that should be fixed (though from the current 
behaviour I think GAS has a problem here as well), but I am afraid broken 
tools and broken binaries are going to be out there for a while yet, so it 
may be a reasonable idea to try to handle them as well as possible.  I 
recognise pointer_to_address() may not be the best choice here though.

 However I think it will have to be involved anyway elsewhere as I find 
the current model assumed by GDB inconsistent.

  Maciej


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: MIPS: Handle manual calls of MIPS16 functions with a call stub
  2008-02-13 18:28             ` Maciej W. Rozycki
@ 2008-02-13 20:54               ` Jim Blandy
  2008-02-15 11:36                 ` Maciej W. Rozycki
  0 siblings, 1 reply; 21+ messages in thread
From: Jim Blandy @ 2008-02-13 20:54 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: Daniel Jacobowitz, gdb-patches, Maciej W. Rozycki

On Feb 13, 2008 10:27 AM, Maciej W. Rozycki <macro@mips.com> wrote:
> On Fri, 8 Feb 2008, Jim Blandy wrote:
> > Putting ISA information in low bits of DWARF attribute values isn't
> > the way we've decided to do things.
>
>  Unfortunately this is what the current versions of the relevant tools do
> and I think it has been like this for a while.  The linker does not
> differentiate between relocations used for taking an address of a function
> for the purpose of making a call and for other uses and the same
> relocation types are used in both cases.
>
>  Ultimately it is the linker that should be fixed (though from the current
> behaviour I think GAS has a problem here as well), but I am afraid broken
> tools and broken binaries are going to be out there for a while yet, so it
> may be a reasonable idea to try to handle them as well as possible.  I
> recognise pointer_to_address() may not be the best choice here though.
>
>  However I think it will have to be involved anyway elsewhere as I find
> the current model assumed by GDB inconsistent.

It seems that we all agree on the following:

- It's contrary to the DWARF spec for producers to put arch-specific
  information in the low bits of the addresses in the line number
  table, function and block address ranges, and so on.  Existing
  toolchains that do this are buggy, but that's life.

- The addresses in GDB's line tables and block ranges should not have
  such extra bits set in them.

- The proper place to store arch-specific data on minimal symbols is
  in the 'info' field of 'struct minimal_symbol'.  In cases like
  MIPS16, the gdbarch_push_dummy_call method can consult that
  information at call time to see which calling convention is
  appropriate.

If we agree that we want to work around the linker bugs in GDB's
reader, that means we have to clear those bits and stash the
information elsewhere when we read the DWARF.  So something like
Maciej's original patch doesn't seem wrong to me.

The name 'make_symbol_special' seems awfully vague, though.  Is the
term 'special' inherited from outside GDB, or did it originate within
GDB?  In either case, I don't want it propagating into the DWARF
reader; that's horrible.  :)


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: MIPS: Handle manual calls of MIPS16 functions with a call stub
  2008-02-13 20:54               ` Jim Blandy
@ 2008-02-15 11:36                 ` Maciej W. Rozycki
  2008-02-18 13:32                   ` Nigel Stephens
  0 siblings, 1 reply; 21+ messages in thread
From: Maciej W. Rozycki @ 2008-02-15 11:36 UTC (permalink / raw)
  To: Jim Blandy
  Cc: Daniel Jacobowitz, Nigel Stephens, gdb-patches, Maciej W. Rozycki

On Wed, 13 Feb 2008, Jim Blandy wrote:

> It seems that we all agree on the following:
> 
> - It's contrary to the DWARF spec for producers to put arch-specific
>   information in the low bits of the addresses in the line number
>   table, function and block address ranges, and so on.  Existing
>   toolchains that do this are buggy, but that's life.
> 
> - The addresses in GDB's line tables and block ranges should not have
>   such extra bits set in them.
> 
> - The proper place to store arch-specific data on minimal symbols is
>   in the 'info' field of 'struct minimal_symbol'.  In cases like
>   MIPS16, the gdbarch_push_dummy_call method can consult that
>   information at call time to see which calling convention is
>   appropriate.

 I certainly agree here.

> If we agree that we want to work around the linker bugs in GDB's
> reader, that means we have to clear those bits and stash the
> information elsewhere when we read the DWARF.  So something like
> Maciej's original patch doesn't seem wrong to me.

 OK, but the original patch does not clear the bit, but actually it sets 
it in the single case discovered so far where GAS gets it wrong (and 
linker does not fix up in any way).  What I attempted meanwhile was to 
adjust the DWARF reader so that it effectively ignores the bit.  It proved 
not as straightforward as it could immediately be thought it would be as 
with some structures addresses are calculated cumulatively and one or more 
addends may be odd.  It is therefore necessary, where applicable, not only 
to mask out the LSB, but also to remember its value and carry it forward 
to the next addition.

 What I have developed is a crude hack as a proof of concept which just 
does all the extra calculation unconditionally so that I could verify it 
was at all workable as well as identify all the places that would have to 
be changed and how.  If there is agreement to push this approach forward I 
will have a look at how to do this properly.

> The name 'make_symbol_special' seems awfully vague, though.  Is the
> term 'special' inherited from outside GDB, or did it originate within
> GDB?  In either case, I don't want it propagating into the DWARF
> reader; that's horrible.  :)

 I have just derived an arbitrary name from make_msymbol_special.  No 
preference either way, but I gather this is not the way we want to go 
anyway.

  Maciej


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: MIPS: Handle manual calls of MIPS16 functions with a call stub
  2008-02-15 11:36                 ` Maciej W. Rozycki
@ 2008-02-18 13:32                   ` Nigel Stephens
  2008-02-18 16:28                     ` Maciej W. Rozycki
  2008-02-22 16:38                     ` Jim Blandy
  0 siblings, 2 replies; 21+ messages in thread
From: Nigel Stephens @ 2008-02-18 13:32 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Jim Blandy, Daniel Jacobowitz, gdb-patches, Maciej W. Rozycki

Sorry if I repeat anything -- I've not been copied on this conversation
until now.

Maciej W. Rozycki wrote:
> On Wed, 13 Feb 2008, Jim Blandy wrote:
>
>   
>> It seems that we all agree on the following:
>>
>> - It's contrary to the DWARF spec for producers to put arch-specific
>>   information in the low bits of the addresses in the line number
>>   table, function and block address ranges, and so on.  Existing
>>   toolchains that do this are buggy, but that's life.
>>     

Hmm. The ELF symbol table has an "out of band" mechanism to distinguish
symbols which refer to different architectural encodings, using the
architecture-specific bits in the st_info field. But how would you
represent that in  DWARF's object file encoding, noting that the
"MIPS16-ness" of an undefined symbol is not known at compile time, only
at final link time.

And this is not just a way of sneaking architecture-specific
"information" through the object file: it is a fundamental aspect of the
MIPS architecture. From the running software's point of view bit 0 of
the PC must be set to execute MIPS16 instructions (e.g. when performing
an indirect jump or function call) and it will always be viewed as set
when made visible to software (e.g. in the return address register after
a function call, or saved on the stack, or in the exception program
counter). Bit 0 must therefore be set in any data which points to a
MIPS16 instruction. Because of the way that DWARF information is
generated using assembler directives, then by the time a program is
fully linked bit 0 of a to a MIPS16 address within a DWARF section will
"by definition" be set: it would otherwise not be a valid pointer to a
MIPS16 instruction. I suppose that we could invent new relocation types
for DWARF data, or get the linker to relocate data within .dwarf*
sections differently, but that would be an intrusive change.

>> - The addresses in GDB's line tables and block ranges should not have
>>   such extra bits set in them.
>>     

Well, I suppose that you could clear bit 0, after stashing it in some
other field of the line table or block range. But you'd then have to
make sure that it continued to be passed around, along with the address
-- or else at some point you'd have to fold it back into bit0 of the
address anyway.

>> - The proper place to store arch-specific data on minimal symbols is
>>   in the 'info' field of 'struct minimal_symbol'.  In cases like
>>   MIPS16, the gdbarch_push_dummy_call method can consult that
>>   information at call time to see which calling convention is
>>   appropriate.
>>     


>
>  I certainly agree here.
>   

What about when there isn't an ELF symbol associated with an address?

>   
>> If we agree that we want to work around the linker bugs in GDB's
>> reader, that means we have to clear those bits and stash the
>> information elsewhere when we read the DWARF.  So something like
>> Maciej's original patch doesn't seem wrong to me.
>>     
>
>  OK, but the original patch does not clear the bit, but actually it sets 
> it in the single case discovered so far where GAS gets it wrong (and 
> linker does not fix up in any way). 

Maybe fix GAS then? Maciej, can you provide example assembler code that
GAS handles incorrectly?

>  What I attempted meanwhile was to 
> adjust the DWARF reader so that it effectively ignores the bit.  It proved 
> not as straightforward as it could immediately be thought it would be as 
> with some structures addresses are calculated cumulatively and one or more 
> addends may be odd.  It is therefore necessary, where applicable, not only 
> to mask out the LSB, but also to remember its value and carry it forward 
> to the next addition.
>
>  What I have developed is a crude hack as a proof of concept which just 
> does all the extra calculation unconditionally so that I could verify it 
> was at all workable as well as identify all the places that would have to 
> be changed and how.  If there is agreement to push this approach forward I 
> will have a look at how to do this properly.
>
>   
>> The name 'make_symbol_special' seems awfully vague, though.  Is the
>> term 'special' inherited from outside GDB, or did it originate within
>> GDB?  In either case, I don't want it propagating into the DWARF
>> reader; that's horrible.  :)
>>     
>
>  I have just derived an arbitrary name from make_msymbol_special.  No 
> preference either way, but I gather this is not the way we want to go 
> anyway.
>
>   Maciej
>   

-- 
Nigel Stephens     | MIPS Technologies (UK)      | t:(+44|0)1223 203110
Technical Director | 7200 Cambridge Research Pk  | f:(+44|0)1223 203181
.                  | Cambridge, England CB25 9TL | c:(+44|0)7976 686470


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: MIPS: Handle manual calls of MIPS16 functions with a call stub
  2008-02-18 13:32                   ` Nigel Stephens
@ 2008-02-18 16:28                     ` Maciej W. Rozycki
  2008-02-19 19:48                       ` Michael Snyder
  2008-02-22 16:38                     ` Jim Blandy
  1 sibling, 1 reply; 21+ messages in thread
From: Maciej W. Rozycki @ 2008-02-18 16:28 UTC (permalink / raw)
  To: Nigel Stephens
  Cc: Jim Blandy, Daniel Jacobowitz, gdb-patches, Maciej W. Rozycki

On Mon, 18 Feb 2008, Nigel Stephens wrote:

> >> - It's contrary to the DWARF spec for producers to put arch-specific
> >>   information in the low bits of the addresses in the line number
> >>   table, function and block address ranges, and so on.  Existing
> >>   toolchains that do this are buggy, but that's life.
> >>     
> 
> Hmm. The ELF symbol table has an "out of band" mechanism to distinguish
> symbols which refer to different architectural encodings, using the
> architecture-specific bits in the st_info field. But how would you
> represent that in  DWARF's object file encoding, noting that the
> "MIPS16-ness" of an undefined symbol is not known at compile time, only
> at final link time.

 As I see it there is really no need to notice the difference of the 
MIPS16 mode text references in DWARF records.  The architecture already 
observes the bit #0 in text references is not a part of the address and is 
merely a mode bit.  We have, in particular, joint text and data address 
space and where a MIPS16 instruction is treated as data, e.g. when 
inserting a breakpoint or with self-modifying code the corresponding data 
address has its bit #0 clear.

 Therefore it is safe and intuitive to actually mask it out everywhere but 
where the "pc" is to be addressed.  We already have the infrastructure in 
place: gdbarch_read_pc(), gdbarch_write_pc() and gdbarch_unwind_pc() are 
enough to get things in order as my proof-of-concept patch has shown.

> And this is not just a way of sneaking architecture-specific
> "information" through the object file: it is a fundamental aspect of the
> MIPS architecture. From the running software's point of view bit 0 of
> the PC must be set to execute MIPS16 instructions (e.g. when performing
> an indirect jump or function call) and it will always be viewed as set
> when made visible to software (e.g. in the return address register after
> a function call, or saved on the stack, or in the exception program
> counter). Bit 0 must therefore be set in any data which points to a
> MIPS16 instruction. Because of the way that DWARF information is
> generated using assembler directives, then by the time a program is
> fully linked bit 0 of a to a MIPS16 address within a DWARF section will
> "by definition" be set: it would otherwise not be a valid pointer to a
> MIPS16 instruction. I suppose that we could invent new relocation types
> for DWARF data, or get the linker to relocate data within .dwarf*
> sections differently, but that would be an intrusive change.

 Now that is where the problem starts -- we do not really have specific 
text-reference relocations for MIPS16 pointers and it makes things tricky.  
In principle any reference to a MIPS16 instruction which is not made for 
the purpose of making a function call should have its bit 0 set.  Imagine 
this piece of code:

#include <stdio.h>
#include <stdint.h>

void foo(void)
{
	return;
}

int main(void)
{
	printf("%04x\n", *(uint16_t *)foo);
	return 0;
}

One could reasonably expect (though probably not specified by the C 
standard) that code above would print the first instruction of foo() as a 
hexadecimal value.  As it is it does not.

> >> - The addresses in GDB's line tables and block ranges should not have
> >>   such extra bits set in them.
> >>     
> 
> Well, I suppose that you could clear bit 0, after stashing it in some
> other field of the line table or block range. But you'd then have to
> make sure that it continued to be passed around, along with the address
> -- or else at some point you'd have to fold it back into bit0 of the
> address anyway.

 It is carried around in the minimal symbol table (which is built from the 
ELF symbol table) and can be queried with pc_is_mips16().

> >> - The proper place to store arch-specific data on minimal symbols is
> >>   in the 'info' field of 'struct minimal_symbol'.  In cases like
> >>   MIPS16, the gdbarch_push_dummy_call method can consult that
> >>   information at call time to see which calling convention is
> >>   appropriate.
> >>     
> 
> 
> >
> >  I certainly agree here.
> >   
> 
> What about when there isn't an ELF symbol associated with an address?

 Then we have a problem anyway.  Functions marked as MIPS16 in the ELF 
symbol table define their respective ranges of addresses to be treated as 
MIPS16 code and everything else is standard MIPS -- this is no different 
now.  And with the ELF symbol table missing we cannot notice MIPS16 code 
now either except by setting the bit #0 where appropriate (e.g. "break 
*<address>") explicitly.

> >  OK, but the original patch does not clear the bit, but actually it sets 
> > it in the single case discovered so far where GAS gets it wrong (and 
> > linker does not fix up in any way). 
> 
> Maybe fix GAS then? Maciej, can you provide example assembler code that
> GAS handles incorrectly?

 I'll skip full code sent here previously and leave what's really 
relevant:

	.globl	print_ten_doubles
.LFB20:
	.loc 1 664 0
	# Stub function for print_ten_doubles (double, double)
	.set	nomips16
	.section	.mips16.fn.print_ten_doubles,"ax",@progbits
	.align	2
	.ent	__fn_stub_print_ten_doubles
__fn_stub_print_ten_doubles:

	# [...]

	.end	__fn_stub_print_ten_doubles
	.text
	.set	mips16
	.ent	print_ten_doubles
print_ten_doubles:

The presence of a section flip above makes relocations against ".LFB20" 
(which is really an alias to "print_ten_doubles") be converted to ones 
against .text which defeats the check to detect references to MIPS16 code 
in the linker.

 I think the bug in GAS here is inconsistency.  How the relocation is 
calculated should not depend on what syntactically lies between the 
instance of a label and the location it corresponds to.  This is 
independent from our abuse of DWARF records though.

  Maciej


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: MIPS: Handle manual calls of MIPS16 functions with a call stub
  2008-02-18 16:28                     ` Maciej W. Rozycki
@ 2008-02-19 19:48                       ` Michael Snyder
  0 siblings, 0 replies; 21+ messages in thread
From: Michael Snyder @ 2008-02-19 19:48 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Nigel Stephens, Jim Blandy, Daniel Jacobowitz, gdb-patches,
	Maciej W. Rozycki

On Mon, 2008-02-18 at 16:27 +0000, Maciej W. Rozycki wrote:
> On Mon, 18 Feb 2008, Nigel Stephens wrote:
> 
> > >> - It's contrary to the DWARF spec for producers to put arch-specific
> > >>   information in the low bits of the addresses in the line number
> > >>   table, function and block address ranges, and so on.  Existing
> > >>   toolchains that do this are buggy, but that's life.
> > >>     
> > 
> > Hmm. The ELF symbol table has an "out of band" mechanism to distinguish
> > symbols which refer to different architectural encodings, using the
> > architecture-specific bits in the st_info field. But how would you
> > represent that in  DWARF's object file encoding, noting that the
> > "MIPS16-ness" of an undefined symbol is not known at compile time, only
> > at final link time.
> 
>  As I see it there is really no need to notice the difference of the 
> MIPS16 mode text references in DWARF records.  The architecture already 
> observes the bit #0 in text references is not a part of the address and is 
> merely a mode bit.  We have, in particular, joint text and data address 
> space and where a MIPS16 instruction is treated as data, e.g. when 
> inserting a breakpoint or with self-modifying code the corresponding data 
> address has its bit #0 clear.

Isn't this a lot like Arm/Thumb?
How's it handled there?




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: MIPS: Handle manual calls of MIPS16 functions with a call stub
  2008-02-18 13:32                   ` Nigel Stephens
  2008-02-18 16:28                     ` Maciej W. Rozycki
@ 2008-02-22 16:38                     ` Jim Blandy
  1 sibling, 0 replies; 21+ messages in thread
From: Jim Blandy @ 2008-02-22 16:38 UTC (permalink / raw)
  To: Nigel Stephens
  Cc: Maciej W. Rozycki, Daniel Jacobowitz, gdb-patches, Maciej W. Rozycki

On Mon, Feb 18, 2008 at 5:32 AM, Nigel Stephens <nigel@mips.com> wrote:
>  Maciej W. Rozycki wrote:
>  > On Wed, 13 Feb 2008, Jim Blandy wrote:
>  >> It seems that we all agree on the following:
>  >>
>  >> - It's contrary to the DWARF spec for producers to put arch-specific
>  >>   information in the low bits of the addresses in the line number
>  >>   table, function and block address ranges, and so on.  Existing
>  >>   toolchains that do this are buggy, but that's life.
>
>  Hmm. The ELF symbol table has an "out of band" mechanism to distinguish
>  symbols which refer to different architectural encodings, using the
>  architecture-specific bits in the st_info field. But how would you
>  represent that in  DWARF's object file encoding, noting that the
>  "MIPS16-ness" of an undefined symbol is not known at compile time, only
>  at final link time.
>
>  And this is not just a way of sneaking architecture-specific
>  "information" through the object file: it is a fundamental aspect of the
>  MIPS architecture. From the running software's point of view bit 0 of
>  the PC must be set to execute MIPS16 instructions (e.g. when performing
>  an indirect jump or function call) and it will always be viewed as set
>  when made visible to software (e.g. in the return address register after
>  a function call, or saved on the stack, or in the exception program
>  counter). Bit 0 must therefore be set in any data which points to a
>  MIPS16 instruction.

If you're going to conclude that DWARF DW_AT_low_pc and DW_AT_high_pc
attributes ought to have bit zero set when they show the extent of
MIPS16 code, I don't think that's right.  After all, if I want to
disassemble such an instruction, which address do I start reading
bytes from? I must clear bit zero.

Does DWARF information refer to undefined symbols?  Doesn't the
compiler know the mode associated with every symbol it refers to in
DWARF?  Couldn't relocs against DWARF data citing MIPS16 symbols
subtract off bit 0 in the addend?

GDB actually does have mechanisms (which Maciej mentions) for setting
and masking off bit zero when writing and reading the PC.  It seems to
me GDB could look up the linker symbol associated with an address to
find the appropriate mode.


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2008-02-22 11:17 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-01-31 18:14 MIPS: Handle manual calls of MIPS16 functions with a call stub Maciej W. Rozycki
2008-01-31 22:08 ` Daniel Jacobowitz
2008-02-01 10:27   ` Maciej W. Rozycki
2008-02-01 14:19     ` Daniel Jacobowitz
2008-02-01 15:34       ` Maciej W. Rozycki
2008-02-01 16:58         ` Daniel Jacobowitz
2008-02-01 17:07           ` Maciej W. Rozycki
2008-02-01 17:15             ` Daniel Jacobowitz
2008-02-04 16:14     ` Maciej W. Rozycki
2008-02-04 16:39       ` Daniel Jacobowitz
2008-02-08 14:23         ` Maciej W. Rozycki
2008-02-08 14:57           ` Daniel Jacobowitz
2008-02-08 18:06           ` Jim Blandy
2008-02-08 18:08           ` Jim Blandy
2008-02-13 18:28             ` Maciej W. Rozycki
2008-02-13 20:54               ` Jim Blandy
2008-02-15 11:36                 ` Maciej W. Rozycki
2008-02-18 13:32                   ` Nigel Stephens
2008-02-18 16:28                     ` Maciej W. Rozycki
2008-02-19 19:48                       ` Michael Snyder
2008-02-22 16:38                     ` Jim Blandy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox