[Bug ld/13899] New: [avr]: Wrong relaxation of R_AVR_16_PM with gs()

http://sourceware.org/bugzilla/show_bug.cgi?id=13899

             Bug #: 13899
           Summary: [avr]: Wrong relaxation of R_AVR_16_PM with gs()
           Product: binutils
           Version: 2.23 (HEAD)
            Status: NEW
          Severity: normal
          Priority: P2
         Component: ld
        AssignedTo: unassigned@...
        ReportedBy: gjl@...
                CC: eric.weddington@...
    Classification: Unclassified
            Target: avr


Created attachment 6301
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6301
bug-relax.zip: Files to reproduce

The linker computes wrong values for gs() initializers like from text.s:

.global    menuitem_text
    .section    .progmem.data,"a",@progbits
    .type    menuitem_text, @object
    .size    menuitem_text, 14
menuitem_text:
    .word    gs(onPixel_text)
    .word    gs(onRedraw_text)
    .word    gs(onKey_text)
    .word    gs(onEnter_text)

I tried to work out a small example, but with no avail.
So I post the whole project.

Steps to reproduce:

1) Unpack the attached zip file. It inflates to ./bug-relax/

2) cd ./bug-relax

3) There are assembler sources in the directory and a small Makefile. Run
   make obj all

4) This generates two files bug.elf and bug.lst
   bug.lst is diff'ed against good.lst generated with avr-ld 2.21

All in all, the "make obj all" will print something like

avr-as -mmcu=atmega168 frac8.ss -o frac8-asm.o
avr-as -mmcu=atmega168 parith-16.ss -o parith-16-asm.o
...
avr-ld -m avr5 -Tdata 0x800100 crtm168.o -v -o bug.elf --relax ...
GNU ld (GNU Binutils) 2.22.52.20120322

diff -u1 good.lst bug.lst

-good.elf:     file format elf32-avr
+bug.elf:     file format elf32-avr

@@ -81,3 +81,3 @@
 0000008e <menuitem_text>:
-      8e:    78 06 23 06 af 05 18 06 19 01 28 04 4f 01
+      8e:    7c 06 24 06 af 05 18 06 19 01 28 04 4f 01

@@ -163,3 +163,3 @@
 00000299 <menuitem_asteroids>:
-     299:    61 0c 6a 0d 1a 0b de 0a 8f 02 08 02 e8 02
+     299:    70 0c 7f 0d 1b 0b de 0a 8f 02 08 02 e8 02

@@ -217,3 +217,3 @@
 000003ae <menuitem_snake>:
-     3ae:    92 11 7d 10 24 0f 58 0f 6d 03 52 02 bc 03
+     3ae:    92 11 7f 10 24 0f 58 0f 6d 03 52 02 bc 03

@@ -263,3 +263,3 @@
 00000491 <menuitem_schoner>:
-     491:    9a 14 e3 13 18 14 09 14 02 01 00 08 9f 04
+     491:    9b 14 e3 13 18 14 09 14 02 01 00 08 9f 04


In text.s: menuitem_text the first enty is
   .word    gs(onPixel_text)

This function is located at word address 0x0678:
   00000cf0 <onPixel_text>

But bug.lst has 0x67c as printed in the diff above.

--
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

_______________________________________________
bug-binutils mailing list
bug-binutils@...
https://lists.gnu.org/mailman/listinfo/bug-binutils

http://sourceware.org/bugzilla/show_bug.cgi?id=13899

--- Comment #1 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2012-03-28 11:51:22 UTC ---
Notes

If you work on linux, you will want to

$ dos2unix good.lst

in order to change newlines to unix style.

And in the Makefile, diff results get smaller if objdump gets called with -d
instead of with -S like so:

exe:    
    $(LD) -o $(EXE).elf --relax $(OBJ) -L. -lgcc -lc -lgcc
    $(OBJDUMP) -z -h -d -j .data -j .text $(EXE).elf > $(EXE).lst

_________________________^^ -d instead of -S

--
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

_______________________________________________
bug-binutils mailing list
bug-binutils@...
https://lists.gnu.org/mailman/listinfo/bug-binutils

http://sourceware.org/bugzilla/show_bug.cgi?id=13899

