Bug 13509 - branch target to the wrong location
Summary: branch target to the wrong location
Status: ASSIGNED
Alias: None
Product: binutils
Classification: Unclassified
Component: gas (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Andrew Pinski
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-16 03:40 UTC by Andrew Pinski
Modified: 2024-01-27 17:33 UTC (History)
2 users (show)

See Also:
Host:
Target: mips-linux
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Pinski 2011-12-16 03:40:39 UTC
Take:
.align 2
        .global main
        .ent main
main:
	beqz $5, .L1
	nop
	addi $5, $5, 1
	b .L2
	nop
	
.L1:
#	.L3=.
	.align 3
.L2:
	la $5, .L3
        jr      $31
        nop
        .end main
--- CUT ---
The two branches will take you to L2.
If we uncomment the assignment to .L3, we get something different, where one is before the .align and the other is after.  This causes in the end causes a miscompare inside GCC when comparing with and without debugging info.
Comment 1 Alan Modra 2011-12-16 05:16:24 UTC
Seen better if .align 4 or larger alignment is used rather than .align 3
Comment 2 Andrew Pinski 2011-12-16 20:08:29 UTC
(In reply to comment #1)
> Seen better if .align 4 or larger alignment is used rather than .align 3

Sorry for the poor bug report at the time.  I was going to fix up later in the evening as I knew I forgot the target.  Anyways I am looking into fixing this bug.
Comment 3 Andrew Pinski 2011-12-16 20:55:14 UTC
   The MIPS assembler also automatically adjusts any preceding
   label.  */
Comment 4 Andrew Pinski 2011-12-16 22:18:04 UTC
The reason why GCC emits "LABEL = ." is because of the following reason:
* Print debug labels as "foo = ." rather than "foo:" because they should
   represent a byte pointer rather than an ISA-encoded address.
...

The .uleb128 requies $LFBxxx to match the FDE start address, which is
   likewise a byte pointer rather than an ISA-encoded address.

So I think we should just delete the code in the assembler which moves the labels since they don't move "= ." based labels.
Comment 5 Andrew Pinski 2011-12-16 22:52:49 UTC
Or maybe I should add support to mips_record_label for recording the "= ." based labels.
Comment 6 Andrew Pinski 2011-12-16 23:25:39 UTC
Looks like this version of the mips patch:
http://sourceware.org/ml/binutils/2010-10/msg00512.html
Would have fixed it also.  But it was rejected and only a fake labels were added rather than all cases where the "symbol = ." .
Comment 7 Andrew Pinski 2011-12-17 00:36:59 UTC
Actually the code does not do what the comment says it does, it only moves one previous label.  Moving all labels allows the code to be correct.
Comment 8 Andrew Pinski 2011-12-17 00:38:56 UTC
Something like:
Index: config/tc-mips.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.c,v
retrieving revision 1.500
diff -u -p -r1.500 tc-mips.c
--- config/tc-mips.c	8 Dec 2011 20:47:25 -0000	1.500
+++ config/tc-mips.c	17 Dec 2011 00:38:28 -0000
@@ -15633,7 +15633,7 @@ get_symbol (void)
    label.  */
 
 static void
-mips_align (int to, int *fill, symbolS *label)
+mips_align (int to, int *fill, struct insn_label_list *labels)
 {
   mips_emit_delays ();
   mips_record_compressed_mode ();
@@ -15642,11 +15642,13 @@ mips_align (int to, int *fill, symbolS *
   else
     frag_align (to, fill ? *fill : 0, 0);
   record_alignment (now_seg, to);
-  if (label != NULL)
+ while (labels != NULL)
     {
+      symbolS *label = labels->label;
       gas_assert (S_GET_SEGMENT (label) == now_seg);
       symbol_set_frag (label, frag_now);
       S_SET_VALUE (label, (valueT) frag_now_fix ());
+      labels = labels->next;
     }
 }
 
@@ -15689,7 +15691,7 @@ s_align (int x ATTRIBUTE_UNUSED)
       struct insn_label_list *l = si->label_list;
       /* Auto alignment should be switched on by next section change.  */
       auto_align = 1;
-      mips_align (temp, fill_ptr, l != NULL ? l->label : NULL);
+      mips_align (temp, fill_ptr, l);
     }
   else
     {
@@ -15859,12 +15861,10 @@ s_cons (int log_size)
 {
   segment_info_type *si = seg_info (now_seg);
   struct insn_label_list *l = si->label_list;
-  symbolS *label;
 
-  label = l != NULL ? l->label : NULL;
   mips_emit_delays ();
   if (log_size > 0 && auto_align)
-    mips_align (log_size, 0, label);
+    mips_align (log_size, 0, l);
   cons (1 << log_size);
   mips_clear_insn_labels ();
 }
@@ -15874,18 +15874,15 @@ s_float_cons (int type)
 {
   segment_info_type *si = seg_info (now_seg);
   struct insn_label_list *l = si->label_list;
-  symbolS *label;
-
-  label = l != NULL ? l->label : NULL;
 
   mips_emit_delays ();
 
   if (auto_align)
     {
       if (type == 'd')
-	mips_align (3, 0, label);
+	mips_align (3, 0, l);
       else
-	mips_align (2, 0, label);
+	mips_align (2, 0, l);
     }
 
   float_cons (type);
@@ -16653,7 +16650,6 @@ s_gpword (int ignore ATTRIBUTE_UNUSED)
 {
   segment_info_type *si;
   struct insn_label_list *l;
-  symbolS *label;
   expressionS ex;
   char *p;
 
@@ -16666,10 +16662,9 @@ s_gpword (int ignore ATTRIBUTE_UNUSED)
 
   si = seg_info (now_seg);
   l = si->label_list;
-  label = l != NULL ? l->label : NULL;
   mips_emit_delays ();
   if (auto_align)
-    mips_align (2, 0, label);
+    mips_align (2, 0, l);
 
   expression (&ex);
   mips_clear_insn_labels ();
@@ -16693,7 +16688,6 @@ s_gpdword (int ignore ATTRIBUTE_UNUSED)
 {
   segment_info_type *si;
   struct insn_label_list *l;
-  symbolS *label;
   expressionS ex;
   char *p;
 
@@ -16706,10 +16700,9 @@ s_gpdword (int ignore ATTRIBUTE_UNUSED)
 
   si = seg_info (now_seg);
   l = si->label_list;
-  label = l != NULL ? l->label : NULL;
   mips_emit_delays ();
   if (auto_align)
-    mips_align (3, 0, label);
+    mips_align (3, 0, l);
 
   expression (&ex);
   mips_clear_insn_labels ();