This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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 and filter fake labels there calling the recording 
hook like it is done for regular labels (except DWARF-2 information is 
not produced preserving current behaviour).

 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 fake labels.
	* config/tc-mips.h (tc_symbol_new_hook): New macro.
	(mips_symbol_new_hook): New prototype.

 OK to apply?

  Maciej

binutils-gas-mips-fake.diff
Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.c	2010-07-24 18:30:03.000000000 +0100
+++ binutils-fsf-trunk-quilt/gas/config/tc-mips.c	2010-07-24 22:37:13.000000000 +0100
@@ -11178,26 +11178,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 *
@@ -12746,8 +12732,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
@@ -12769,9 +12755,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
@@ -13536,9 +13521,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)
     {
@@ -13576,9 +13561,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)
     {
@@ -14735,12 +14720,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;
@@ -14756,11 +14741,35 @@ 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 fake label symbol created from `.', then we record the
+   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 (TC_FAKE_LABEL (S_GET_NAME (sym))
+      && 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-07-24 18:21:49.000000000 +0100
+++ binutils-fsf-trunk-quilt/gas/config/tc-mips.h	2010-07-24 18:40:48.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);
 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]