Alastair D'Silva <sourceware@...> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sourceware@...

--
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

_______________________________________________
bug-binutils mailing list
bug-binutils@...
https://lists.gnu.org/mailman/listinfo/bug-binutils

http://sourceware.org/bugzilla/show_bug.cgi?id=13899

--- Comment #2 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2012-04-27 23:50:14 UTC ---
Created attachment 6379
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6379
uhr.s

Here is a condensed test case.

The first entry of menuitem_uhr shall contain the word address of onPixel_uhr:

.global    menuitem_uhr
    .section    .progmem.data,"a",@progbits
    .type    menuitem_uhr, @object
menuitem_uhr:
    .word    gs(onPixel_uhr)

But avr-ld generates a wrong entry 0x000a instead of the correct 0x0007.

With the attached assembler file, perform

$ avr-as -mmcu=atmega168 uhr.s -o uhr.o
$ avr-ld -o bug13899.elf -m avr5 -Tdata 0x800100 --relax -defsym morse=0 -v
uhr.o
$ avr-objdump -h -S -j .data -j .text bug13899.elf


The output is:

GNU ld (GNU Binutils) 2.22.52.20120322

bug13899.elf:     file format elf32-avr

Sections:
Idx Name          Size      VMA       LMA       File off  Algn
  0 .text         00000016  00000000  00000000  00000054  2**1
                  CONTENTS, ALLOC, LOAD, READONLY, CODE

Disassembly of section .text:

00000000 <menuitem_uhr>:
   0:    0a 00 06 00 04 00 04 00                             ........

00000008 <__ctors_end>:
   8:    fb df           rcall    .-10         ; 0x0 <menuitem_uhr>

0000000a <onRedraw_uhr>:
   a:    fa df           rcall    .-12         ; 0x0 <menuitem_uhr>
   c:    f9 df           rcall    .-14         ; 0x0 <menuitem_uhr>

0000000e <onPixel_uhr>:
   e:    f8 df           rcall    .-16         ; 0x0 <menuitem_uhr>
  10:    f7 df           rcall    .-18         ; 0x0 <menuitem_uhr>
  12:    f6 df           rcall    .-20         ; 0x0 <menuitem_uhr>
  14:    f5 df           rcall    .-22         ; 0x0 <menuitem_uhr>

The first entry of menuitem_uhr is 0x000a.
The correct value is 0x0007 = 0xe / 2, i.e. the word address of menuitem_uhr.

--
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

_______________________________________________
bug-binutils mailing list
bug-binutils@...
https://lists.gnu.org/mailman/listinfo/bug-binutils

http://sourceware.org/bugzilla/show_bug.cgi?id=13899

