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]

[ARM] Fix two cortex-a8 bugs


This patch fixes a couple of cortex-a8 erratum bugs I found.

1) We were presuming that merely finding the same number of cortex-a8 fixes was sufficient to determine we'd converged. This is incorrect. We may have found a different fix requiring a differently sized veneer. This causes the linker to blow up later (with the inscrutable 'bad value' error) when the second fix was longer than the first and we'd not recalculated section sizes. As you can see from the test case, the trigger is two potential fixups close together, such that initially we find the second one, then upon recalculation an unrelated thumb<->arm trampoline moves things such that the first fixup is now the critical one.

2) We place thumb<->arm trampolines and cortex-a8 veneers in the same stub section. The former are all multiples of 4 bytes and presume 4 byte alignment, but the latter are multiples of 2 bytes in some cases, and do not presume alignment. When calculating the stub sizes we do not insert alignment padding, because we don't know which order the stubs will be emitted in. The emission order is essentially random, being dictated by a hash table walk. Rather than arbitrarily pad all veneers to 4 bytes, I changed the stub emission to do 2 walks, one for non-cortex-a8 veneers and then a later one for those veneers. Thus guaranteeing the alignments required.

The latter bug is the one also found by Doug Kwan -- http://sourceware.org/ml/binutils/2009-07/msg00380.html. This fix avoids the additional padding of Doug's patch. Doug, if you'd like to check your testcases are fixed by this patch that'd be great!

tested on arm-eabi, ok?

nathan

--
Nathan Sidwell    ::   http://www.codesourcery.com   ::         CodeSourcery

2009-08-04  Nathan Sidwell  <nathan@codesourcery.com>

	bfd/
	* elf32-qrm.c (elf32_arm_stub_type): Add arm_stub_a8_veneer_lwm.
	(arm_build_one_stub): Build a8 veneers as a separate pass.
	(cortex_a8_erratum_scan): Add prev_num_a8_fixes and stub_changed_p
	parameters.  Use them to check if we create a different a8 fixup
	than the previous pass.
	(elf32_arm_size_stubs): Move scope of stub_changed and
	prev_num_a8_fixes into main loop.
	(elf32_arm_build_stubs): Build a8 veneers in a second pass.

	ld/testsuite/
	* ld-arm/cortex-a8-far-1.s: New.
	* ld-arm/cortex-a8-far-2.s: New.
	* ld-arm/cortex-a8-far.d: New.
	* ld-arm/arm-elf.exp: Add new test.

Index: bfd/elf32-arm.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-arm.c,v
retrieving revision 1.206
diff -c -3 -p -r1.206 elf32-arm.c
*** bfd/elf32-arm.c	27 Jul 2009 23:04:05 -0000	1.206
--- bfd/elf32-arm.c	4 Aug 2009 13:31:13 -0000
*************** static const insn_sequence elf32_arm_stu
*** 2228,2233 ****
--- 2228,2235 ----
  enum elf32_arm_stub_type {
    arm_stub_none,
    DEF_STUBS
+   /* Note the first a8_veneer type */
+   arm_stub_a8_veneer_lwm = arm_stub_a8_veneer_b_cond
  };
  #undef DEF_STUB
  
