[PATCH 3.0/4 v2] MIPS/GAS: Record fake labels like regular ones

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

Hi,

 Unlike ordinary labels fake ones associated with each use of the "dot" 
special symbol are not recorded by MIPS code for later processing.  As a 
result they are not noticed when branch swapping is made and bad code is 
generated as a result.  This program:

	.text
foo:
	nop
	b	.

assembles to this:

Disassembly of section .text:

00000000 <foo>:
   0:	10000000 	b	4 <foo+0x4>
   4:	00000000 	nop

which is clearly not what was intended.  The obvious fix is to record 
these fake labels like ordinary ones are.  Branch swapping code will then 
notice them and refrain from doing so.

 The fix below implements it.  I've used the hook called by generic code 
for each new symbol, but compared to the initial version I have removed 
the explicit check for fake labels based on your comments addressing the 
other change.  Like with the original the recording hook is called like it 
is done for regular labels (except DWARF-2 information is not produced 
preserving current behaviour).

 Depending on the way symbols are defined this approach causes the 
recording hook to be called once or twice for the same symbol.  It will be 
called once for symbols that are not defined as labels, i.e. other than 
with colon() (which I think still should prevent branch swapping if they 
refer to code), and for true labels that have been referenced before the 
actual definition (i.e. forward referenced in the other sense ;) ).  It 
will be called twice for true labels whose definition is lexically the 
first ocurrence of the symbol defined.  Due to the complicated matter of 
dependencies here I have failed to find a way to avoid this double call 
without turning half of GAS upside down and I didn't feel like defining a 
new target hook would be the right way to do that either.  Except from a 
small performance hit and some aesthetical concerns this approach has no 
negative effects.

 A nice side effect is special code to annotate MIPS16 fake labels (and 
the upcoming corresponding bit for microMIPS code) can now be removed as 
label handling code elsewhere will do that automatically.

 I had to reorder call to clear labels recorded for data directives 
though.  The problem with them was a newly generated label was recorded 
in the list as if applying to code that follows.  That would in turn move 
the label incorrectly if alignment operation was involved.  It could be 
triggered by code like this:

	.word	.
	.word	.

and possibly for symbols created with .eqv (I haven't checked the latter).  
The new arrangement is consistent with that used by append_insn() which 
only calls mips_clear_insn_labels() once all instruction processing has 
been made that further assures me it's the right change.

2010-07-26  Maciej W. Rozycki  <macro@codesourcery.com>

	gas/
	* config/tc-mips.c (my_getExpression): Remove MIPS16 fake label 
	annotation.
	(s_cons, s_float_cons, s_gpword, s_gpdword): Only clear labels 
	recorded once data expressions have been evaluated.
	(mips_define_label): Move code to record labels over to...
	(mips_record_label): ... this new function.
	(mips_symbol_new_hook): New function.  Call mips_record_label
	for symbols referring to the current location.
	* config/tc-mips.h (tc_symbol_new_hook): New macro.
	(mips_symbol_new_hook): New prototype.

 OK to apply?

  Maciej

binutils-gas-mips-label.diff
Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.c	2010-10-29 09:07:10.000000000 +0100
+++ binutils-fsf-trunk-quilt/gas/config/tc-mips.c	2010-10-29 09:07:10.000000000 +0100
@@ -11121,26 +11121,12 @@ static void
 my_getExpression (expressionS *ep, char *str)
 {
   char *save_in;
-  valueT val;
 
   save_in = input_line_pointer;
   input_line_pointer = str;
   expression (ep);
   expr_end = input_line_pointer;
   input_line_pointer = save_in;
-
-  /* If we are in mips16 mode, and this is an expression based on `.',
-     then we bump the value of the symbol by 1 since that is how other
-     text symbols are handled.  We don't bother to handle complex
-     expressions, just `.' plus or minus a constant.  */
-  if (mips_opts.mips16
-      && ep->X_op == O_symbol
-      && strcmp (S_GET_NAME (ep->X_add_symbol), FAKE_LABEL_NAME) == 0
-      && S_GET_SEGMENT (ep->X_add_symbol) == now_seg
-      && symbol_get_frag (ep->X_add_symbol) == frag_now
-      && symbol_constant_p (ep->X_add_symbol)
-      && (val = S_GET_VALUE (ep->X_add_symbol)) == frag_now_fix ())
-    S_SET_VALUE (ep->X_add_symbol, val + 1);
 }
 
 char *
@@ -12712,8 +12698,8 @@ s_cons (int log_size)
   mips_emit_delays ();
   if (log_size > 0 && auto_align)
     mips_align (log_size, 0, label);
-  mips_clear_insn_labels ();
   cons (1 << log_size);
+  mips_clear_insn_labels ();
 }
 
 static void
@@ -12735,9 +12721,8 @@ s_float_cons (int type)
 	mips_align (2, 0, label);
     }
 
-  mips_clear_insn_labels ();
-
   float_cons (type);
+  mips_clear_insn_labels ();
 }
 
 /* Handle .globl.  We need to override it because on Irix 5 you are
@@ -13502,9 +13487,9 @@ s_gpword (int ignore ATTRIBUTE_UNUSED)
   mips_emit_delays ();
   if (auto_align)
     mips_align (2, 0, label);
-  mips_clear_insn_labels ();
 
   expression (&ex);
+  mips_clear_insn_labels ();
 
   if (ex.X_op != O_symbol || ex.X_add_number != 0)
     {
@@ -13542,9 +13527,9 @@ s_gpdword (int ignore ATTRIBUTE_UNUSED)
   mips_emit_delays ();
   if (auto_align)
     mips_align (3, 0, label);
-  mips_clear_insn_labels ();
 
   expression (&ex);
+  mips_clear_insn_labels ();
 
   if (ex.X_op != O_symbol || ex.X_add_number != 0)
     {
@@ -14692,12 +14677,12 @@ 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.  */
+/* This function is called whenever a label is defined, including fake
+   labels.  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)
+static void
+mips_record_label (symbolS *sym)
 {
   segment_info_type *si = seg_info (now_seg);
   struct insn_label_list *l;
@@ -14713,11 +14698,34 @@ mips_define_label (symbolS *sym)
   l->label = sym;
   l->next = si->label_list;
   si->label_list = l;
+}
+
+/* This function is called as tc_frob_label() whenever a label is defined
+   and adds a DWARF-2 record we only want for true labels.  */
 
+void
+mips_define_label (symbolS *sym)
+{
+  mips_record_label (sym);
 #ifdef OBJ_ELF
   dwarf2_emit_label (sym);
 #endif
 }
+
+/* If this is a symbol referring to the current location, then we record
+   it as a label so that it is handled like ordinary text symbols are.
+   This prevents branches from being moved and bumps the value of the
+   symbol by 1 in compressed code.  */
+
+void
+mips_symbol_new_hook (symbolS *sym)
+{
+  if (S_GET_SEGMENT (sym) == now_seg
+      && symbol_get_frag (sym) == frag_now
+      && symbol_constant_p (sym)
+      && S_GET_VALUE (sym) == frag_now_fix ())
+    mips_record_label (sym);
+}
 
 #if defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)
 
Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.h
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.h	2010-10-29 09:07:06.000000000 +0100
+++ binutils-fsf-trunk-quilt/gas/config/tc-mips.h	2010-10-29 09:07:10.000000000 +0100
@@ -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);