[needs GWP] Fix CFI directives for MIPS16

[Richard Sandiford <rdsandiford at googlemail dot com>]

This patch fixes a problem with .cfi_* directives on MIPS16.

If GCC 4.4 needs to write exception information for a function,
it starts the function with a construct like this:

$Lfoo:
        .cfi_startproc

And .gcc_except_table stores code addresses as PC-relative offsets
from the FDE's code start address.  This FDE address in turn is
represented by a fake label created by .cfi_startproc, but since
GCC doesn't have access to that label, it uses $Lfoo instead.
So GCC encodes a code address "bar" as:

        bar-$Lfoo

The problem for MIPS16 code is that $Lfoo is a MIPS16 label while the
fake label created by .cfi_startproc is not.  So $Lfoo is 1 greater than
the FDE start address, which means that all runtime calculations based
on .gcc_except_table offsets are out by 1.

We could fix this in GCC by forcing it to use a non-MIPS16 base label
instead of a MIPS16 base label.  However, we'd have to jump through some
hoops to do that, and assembly coders who want the same sorts of table
would need to do the same thing.  This seems to go against the spirit
of having user-friendly .cfi_* directives.

I think it would be better if .cfi_startproc used ISA-encoded start
addresses, which is what we've traditionally used in pre-.cfi FDEs.
The question then is how to implement that.

