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] Bugfix: AVR Linker relaxation


Hi,

I have now found out a couple of issues that show up when using linker
relaxation with the present cvs state. The attached patch is supposed to fix
these bugs.

One problem was that the adjustment of changed addresses (address changes due
to the code-shrinking) were only applied in the .text section itself. It
could, however happen that there are references in the .data or .rodata
sections of the same module. E.g. when considering initialized function
pointer tables.
With the attached patch the "delete_bytes" function now looks for relocs in
all of the sections of an individual bfd.

The second issue that this patch addresses are PC relative expressions that
seemingly could be resolved at assembly time, but that could become invalid
when the code shrinks due to relaxation. With the patch gas is ordered to
always use relocs instead of fixups for these program memory related address
expressions.

The patch has been tested on several hardware projects without exhibiting
problems. Code size reduction amounted to 2-3 % for an 128k device and to
5-6% on a 16k device. The gas testsuite did not exhibit new regressions.

OK for mainline?

Bjoern

2006-04-19 ÂBjoern Haase Â<bjoern.m.haase@web.de>

ÂÂÂÂÂÂÂÂ* bfd/elf32-avr.c:
    elf32_avr_relax_delete_bytes:
	Iterate over all of the bfd's sections for reloc-addend adjustments.

ÂÂÂÂÂÂÂÂ* gas/config/tc-avr.h:
ÂÂÂÂÂÂÂÂTC_VALIDATE_FIX: add.