--- Comment #3 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2012-04-28 12:33:28 UTC ---
(In reply to comment #2)

>
> Disassembly of section .text:
>
> 00000000 <menuitem_uhr>:
>    0:    0a 00 [...]
>
> 0000000e <onPixel_uhr>:
>
> The first entry of menuitem_uhr is 0x000a. The correct value
> is 0x0007 = 0xe / 2, i.e. the word address of menuitem_uhr.

_______________________________________________^^^^^^^^^^^^^^^

The word address of onPixel_uhr, of course.



Note: The testcase can be simplified even more to:

.text
    call onPixel_uhr
onPixel_uhr:
    nop
.section .progmem.data,"a",@progbits
.type menuitem_uhr, @object
menuitem_uhr:
    .word gs(onPixel_uhr)

And then assembled/linked/dumped

$ avr-as -mmcu=atmega168 uhr.s -o uhr.o
$ avr-ld -o bug13899.elf -m avr5 --relax uhr.o
$ avr-objdump -d bug13899.elf

Disassembly of section .text:

00000000 <menuitem_uhr>:
   0:    03 00                                               ..

00000002 <__ctors_end>:
   2:    00 d0           rcall    .+0          ; 0x4 <onPixel_uhr>

00000004 <onPixel_uhr>:
    ...

The word address of onPixel_uhr is 0x0002, but menuitem_uhr recorded 0x0003.

The error also occurs if menuitem_uhr is located in .data instead of in section
.progmem.data

--
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

_______________________________________________
bug-binutils mailing list
bug-binutils@...
https://lists.gnu.org/mailman/listinfo/bug-binutils

http://sourceware.org/bugzilla/show_bug.cgi?id=13899

Jan Waclawek <konfera at efton dot sk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |konfera at efton dot sk

--- Comment #4 from Jan Waclawek <konfera at efton dot sk> 2012-05-09 07:59:38 UTC ---
During relax, if a call is shrunk to rcall, the relocs are modified accordingly
in elf32_avr_relax_delete_bytes(). This assumes that the relocs in case are
permanently kept in memory, which apparently ceased to be the case somewhere
between version 2.21 and 2.22 for reasons to me unknown. This has been noticed
also as http://sourceware.org/bugzilla/show_bug.cgi?id=13612, but "fix" for
http://sourceware.org/bugzilla/show_bug.cgi?id=12161 covered this up, by
loading the relocs, but they were not loaded permanently (keep-memory parameter
is FALSE).

Note that this did not occur with avr6 architecture, where stubs section is
created (unless --no-stubs is used); again I don't quite understand the
mechanisms behind this (sidenote: gs() is not appropriate for avr5
architecture).

The remedy is again surprisingly simple: _bfd_elf_link_read_relocs() has to be
called with keep-memory TRUE. (The below patch removes also the unneeded free()
and adds a couple of diagnostic printouts). While will probably increase memory
usage, in typical programs where multiple shrinks occur it should decrease the
linking time.

--- ../../elf32-avr.c    Mon May  7 05:27:50 2012
+++ elf32-avr.c    Wed May  9 09:12:47 2012
@@ -1513,7 +1513,11 @@
        irel = elf_section_data (isec)->relocs;
        /* PR 12161: Read in the relocs for this section if necessary.  */
        if (irel == NULL)
-     irel = _bfd_elf_link_read_relocs (abfd, isec, NULL, NULL, FALSE);
+       {
+         irel = _bfd_elf_link_read_relocs (abfd, isec, NULL, NULL, TRUE);
+         if (debug_relax)
+           printf ("Relocs read in for %sec \n", isec->name);
+       }

        for (irelend = irel + isec->reloc_count;
             irel < irelend;
@@ -1573,8 +1577,6 @@
           the addend.  */
      }

-       if (elf_section_data (isec)->relocs == NULL)
-     free (irelend - isec->reloc_count);
      }
   }

@@ -2276,6 +2278,8 @@
   return TRUE;

  error_return:
+  if (debug_relax)
+    printf ("Something went wrong during relaxing...\n");
   if (isymbuf != NULL
       && symtab_hdr->contents != (unsigned char *) isymbuf)
     free (isymbuf);

--
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

_______________________________________________
bug-binutils mailing list
bug-binutils@...
https://lists.gnu.org/mailman/listinfo/bug-binutils

http://sourceware.org/bugzilla/show_bug.cgi?id=13899

--- Comment #5 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> 2012-07-24 21:44:48 UTC ---
CVSROOT:    /cvs/src
Module name:    src
Changes by:    eweddington@...    2012-07-24 21:44:44

Modified files:
    bfd            : ChangeLog elf32-avr.c

Log message:
    2012-07-24  Jan Waclawek <konfera@...>
    PR 13899
    * elf32-avr.c (elf32_avr_relax_delete_bytes): Call
    _bfd_elf_link_read_relocs with keep_memory as TRUE.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/ChangeLog.diff?cvsroot=src&r1=1.5755&r2=1.5756
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf32-avr.c.diff?cvsroot=src&r1=1.57&r2=1.58

--
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

_______________________________________________
bug-binutils mailing list
bug-binutils@...
https://lists.gnu.org/mailman/listinfo/bug-binutils

http://sourceware.org/bugzilla/show_bug.cgi?id=13899

Eric  Weddington <eric.weddington at atmel dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #6 from Eric  Weddington <eric.weddington at atmel dot com> 2012-07-24 21:48:15 UTC ---
Fixed with with patch from Jan Waclawek.

--
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

_______________________________________________
bug-binutils mailing list
bug-binutils@...
https://lists.gnu.org/mailman/listinfo/bug-binutils