The MIPS port normally uses the tc_frob_label hook to watch out for
potential MIPS16 labels.  But fake labels are not passed through this
hook.  Rather than change that (which might not be desirable on other
ports) I thought it would be better to define tc_symbol_new_hook and
look out for fake labels there.  (We can't simply replace tc_frob_label
with tc_symbol_new_hook because local labels don't go through the latter.)

symbols.c already has code to ask "is this symbol equivalent to '.'"?
I split that out into a separate function so that I could reuse it here.

Note that labels before a ".end" are always even, so:

     .cfi_endproc
     .end ...

sets the FDE end address to the real (non-ISA-encoded) end address.
This is what we want, and is again what pre-.cfi FDEs do.

Tested for binutils on mips64-linux-gnu and mipsisa64-elfoabi.
Also tested for gcc on mipsisa64-elfoabi, where it fixes a lot
of EH failures.  OK to install?

(For the record, I'll probably have to patch GCC so that it doesn't
use .cfi directives for gas < 2.20.)

Richard


gas/
	* symbols.h (symbol_value_now_p): Declare.
	* symbols.c (symbol_value_now_p): New function, split out from...
	(colon): ...here.
	* config/tc-mips.h (tc_symbol_new_hook): Define.
	(mips_symbol_new_hook): Declare.
	* config/tc-mips.c (mips_fix_adjustable): Return true for fixups
	involving a fake label.
	(mips_record_insn_label): New function, split out from...
	(mips_define_label): ...here.
	(mips_symbol_new_hook): New function.

gas/testsuite/
	* gas/mips/cfi-mips16-1.s, gas/mips/cfi-mips16-1.d: New test.
	* gas/mips/mips.exp: Run it.

Index: gas/symbols.h
===================================================================
--- gas/symbols.h	2008-11-22 20:54:31.000000000 +0000
+++ gas/symbols.h	2008-11-22 20:57:12.000000000 +0000
@@ -175,6 +175,7 @@ extern expressionS *symbol_get_value_exp
 extern void symbol_set_value_expression (symbolS *, const expressionS *);
 extern offsetT *symbol_X_add_number (symbolS *);
 extern void symbol_set_value_now (symbolS *);
+extern bfd_boolean symbol_value_now_p (symbolS *);
 extern void symbol_set_frag (symbolS *, fragS *);
 extern fragS *symbol_get_frag (symbolS *);
 extern void symbol_mark_used (symbolS *);
Index: gas/symbols.c
===================================================================
--- gas/symbols.c	2008-11-22 20:54:31.000000000 +0000
+++ gas/symbols.c	2008-11-23 11:34:25.000000000 +0000
@@ -425,9 +425,7 @@ colon (/* Just seen "x:" - rattle symbol
       else
 	{
 	  /* Don't blow up if the definition is the same.  */
-	  if (!(frag_now == symbolP->sy_frag
-		&& S_GET_VALUE (symbolP) == frag_now_fix ()
-		&& S_GET_SEGMENT (symbolP) == now_seg))
+	  if (symbol_value_now_p (symbolP))
 	    {
 	      as_bad (_("symbol `%s' is already defined"), sym_name);
 	      symbolP = symbol_clone (symbolP, 0);
@@ -2405,6 +2403,17 @@ symbol_set_value_now (symbolS *sym)
   symbol_set_frag (sym, frag_now);
 }
 
+/* Return true if SYM points to the current position in the current
+   segment.  */
+
+bfd_boolean
+symbol_value_now_p (symbolS *sym)
+{
+  return (frag_now == sym->sy_frag
+	  && S_GET_VALUE (sym) == frag_now_fix ()
+	  && S_GET_SEGMENT (sym) == now_seg);
+}
+
 /* Set the frag of a symbol.  */
 
 void
Index: gas/config/tc-mips.h
===================================================================
--- gas/config/tc-mips.h	2008-11-22 20:54:31.000000000 +0000
+++ gas/config/tc-mips.h	2008-11-22 20:57:12.000000000 +0000
@@ -109,6 +109,9 @@ extern int mips_parse_long_option (const
 #define tc_frob_label(sym) mips_define_label (sym)
 extern void mips_define_label (symbolS *);
 
+#define tc_symbol_new_hook(sym) mips_symbol_new_hook (sym)
+extern void mips_symbol_new_hook (symbolS *);
+
 #define tc_frob_file_before_adjust() mips_frob_file_before_adjust ()
 extern void mips_frob_file_before_adjust (void);
 
Index: gas/config/tc-mips.c
===================================================================
--- gas/config/tc-mips.c	2008-11-22 20:54:31.000000000 +0000
+++ gas/config/tc-mips.c	2008-11-23 11:37:44.000000000 +0000
@@ -13963,6 +13963,13 @@ md_estimate_size_before_relax (fragS *fr
 int
 mips_fix_adjustable (fixS *fixp)
 {
+  /* Fixups against the fake symbol must always be reducible.
+     It is up to the creator of the fake label to ensure that
+     this is safe.  In particular, fake MIPS16 labels are not used
+     in ways that would require the special treatment described below.  */
+  if (strcmp (S_GET_NAME (fixp->fx_addsy), FAKE_LABEL_NAME) == 0)
+    return 1;
+
   if (fixp->fx_r_type == BFD_RELOC_VTABLE_INHERIT
       || fixp->fx_r_type == BFD_RELOC_VTABLE_ENTRY)
     return 0;
@@ -14524,16 +14531,15 @@ mips_frob_file_after_relocs (void)
 
 #endif
 
-/* This function is called whenever a label is defined.  It is used
-   when handling branch delays; if a branch has a label, we assume we
-   can not move it.  */
+/* Record that symbol SYM might refer to the next instruction.  */
 
-void
-mips_define_label (symbolS *sym)
+static void
+mips_record_insn_label (symbolS *sym)
 {
-  segment_info_type *si = seg_info (now_seg);
+  segment_info_type *si;
   struct insn_label_list *l;
 
+  si = seg_info (now_seg);
   if (free_insn_labels == NULL)
     l = (struct insn_label_list *) xmalloc (sizeof *l);
   else
@@ -14545,7 +14551,28 @@ mips_define_label (symbolS *sym)
   l->label = sym;
   l->next = si->label_list;
   si->label_list = l;
+}
 
+/* The MIPS implementation of tc_symbol_new_hook.  */
+
+void
+mips_symbol_new_hook (symbolS *sym)
+{
+  /* Look for fake symbols that are equivalent ".".  Such labels are
+     not passed through mips_define_label.  */
+  if (strcmp (S_GET_NAME (sym), FAKE_LABEL_NAME) == 0
+      && symbol_value_now_p (sym))
+    mips_record_insn_label (sym);
+}
+
+/* This function is called whenever a label is defined.  It is used
+   when handling branch delays; if a branch has a label, we assume we
+   can not move it.  */
+
+void
+mips_define_label (symbolS *sym)
+{
+  mips_record_insn_label (sym);
 #ifdef OBJ_ELF
   dwarf2_emit_label (sym);
 #endif
Index: gas/testsuite/gas/mips/cfi-mips16-1.s
===================================================================
--- /dev/null	2008-11-22 08:45:49.564097750 +0000
+++ gas/testsuite/gas/mips/cfi-mips16-1.s	2008-11-23 10:42:09.000000000 +0000
@@ -0,0 +1,17 @@
+start:
+	.cfi_startproc
+	.set	mips16
+	.set	noreorder
+	.global	foo
+	.type	foo,@function
+	.ent	foo
+foo:
+	nop
+x:
+	jr	$31
+	nop
+	.cfi_endproc
+	.end	foo
+
+	.data
+	.word	x - start
Index: gas/testsuite/gas/mips/cfi-mips16-1.d
===================================================================
--- /dev/null	2008-11-22 08:45:49.564097750 +0000
+++ gas/testsuite/gas/mips/cfi-mips16-1.d	2008-11-23 11:04:18.000000000 +0000
@@ -0,0 +1,28 @@
+#as: -32 -EB
+#objdump: -r -sj.data -sj.eh_frame
+
+.*
+
+RELOCATION RECORDS FOR \[\.eh_frame\]:
+OFFSET   TYPE              VALUE *
+0000001c R_MIPS_32         \.text
+
+# We're testing three things:
+#
+#   1) that x - start == 2
+#
+#   2) that, taking the in-place addend into account, the relocation
+#      is against .text+1 rather than .text
+#
+#   3) that the FDE code length at 0x20 is 5 rather than 6.
+#      (This is because symbols before a .end are never MIPS16
+#      symbols; they are always the true end address.  So while
+#      the symbol created by .cfi_startproc is MIPS16, the symbol
+#      created by .cfi_endproc is not.
+#
+Contents of section \.data:
+ 0000 00000002 .*
+Contents of section \.eh_frame:
+ 0000 00000010 00000000 017a5200 017c1f01  .........zR.....
+ 0010 0b0d1d00 00000010 00000018 00000001  ................
+ 0020 00000005 00000000                    ........ *
Index: gas/testsuite/gas/mips/mips.exp
===================================================================
--- gas/testsuite/gas/mips/mips.exp	2008-11-23 10:45:25.000000000 +0000
+++ gas/testsuite/gas/mips/mips.exp	2008-11-23 10:45:35.000000000 +0000
@@ -834,4 +834,5 @@ if { [istarget mips*-*-vxworks*] } {
     run_dump_test "mips16-vis-1"
     run_dump_test "call-nonpic-1"
     if $has_newabi { run_dump_test "cfi-n64-1" }
+    run_dump_test "cfi-mips16-1"
 }