*************** arm_build_one_stub (struct bfd_hash_entr
*** 3437,3442 ****
--- 3439,3450 ----
    htab = elf32_arm_hash_table (info);
    stub_sec = stub_entry->stub_sec;
  
+   if ((htab->fix_cortex_a8 < 0)
+       != (stub_entry->stub_type >= arm_stub_a8_veneer_lwm))
+     /* We have to do the a8 fixes last, as they are less aligned than
+        the other veneers.  */
+     return TRUE;
+   
    /* Make a note of the offset within the stubs for this entry.  */
    stub_entry->stub_offset = stub_sec->size;
    loc = stub_sec->contents + stub_entry->stub_offset;
*************** cortex_a8_erratum_scan (bfd *input_bfd,
*** 3893,3899 ****
  			unsigned int *num_a8_fixes_p,
  			unsigned int *a8_fix_table_size_p,
  			struct a8_erratum_reloc *a8_relocs,
! 			unsigned int num_a8_relocs)
  {
    asection *section;
    struct elf32_arm_link_hash_table *htab = elf32_arm_hash_table (info);
--- 3901,3909 ----
  			unsigned int *num_a8_fixes_p,
  			unsigned int *a8_fix_table_size_p,
  			struct a8_erratum_reloc *a8_relocs,
! 			unsigned int num_a8_relocs,
! 			unsigned prev_num_a8_fixes,
! 			bfd_boolean *stub_changed_p)
  {
    asection *section;
    struct elf32_arm_link_hash_table *htab = elf32_arm_hash_table (info);
*************** cortex_a8_erratum_scan (bfd *input_bfd,
*** 4105,4111 ****
  
                        if (((base_vma + i) & ~0xfff) == (target & ~0xfff))
                          {
!                           char *stub_name;
  
                            if (num_a8_fixes == a8_fix_table_size)
                              {
--- 4115,4121 ----
  
                        if (((base_vma + i) & ~0xfff) == (target & ~0xfff))
                          {
!                           char *stub_name = NULL;
  
                            if (num_a8_fixes == a8_fix_table_size)
                              {
*************** cortex_a8_erratum_scan (bfd *input_bfd,
*** 4115,4123 ****
                                  * a8_fix_table_size);
                              }
  
!                           stub_name = bfd_malloc (8 + 1 + 8 + 1);
!                           if (stub_name != NULL)
!                             sprintf (stub_name, "%x:%x", section->id, i);
  
                            a8_fixes[num_a8_fixes].input_bfd = input_bfd;
                            a8_fixes[num_a8_fixes].section = section;
--- 4125,4152 ----
                                  * a8_fix_table_size);
                              }
  
! 			  if (num_a8_fixes < prev_num_a8_fixes)
! 			    {
! 			      /* If we're doing a subsequent scan,
! 				 check if we've found the same fix as
! 				 before, and try and reuse the stub
! 				 name.  */
! 			      stub_name = a8_fixes[num_a8_fixes].stub_name;
! 			      if ((a8_fixes[num_a8_fixes].section != section)
! 				  || (a8_fixes[num_a8_fixes].offset != i))
! 				{
! 				  free (stub_name);
! 				  stub_name = NULL;
! 				  *stub_changed_p = TRUE;
! 				}
! 			    }
! 
! 			  if (!stub_name)
! 			    {
! 			      stub_name = bfd_malloc (8 + 1 + 8 + 1);
! 			      if (stub_name != NULL)
! 				sprintf (stub_name, "%x:%x", section->id, i);
! 			    }
  
                            a8_fixes[num_a8_fixes].input_bfd = input_bfd;
                            a8_fixes[num_a8_fixes].section = section;
*************** elf32_arm_size_stubs (bfd *output_bfd,
*** 4165,4174 ****
  {
    bfd_size_type stub_group_size;
    bfd_boolean stubs_always_after_branch;
-   bfd_boolean stub_changed = 0;
    struct elf32_arm_link_hash_table *htab = elf32_arm_hash_table (info);
    struct a8_erratum_fix *a8_fixes = NULL;
!   unsigned int num_a8_fixes = 0, prev_num_a8_fixes = 0, a8_fix_table_size = 10;
    struct a8_erratum_reloc *a8_relocs = NULL;
    unsigned int num_a8_relocs = 0, a8_reloc_table_size = 10, i;
  
--- 4194,4202 ----
  {
    bfd_size_type stub_group_size;
    bfd_boolean stubs_always_after_branch;
    struct elf32_arm_link_hash_table *htab = elf32_arm_hash_table (info);
    struct a8_erratum_fix *a8_fixes = NULL;
!   unsigned int num_a8_fixes = 0, a8_fix_table_size = 10;
    struct a8_erratum_reloc *a8_relocs = NULL;
    unsigned int num_a8_relocs = 0, a8_reloc_table_size = 10, i;
  
*************** elf32_arm_size_stubs (bfd *output_bfd,
*** 4223,4231 ****
        bfd *input_bfd;
        unsigned int bfd_indx;
        asection *stub_sec;
  
        num_a8_fixes = 0;
- 
        for (input_bfd = info->input_bfds, bfd_indx = 0;
  	   input_bfd != NULL;
  	   input_bfd = input_bfd->link_next, bfd_indx++)
--- 4251,4260 ----
        bfd *input_bfd;
        unsigned int bfd_indx;
        asection *stub_sec;
+       bfd_boolean stub_changed = FALSE;
+       unsigned prev_num_a8_fixes = num_a8_fixes;
  
        num_a8_fixes = 0;
        for (input_bfd = info->input_bfds, bfd_indx = 0;
  	   input_bfd != NULL;
  	   input_bfd = input_bfd->link_next, bfd_indx++)
*************** elf32_arm_size_stubs (bfd *output_bfd,
*** 4452,4457 ****
--- 4481,4487 ----
  			{
  			  /* The proper stub has already been created.  */
  			  free (stub_name);
+ 			  stub_entry->target_value = sym_value;
  			  break;
  			}
  
*************** elf32_arm_size_stubs (bfd *output_bfd,
*** 4548,4565 ****
            if (htab->fix_cortex_a8)
  	    {
                /* Sort relocs which might apply to Cortex-A8 erratum.  */
!               qsort (a8_relocs, num_a8_relocs, sizeof (struct a8_erratum_reloc),
                       &a8_reloc_compare);
  
                /* Scan for branches which might trigger Cortex-A8 erratum.  */
                if (cortex_a8_erratum_scan (input_bfd, info, &a8_fixes,
  					  &num_a8_fixes, &a8_fix_table_size,
! 					  a8_relocs, num_a8_relocs) != 0)
  		goto error_ret_free_local;
  	    }
  	}
  
!       if (htab->fix_cortex_a8 && num_a8_fixes != prev_num_a8_fixes)
          stub_changed = TRUE;
  
        if (!stub_changed)
--- 4578,4598 ----
            if (htab->fix_cortex_a8)
  	    {
                /* Sort relocs which might apply to Cortex-A8 erratum.  */
!               qsort (a8_relocs, num_a8_relocs,
! 		     sizeof (struct a8_erratum_reloc),
                       &a8_reloc_compare);
  
                /* Scan for branches which might trigger Cortex-A8 erratum.  */
                if (cortex_a8_erratum_scan (input_bfd, info, &a8_fixes,
  					  &num_a8_fixes, &a8_fix_table_size,
! 					  a8_relocs, num_a8_relocs,
! 					  prev_num_a8_fixes, &stub_changed)
! 		  != 0)
  		goto error_ret_free_local;
  	    }
  	}
  
!       if (prev_num_a8_fixes != num_a8_fixes)
          stub_changed = TRUE;
  
        if (!stub_changed)
*************** elf32_arm_size_stubs (bfd *output_bfd,
*** 4598,4605 ****
  
        /* Ask the linker to do its stuff.  */
        (*htab->layout_sections_again) ();
-       stub_changed = FALSE;
-       prev_num_a8_fixes = num_a8_fixes;
      }
  
    /* Add stubs for Cortex-A8 erratum fixes now.  */
--- 4631,4636 ----
*************** elf32_arm_build_stubs (struct bfd_link_i
*** 4696,4701 ****
--- 4727,4738 ----
    /* Build the stubs as directed by the stub hash table.  */
    table = &htab->stub_hash_table;
    bfd_hash_traverse (table, arm_build_one_stub, info);
+   if (htab->fix_cortex_a8)
+     {
+       /* Place the cortex a8 stubs last.  */
+       htab->fix_cortex_a8 = -1;
+       bfd_hash_traverse (table, arm_build_one_stub, info);
+     }
  
    return TRUE;
  }
Index: ld/testsuite/ld-arm/arm-elf.exp
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-arm/arm-elf.exp,v
retrieving revision 1.59
diff -c -3 -p -r1.59 arm-elf.exp
*** ld/testsuite/ld-arm/arm-elf.exp	22 May 2009 11:58:45 -0000	1.59
--- ld/testsuite/ld-arm/arm-elf.exp	4 Aug 2009 13:31:14 -0000
*************** set armelftests {
*** 205,210 ****
--- 205,214 ----
       "-EL -Ttext=0x8f00 --fix-cortex-a8" "-EL" {cortex-a8-thumb-target.s cortex-a8-fix-blx-rel.s}
       {{objdump -dr cortex-a8-fix-blx-rel-thumb.d}}
       "cortex-a8-fix-blx-rel-thumb"}
+     {"Cortex-A8 erratum fix, relocate bl.w and far call"
+      "-EL -Ttext=0x00 --fix-cortex-a8 --defsym far_fn1=0x80000000 --defsym far_fn2=0x80000004  --defsym far_fn=0x7fff0000 --defsym _start=0" "-EL" {cortex-a8-far-1.s cortex-a8-far-2.s}
+      {{objdump -dr cortex-a8-far.d}}
+      "cortex-a8-far"}
      {"Unwinding and -gc-sections" "-gc-sections" "" {gc-unwind.s}
       {{objdump -sj.data gc-unwind.d}}
       "gc-unwind"}
Index: ld/testsuite/ld-arm/cortex-a8-far-1.s
===================================================================
RCS file: ld/testsuite/ld-arm/cortex-a8-far-1.s
diff -N ld/testsuite/ld-arm/cortex-a8-far-1.s
*** /dev/null	1 Jan 1970 00:00:00 -0000
--- ld/testsuite/ld-arm/cortex-a8-far-1.s	4 Aug 2009 13:31:15 -0000
***************
*** 0 ****
--- 1,8 ----
+ 	.syntax unified
+ 	.thumb
+ 	.globl two
+ two:	
+ 	bl far_fn
+ 	.rept 0x200000
+ 	.long 0
+ 	.endr
Index: ld/testsuite/ld-arm/cortex-a8-far-2.s
===================================================================
RCS file: ld/testsuite/ld-arm/cortex-a8-far-2.s
diff -N ld/testsuite/ld-arm/cortex-a8-far-2.s
*** /dev/null	1 Jan 1970 00:00:00 -0000
--- ld/testsuite/ld-arm/cortex-a8-far-2.s	4 Aug 2009 13:31:15 -0000
***************
*** 0 ****
--- 1,20 ----
+ 	.syntax unified
+ 	.thumb
+ three:
+ 	bl far_fn1
+ 	bl far_fn2
+ 	.rept 1016
+ 	.long 0
+ 	.endr
+ 	nop
+ label1:	
+ 	eor.w   r0, r1, r2
+ 	beq.w     label1
+ 
+ 	eor.w   r0, r1, r2
+  
+ 	eor.w   r0, r1, r2
+ 	b.w     label1
+ 
+ 	eor.w   r0, r1, r2
+ 	eor.w   r0, r1, r2
Index: ld/testsuite/ld-arm/cortex-a8-far.d
===================================================================
RCS file: ld/testsuite/ld-arm/cortex-a8-far.d
diff -N ld/testsuite/ld-arm/cortex-a8-far.d
*** /dev/null	1 Jan 1970 00:00:00 -0000
--- ld/testsuite/ld-arm/cortex-a8-far.d	4 Aug 2009 13:31:15 -0000
***************
*** 0 ****
--- 1,40 ----
+ 
+ .*:     file format .*
+ 
+ 
+ Disassembly of section \.text:
+ 
+ 00000000 <two>:
+        0:	f000 c802 	blx	800008 <__far_fn_from_thumb>
+ 	...
+ #...
+ 00800008 <__far_fn_from_thumb>:
+   800008:	e51ff004 	ldr	pc, \[pc, #-4\]	; 80000c <__far_fn_from_thumb\+0x4>
+   80000c:	7fff0000 	.word	0x7fff0000
+ 
+ 00800010 <three>:
+   800010:	f001 e806 	blx	801020 <__far_fn1_from_thumb>
+   800014:	f001 e800 	blx	801018 <__far_fn2_from_thumb>
+ 	...
+   800ff8:	bf00      	nop
+ 
+ 00800ffa <label1>:
+   800ffa:	ea81 0002 	eor.w	r0, r1, r2
+   800ffe:	f000 b813 	b.w	801028 <__far_fn1_from_thumb\+0x8>
+   801002:	ea81 0002 	eor.w	r0, r1, r2
+   801006:	ea81 0002 	eor.w	r0, r1, r2
+   80100a:	f7ff bff6 	b.w	800ffa <label1>
+   80100e:	ea81 0002 	eor.w	r0, r1, r2
+   801012:	ea81 0002 	eor.w	r0, r1, r2
+ 	...
+ 
+ 00801018 <__far_fn2_from_thumb>:
+   801018:	e51ff004 	ldr	pc, \[pc, #-4\]	; 80101c <__far_fn2_from_thumb\+0x4>
+   80101c:	80000004 	.word	0x80000004
+ 
+ 00801020 <__far_fn1_from_thumb>:
+   801020:	e51ff004 	ldr	pc, \[pc, #-4\]	; 801024 <__far_fn1_from_thumb\+0x4>
+   801024:	80000000 	.word	0x80000000
+   801028:	d001      	beq.n	80102e <__far_fn1_from_thumb\+0xe>
+   80102a:	f7ff bfea 	b.w	801002 <label1\+0x8>
+   80102e:	f7ff bfe4 	b.w	800ffa <label1>

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