-------------------------------------------------------
Index: bfd/elf32-avr.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-avr.c,v
retrieving revision 1.24
diff -U12 -r1.24 elf32-avr.c
--- bfd/elf32-avr.c	3 Mar 2006 15:54:23 -0000	1.24
+++ bfd/elf32-avr.c	20 Apr 2006 07:10:59 -0000
@@ -1040,27 +1040,27 @@
 
 
 /* Enable debugging printout at stdout with a value of 1.  */
 #define DEBUG_RELAX 0
 
 /* Delete some bytes from a section while changing the size of an instruction.
    The parameter "addr" denotes the section-relative offset pointing just
    behind the shrinked instruction. "addr+count" point at the first
    byte just behind the original unshrinked instruction.  */
 
 static bfd_boolean
 elf32_avr_relax_delete_bytes (bfd *abfd,
-			      asection *sec,
+                              asection *sec,
                               bfd_vma addr,
-			      int count)
+                              int count)
 {
   Elf_Internal_Shdr *symtab_hdr;
   unsigned int sec_shndx;
   bfd_byte *contents;
   Elf_Internal_Rela *irel, *irelend;
   Elf_Internal_Rela *irelalign;
   Elf_Internal_Sym *isym;
   Elf_Internal_Sym *isymbuf = NULL;
   Elf_Internal_Sym *isymend;
   bfd_vma toaddr;
   struct elf_link_hash_entry **sym_hashes;
   struct elf_link_hash_entry **end_hashes;
@@ -1076,113 +1076,139 @@
   irelalign = NULL;
   toaddr = sec->size;
 
   irel = elf_section_data (sec)->relocs;
   irelend = irel + sec->reloc_count;
 
   /* Actually delete the bytes.  */
   if (toaddr - addr - count > 0)
     memmove (contents + addr, contents + addr + count,
              (size_t) (toaddr - addr - count));
   sec->size -= count;
 
-  /* Adjust all the relocs.  */
+  /* Adjust all the reloc addresses.  */
   for (irel = elf_section_data (sec)->relocs; irel < irelend; irel++)
     {
-      bfd_vma symval;
       bfd_vma old_reloc_address;
       bfd_vma shrinked_insn_address;
 
       old_reloc_address = (sec->output_section->vma
                            + sec->output_offset + irel->r_offset);
       shrinked_insn_address = (sec->output_section->vma
                               + sec->output_offset + addr - count);
 
       /* Get the new reloc address.  */
       if ((irel->r_offset > addr
            && irel->r_offset < toaddr))
         {
           if (DEBUG_RELAX)
             printf ("Relocation at address 0x%x needs to be moved.\n"
                     "Old section offset: 0x%x, New section offset: 0x%x \n",
                     (unsigned int) old_reloc_address,
                     (unsigned int) irel->r_offset,
                     (unsigned int) ((irel->r_offset) - count));
 
           irel->r_offset -= count;
         }
 
-      /* The reloc's own addresses are now ok. However, we need to readjust
-         the reloc's addend if two conditions are met:
-         1.) the reloc is relative to a symbol in this section that
-             is located in front of the shrinked instruction
-         2.) symbol plus addend end up behind the shrinked instruction.
-
-         This should happen only for local symbols that are progmem related.  */
-
-      /* Read this BFD's local symbols if we haven't done so already.  */
-      if (isymbuf == NULL && symtab_hdr->sh_info != 0)
-        {
-          isymbuf = (Elf_Internal_Sym *) symtab_hdr->contents;
-          if (isymbuf == NULL)
-            isymbuf = bfd_elf_get_elf_syms (abfd, symtab_hdr,
-                                            symtab_hdr->sh_info, 0,
-                                            NULL, NULL, NULL);
-          if (isymbuf == NULL)
-             return FALSE;
-         }
-
-      /* Get the value of the symbol referred to by the reloc.  */
-      if (ELF32_R_SYM (irel->r_info) < symtab_hdr->sh_info)
-        {
-          /* A local symbol.  */
-          Elf_Internal_Sym *isym;
-          asection *sym_sec;
-
-          isym = isymbuf + ELF32_R_SYM (irel->r_info);
-          sym_sec = bfd_section_from_elf_index (abfd, isym->st_shndx);
-          symval = isym->st_value;
-          /* If the reloc is absolute, it will not have
-             a symbol or section associated with it.  */
-          if (sym_sec)
-            {
-               symval += sym_sec->output_section->vma
-                         + sym_sec->output_offset;
-
-               if (DEBUG_RELAX)
-                printf ("Checking if the relocation's "
-                        "addend needs corrections.\n"
-                        "Address of anchor symbol: 0x%x \n"
-                        "Address of relocation target: 0x%x \n"
-                        "Address of relaxed insn: 0x%x \n",
-                        (unsigned int) symval,
-                        (unsigned int) (symval + irel->r_addend),
-                        (unsigned int) shrinked_insn_address);
+    }
 
-               if (symval <= shrinked_insn_address
-                   && (symval + irel->r_addend) > shrinked_insn_address)
-                 {
-                   irel->r_addend -= count;
+   /* The reloc's own addresses are now ok. However, we need to readjust
+      the reloc's addend, i.e. the reloc's value if two conditions are met:
+      1.) the reloc is relative to a symbol in this section that
+          is located in front of the shrinked instruction
+      2.) symbol plus addend end up behind the shrinked instruction.  
+      
+      The most common case where this happens are relocs relative to
+      the section-start symbol.
+         
+      This step needs to be done for all of the sections of the bfd.  */
+
+  {
+    struct bfd_section *isec;
+    for (isec = abfd->sections; isec; isec = isec->next)
+     {
+       bfd_vma symval;
+       bfd_vma shrinked_insn_address;
+
+       shrinked_insn_address = (sec->output_section->vma
+                                + sec->output_offset + addr - count);
+
+       irelend = elf_section_data (isec)->relocs + isec->reloc_count;
+       for (irel = elf_section_data (isec)->relocs; 
+            irel < irelend;
+            irel++)
+         {
+           /* Read this BFD's local symbols if we haven't done 
+              so already.  */
+           if (isymbuf == NULL && symtab_hdr->sh_info != 0)
+             {
+               isymbuf = (Elf_Internal_Sym *) symtab_hdr->contents;
+               if (isymbuf == NULL)
+                 isymbuf = bfd_elf_get_elf_syms (abfd, symtab_hdr,
+                                                 symtab_hdr->sh_info, 0,
+                                                 NULL, NULL, NULL);
+               if (isymbuf == NULL)
+                 return FALSE;
+             }
+
+           /* Get the value of the symbol referred to by the reloc.  */
+           if (ELF32_R_SYM (irel->r_info) < symtab_hdr->sh_info)
+             {
+               /* A local symbol.  */
+               Elf_Internal_Sym *isym;
+               asection *sym_sec;
+
+               isym = isymbuf + ELF32_R_SYM (irel->r_info);
+               sym_sec = bfd_section_from_elf_index (abfd, isym->st_shndx);
+               symval = isym->st_value;
+               /* If the reloc is absolute, it will not have
+                  a symbol or section associated with it.  */
+               if (sym_sec == sec)
+                 { 
+                   symval += sym_sec->output_section->vma
+                             + sym_sec->output_offset;
 
                    if (DEBUG_RELAX)
-                     printf ("Anchor symbol and relocation target bracket "
-                             "shrinked insn address.\n"
-                             "Need for new addend : 0x%x\n",
-                             (unsigned int) irel->r_addend);
+                     printf ("Checking if the relocation's "
+                             "addend needs corrections.\n"
+                             "Address of anchor symbol: 0x%x \n"
+                             "Address of relocation target: 0x%x \n"
+                             "Address of relaxed insn: 0x%x \n",
+                             (unsigned int) symval,
+                             (unsigned int) (symval + irel->r_addend),
+                             (unsigned int) shrinked_insn_address);
+
+                   if (symval <= shrinked_insn_address
+                       && (symval + irel->r_addend) > shrinked_insn_address)
+                     {
+                       irel->r_addend -= count;
+
+                       if (DEBUG_RELAX)
+                         printf ("Relocation's addend needed to be fixed \n");
+                     }
                  }
-            }
-	  /* else ... Reference symbol is absolute.  No adjustment needed.  */
-        }
-      /* else ... Reference symbol is extern. No need for adjusting the addend.  */
-    }
+               else
+                 {
+                   /* Reference symbol is absolute.  No adjustment needed.  */
+                 }
+              }
+            else
+              {
+                /* Reference symbol is extern. No need for adjusting 
+                   the addend.  */
+              }
+          }
+     }
+  }
 
   /* Adjust the local symbols defined in this section.  */
   isym = (Elf_Internal_Sym *) symtab_hdr->contents;
   isymend = isym + symtab_hdr->sh_info;
   for (; isym < isymend; isym++)
     {
       if (isym->st_shndx == sec_shndx
           && isym->st_value > addr
           && isym->st_value < toaddr)
         isym->st_value -= count;
     }
 
Index: gas/config/tc-avr.h
===================================================================
RCS file: /cvs/src/src/gas/config/tc-avr.h,v
retrieving revision 1.11
diff -U12 -r1.11 tc-avr.h
--- gas/config/tc-avr.h	12 Oct 2005 10:56:46 -0000	1.11
+++ gas/config/tc-avr.h	20 Apr 2006 07:11:01 -0000
@@ -111,12 +111,29 @@
 
 /* AVR port uses `$' as a logical line separator */
 #define LEX_DOLLAR 0
 
 /* An `.lcomm' directive with no explicit alignment parameter will
    use this macro to set P2VAR to the alignment that a request for
    SIZE bytes will have.  The alignment is expressed as a power of
    two.  If no alignment should take place, the macro definition
    should do nothing.  Some targets define a `.bss' directive that is
    also affected by this macro.  The default definition will set
    P2VAR to the truncated power of two of sizes up to eight bytes.  */
 #define TC_IMPLICIT_LCOMM_ALIGNMENT(SIZE, P2VAR) (P2VAR) = 0
+
+/* We don't want gas to fixup the following program memory related relocations.
+   We will need them in case that we want to do linker relaxation.
+   We could in principle keep these fixups in gas when not relaxing.
+   However, there is no serious performance penilty when making the linker
+   make the fixup work. */
+#define TC_VALIDATE_FIX(FIXP,SEG,SKIP)                      \
+if (FIXP->fx_r_type == BFD_RELOC_AVR_7_PCREL                \
+    || FIXP->fx_r_type == BFD_RELOC_AVR_13_PCREL            \
+    || FIXP->fx_r_type == BFD_RELOC_AVR_LO8_LDI_PM          \
+    || FIXP->fx_r_type == BFD_RELOC_AVR_HI8_LDI_PM          \
+    || FIXP->fx_r_type == BFD_RELOC_AVR_HH8_LDI_PM          \
+    || FIXP->fx_r_type == BFD_RELOC_AVR_16_PM)              \
+  {                                                         \
+     goto SKIP;                                             \
+  }
+
_______________________________________________
AVR-libc-dev mailing list
AVR-libc-dev@nongnu.org
http://lists.nongnu.org/mailman/listinfo/avr-libc-dev

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