This is the mail archive of the binutils@sources.redhat.com 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]

mn10300 linker relaxations broken (patch)


There are very serious bugs with mn10300 linker relaxations.  The most
simple of them is that we sometimes relax negative values into
zero-extended 16-bit operands.  This seldom shows up in practice,
since it's quite rare to have addresses in the range -0x8000..0, but
it's possible, and it will do bad things then.  I thought of creating
a separate chunk of code that would work on values in the range
0..0xffff, but I was too lazy, so I just arranged for us to skip the
relaxation opportunity if we find the value is negative but the opcode
sign-extends its operand.  While I was at it, I fixed some comments
that were wrong.  Oh, BTW, does anybody know why the linker tests for
values < 0x7fff, instead of <= 0x7fff?  This seems wrong to me.  Not
that it would hit that much, but...

The other, more serious problem, is that we don't adjust
section+offset relocations, and all references to local labels are
adjusted to section+offset by the assembler.  (This is also a problem
because references to labels become ambiguous, especially if the
addends of the relocations are non-zero, but this is probably rare in
comparison with direct references to labels).

One idea I had was to fix the linker so that it searches for
section+offset relocations and transform them in temporary symbols
before doing any relaxations (so they can be adjusted when bytes are
deleted from the section containing them), but implementing this was
beyond my linker-hacking abilities.  Meanwhile, I fixed the assembler
so that it doesn't ambiguate references to labels, letting them make
it to the linker unchanged, so linker relaxations immediately work.

Tested on i686-pc-linux-gnu-x-mn10300-elf.  Ok to install?

Index: bfd/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* elf-m10300.c (mn10300_elf_relax_section): Don't relax
	negative 32-bit operands to 16 operands when the operand is
	going to be zero-extended by the modified opcode.

Index: bfd/elf-m10300.c
===================================================================
RCS file: /cvs/src/src/bfd/elf-m10300.c,v
retrieving revision 1.12
diff -u -p -r1.12 elf-m10300.c
--- bfd/elf-m10300.c 2001/03/08 21:03:58 1.12
+++ bfd/elf-m10300.c 2001/05/14 02:52:00
@@ -2183,6 +2183,11 @@ mn10300_elf_relax_section (abfd, sec, li
 		  case 0x91:
 		  case 0x92:
 		  case 0x93:
+		    /* sp-based offsets are zero-extended.  */
+		    if (code >= 0x90 && code <= 0x93
+			&& (long)value < 0)
+		      continue;
+
 		    /* Note that we've changed the relocation contents, etc.  */
 		    elf_section_data (sec)->relocs = internal_relocs;
 		    free_relocs = NULL;
@@ -2231,6 +2236,11 @@ mn10300_elf_relax_section (abfd, sec, li
 			&& (value & 0x8000))
 		      continue;
 
+		    /* mov imm16, an zero-extends the immediate.  */
+		    if (code == 0xdc
+			&& (long)value < 0)
+		      continue;
+
 		    /* Note that we've changed the relocation contents, etc.  */
 		    elf_section_data (sec)->relocs = internal_relocs;
 		    free_relocs = NULL;
@@ -2276,18 +2286,18 @@ mn10300_elf_relax_section (abfd, sec, li
 		    break;
 
 		  /* mov (abs32),an    -> mov (abs16),an
-		     mov (d32,sp),an   -> mov (d32,sp),an
-		     mov (d32,sp),dn   -> mov (d32,sp),dn
-		     movbu (d32,sp),dn -> movbu (d32,sp),dn
-		     movhu (d32,sp),dn -> movhu (d32,sp),dn
+		     mov (d32,sp),an   -> mov (d16,sp),an
+		     mov (d32,sp),dn   -> mov (d16,sp),dn
+		     movbu (d32,sp),dn -> movbu (d16,sp),dn
+		     movhu (d32,sp),dn -> movhu (d16,sp),dn
 		     add imm32,dn      -> add imm16,dn
 		     cmp imm32,dn      -> cmp imm16,dn
 		     add imm32,an      -> add imm16,an
 		     cmp imm32,an      -> cmp imm16,an
-		     and imm32,dn      -> and imm32,dn
-		     or imm32,dn       -> or imm32,dn
-		     xor imm32,dn      -> xor imm32,dn
-		     btst imm32,dn     -> btst imm32,dn */
+		     and imm32,dn      -> and imm16,dn
+		     or imm32,dn       -> or imm16,dn
+		     xor imm32,dn      -> xor imm16,dn
+		     btst imm32,dn     -> btst imm16,dn */
 
 		  case 0xa0:
 		  case 0xb0:
@@ -2303,6 +2313,16 @@ mn10300_elf_relax_section (abfd, sec, li
 		  case 0xe1:
 		  case 0xe2:
 		  case 0xe3:
+		    /* cmp imm16, an zero-extends the immediate.  */
+		    if (code == 0xdc
+			&& (long)value < 0)
+		      continue;
+
+		    /* So do sp-based offsets.  */
+		    if (code >= 0xb0 && code <= 0xb3
+			&& (long)value < 0)
+		      continue;
+
 		    /* Note that we've changed the relocation contents, etc.  */
 		    elf_section_data (sec)->relocs = internal_relocs;
 		    free_relocs = NULL;
Index: gas/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>
	* config/tc-mn10300.c (mn10300_force_relocation): Don't
	optimize differences between symbols in code sections to
	constants.
	(mn10300_fix_adjustable): Don't adjust to section+offset
	relocations pointing at symbols in code sections.

Index: gas/config/tc-mn10300.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mn10300.c,v
retrieving revision 1.24
diff -u -p -r1.24 tc-mn10300.c
--- gas/config/tc-mn10300.c 2001/05/13 23:16:30 1.24
+++ gas/config/tc-mn10300.c 2001/05/14 00:06:01
@@ -1986,6 +1986,15 @@ mn10300_force_relocation (fixp)
       || fixp->fx_r_type == BFD_RELOC_VTABLE_ENTRY)
     return 1;
 
+  /* Do not adjust relocations involving symbols in code sections,
+     because it breaks linker relaxations.  This could be fixed in the
+     linker, but this fix is simpler, and it pretty much only affects
+     object size a little bit.  */
+  if ((S_GET_SEGMENT (fixp->fx_addsy)->flags & SEC_CODE)
+      && fixp->fx_subsy
+      && S_GET_SEGMENT (fixp->fx_addsy) == S_GET_SEGMENT (fixp->fx_subsy))
+    return 1;
+
   return 0;
 }
 
@@ -2002,6 +2011,13 @@ mn10300_fix_adjustable (fixp)
 
   if (fixp->fx_r_type == BFD_RELOC_VTABLE_INHERIT
       || fixp->fx_r_type == BFD_RELOC_VTABLE_ENTRY)
+    return 0;
+
+  /* Do not adjust relocations involving symbols in code sections,
+     because it breaks linker relaxations.  This could be fixed in the
+     linker, but this fix is simpler, and it pretty much only affects
+     object size a little bit.  */
+  if (S_GET_SEGMENT (fixp->fx_addsy)->flags & SEC_CODE)
     return 0;
 
   return 1;

-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                  aoliva@{cygnus.com, redhat.com}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist    *Please* write to mailing lists, not to me

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