Bug 13410 - [avr]: error: relocation truncated to fit: R_AVR_13_PCREL against symbol
Summary: [avr]: error: relocation truncated to fit: R_AVR_13_PCREL against symbol
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.24
: P2 normal
Target Milestone: ---
Assignee: Vidya Praveen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-14 22:29 UTC by Georg-Johann Lay
Modified: 2012-02-03 12:55 UTC (History)
4 users (show)

See Also:
Host:
Target: avr
Build:
Last reconfirmed:


Attachments
Correct the condition for qualifying candidates for relaxation (1.09 KB, patch)
2012-02-02 07:46 UTC, Vidya Praveen
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Georg-Johann Lay 2011-11-14 22:29:02 UTC
== Affected versions ==

At least 2.20 and 2.21

== source (foo.ss) ==

.text
.global main
.global sin

main:	ret

.section	.progmem.data,"a",@progbits

aaa:	.zero	3848


== Steps to reproduce ==

You need an avr-toolchain installation, i.e. avr-gcc, avr-binutils and avr-libc. I couln'd reproduce the error without linking against avr-libc

$ avr-as foo.ss -o foo.o -mmcu=atmega2560

$ avr-gcc -Wl,--cref -mrelax -Wl,-relax foo.o -mmcu=atmega2560 -o foo.elf -Wl,-Map=foo.map -lm

The bug depends on how big the functions in avr-libc are. If you do not see the bug (which is likely) do the following: Generate a list file:

$ avr-objdump -d foo.elf > foo.lst

The end list file will look like

...
0000133e <__stop_program>:
    133e: ff cf       rjmp .-2    ; 0x133e <__stop_program>

Adjust the size of aaa from foo.ss to that __stop_program will be located at address 0x133c.  In the example, you would have to decrease the size of aaa by 2.


== Full output from linker with -v -Wl,-v ==

> "make" XXX

avr-gcc  -Wl,--cref -mrelax -Wl,-relax foo.o -mmcu=atmega2560 -o foo.elf -Wl,-Map=foo.map -lm -v -Wl,-v

Using built-in specs.
COLLECT_GCC=e:/WinAVR/4.5.2/bin/avr-gcc
COLLECT_LTO_WRAPPER=e:/winavr/4.5.2/bin/../libexec/gcc/avr/4.5.2/lto-wrapper.exe
Target: avr
Configured with: ../gcc-4.5.2/configure --prefix=c:/mhvavrtools --host=i686-pc-mingw32 --target=avr --enable-languages=c,c++ --with-dwarf2 -enable-win32-registry=MHV-AVR-Tools --disable-nls --with-gmp=c:/mhvavrtools --with-mpfr=c:/mhvavrtools --with-mpc=c:/mhvavrtools --disable-libssp
Thread model: single
gcc version 4.5.2 (GCC) 
COMPILER_PATH=e:/winavr/4.5.2/bin/../libexec/gcc/avr/4.5.2/;e:/winavr/4.5.2/bin/../libexec/gcc/;e:/winavr/4.5.2/bin/../lib/gcc/avr/4.5.2/../../../../avr/bin/
LIBRARY_PATH=e:/winavr/4.5.2/bin/../lib/gcc/avr/4.5.2/avr6/;e:/winavr/4.5.2/bin/../lib/gcc/avr/4.5.2/../../../../avr/lib/avr6/;e:/winavr/4.5.2/bin/../lib/gcc/avr/4.5.2/;e:/winavr/4.5.2/bin/../lib/gcc/;e:/winavr/4.5.2/bin/../lib/gcc/avr/4.5.2/../../../../avr/lib/
COLLECT_GCC_OPTIONS='-mrelax' '-mmcu=atmega2560' '-o' 'foo.elf' '-v'
 e:/winavr/4.5.2/bin/../libexec/gcc/avr/4.5.2/collect2.exe --relax -m avr6 -Tdata 0x800200 -o foo.elf e:/winavr/4.5.2/bin/../lib/gcc/avr/4.5.2/../../../../avr/lib/avr6/crtm2561.o -Le:/winavr/4.5.2/bin/../lib/gcc/avr/4.5.2/avr6 -Le:/winavr/4.5.2/bin/../lib/gcc/avr/4.5.2/../../../../avr/lib/avr6 -Le:/winavr/4.5.2/bin/../lib/gcc/avr/4.5.2 -Le:/winavr/4.5.2/bin/../lib/gcc -Le:/winavr/4.5.2/bin/../lib/gcc/avr/4.5.2/../../../../avr/lib --cref -relax foo.o -Map=foo.map -lm -v -lgcc -lc -lgcc
collect2 version 4.5.2 (GNU assembler syntax)
e:/winavr/4.5.2/bin/../lib/gcc/avr/4.5.2/../../../../avr/bin/ld.exe --relax -m avr6 -Tdata 0x800200 -o foo.elf e:/winavr/4.5.2/bin/../lib/gcc/avr/4.5.2/../../../../avr/lib/avr6/crtm2561.o -Le:/winavr/4.5.2/bin/../lib/gcc/avr/4.5.2/avr6 -Le:/winavr/4.5.2/bin/../lib/gcc/avr/4.5.2/../../../../avr/lib/avr6 -Le:/winavr/4.5.2/bin/../lib/gcc/avr/4.5.2 -Le:/winavr/4.5.2/bin/../lib/gcc -Le:/winavr/4.5.2/bin/../lib/gcc/avr/4.5.2/../../../../avr/lib --cref -relax foo.o -Map=foo.map -lm -v -lgcc -lc -lgcc
GNU ld (GNU Binutils) 2.20.1.20100303
Comment 1 Georg-Johann Lay 2011-11-15 08:27:15 UTC
== Complete error message (here with 2.21) ==

GNU ld (GNU Binutils) 2.21 e:/winavr/4.7.0-1/bin/../lib/gcc/avr/4.7.0/../../../../avr/lib/avr6/crtm2560.o: In function `__vectors':
../../../../../../source/avr-libc-1.7.1/crt1/gcrt1.S:53: relocation truncated to fit: R_AVR_13_PCREL against symbol `__vector_1' defined in .text section in e:/winavr/4.7.0-1/bin/../lib/gcc/avr/4.7.0/../../../../avr/lib/avr6/crtm2560.o
collect2.exe: error: ld returned 1 exit status
Comment 2 Georg-Johann Lay 2012-01-29 18:38:03 UTC
CCed Jörg to ensure this is not a bug in avr-libc's startup code.
Comment 3 Vidya Praveen 2012-01-31 16:26:35 UTC
One of the two conditions that determines the candidates for relaxation 
(from JMP/CALL to rjmp/rcall),

            /* If the distance is within -4094..+4098 inclusive, then we can
               relax this jump/call.  +4098 because the call/jump target
               will be closer after the relaxation.  */
            if ((int) gap >= -4094 && (int) gap <= 4098)
              distance_short_enough = 1;

checks for the range of distance -4094..+4098 inclusive. There are two issues
here. 

The first issue is that the range should have been -4094 to +4097 (since we
shrink by 2 bytes, we can consider a longer range than the original -4096 to
+4095).

The second issue is the primary reason for the failure reported in this PR.
During the relaxation (elf32_avr_relax_section) of JMP/CALL to rjmp/rcall, some
cases where we don't want to modify the ordering, 'nop's are inserted instead 
of deleting bytes:

                if (!strcmp (sec->name,".vectors")
                    || !strcmp (sec->name,".jumptables"))
                  {
                    /* Let's insert a nop.  */
                    bfd_put_8 (abfd, 0x00, contents + irel->r_offset + 2);
                    bfd_put_8 (abfd, 0x00, contents + irel->r_offset + 3);
                  }
                else
                  {
                    /* Delete two bytes of data.  */
                    if (!elf32_avr_relax_delete_bytes (abfd, sec,
                                                       irel->r_offset + 2, 2))
                      goto error_return;

                    /* That will change things, so, we should relax again.
                       Note that this is not required, and it may be slow.  */
                    *again = TRUE;
                  }

While this is fine, the condition mentioned above that qualifies the candidates  
doesn't consider the situation where we don't shrink. That is, if the section is 
.vectors or .jumptables, we still check for -4094..+4098 range which assumes 
shrink. Causing larger offsets slipping through and which later found to be 
overflowing!

I have fixed this issue and the following patch should fix this. I have tested
with multiple testcases including the one that is presented here. 

OK for trunk?



2012-01-31  Vidya Praveen <childbear0@gmail.com>

	PR ld/13410
	* bfd/elf32-avr.c (elf32_avr_relax_section): Correct the range
	for qualifying candidates for relaxation

--- bfd/elf32-avr.c     2012-01-24 14:11:41.000000000 +0530
+++ bfd/elf32-avr.c     2012-01-31 20:30:26.000000000 +0530
@@ -1659,6 +1659,15 @@
   Elf_Internal_Sym *isymbuf = NULL;
   struct elf32_avr_link_hash_table *htab;

+  /* if true, do not shrink by deleting bytes while relaxing.Such shrinking can
+    can cause issues for sections such as .vectors and .jumptables.Instead fill
+    with nop instructions */
+  bfd_boolean shrinkable = TRUE;
+
+  if (!strcmp (sec->name,".vectors")
+      || !strcmp (sec->name,".jumptables"))
+    shrinkable = FALSE;
+
   if (link_info->relocatable)
     (*link_info->callbacks->einfo)
       (_("%P%F: --relax and -r may not be used together\n"));
@@ -1815,10 +1824,12 @@
             /* Compute the distance from this insn to the branch target.  */
             gap = value - dot;

-            /* If the distance is within -4094..+4098 inclusive, then we can
-               relax this jump/call.  +4098 because the call/jump target
+            /* If the distance is within -4094..+4097 inclusive, then we can
+               relax this jump/call.  +4097 because the call/jump target
                will be closer after the relaxation.  */
-            if ((int) gap >= -4094 && (int) gap <= 4098)
+            if (!shrinkable && ((int) gap >= -4096 && (int) gap <= 4095))
+              distance_short_enough = 1;
+            else if (shrinkable && ((int) gap >= -4094 && (int) gap <= 4097))
               distance_short_enough = 1;

             /* Here we handle the wrap-around case.  E.g. for a 16k device
@@ -1895,8 +1906,7 @@
                 /* Check for the vector section. There we don't want to
                    modify the ordering!  */

-                if (!strcmp (sec->name,".vectors")
-                    || !strcmp (sec->name,".jumptables"))
+                if (!shrinkable)
                   {
                     /* Let's insert a nop.  */
                     bfd_put_8 (abfd, 0x00, contents + irel->r_offset + 2);
Comment 4 Vidya Praveen 2012-01-31 17:20:39 UTC
posted the same to the mailing list with a mistake corrected in the change log (PR ld/13410 to PR bfd/13410). Sorry about the flowed format, I am trying to figure out how to disable that :-(

http://cygwin.com/ml/binutils/2012-01/msg00285.html

I can repost, if required.

~~~
Comment 5 Vidya Praveen 2012-01-31 19:52:24 UTC
Resubmitted:

http://sourceware.org/ml/binutils/2012-01/msg00286.html
Comment 6 Eric Weddington 2012-02-01 19:14:24 UTC
Hi Vidya,

Please resubmit patch with:

- Email address in the Changelog entry set to your Atmel address. If you don't have a personal FSF contribution form signed off, then you need to submit it under your Atmel address, because we (Atmel) have a corporate FSF assignment on file.

- Fix comments to be complete sentences, with capital letter on start, period at end of sentence and space after period and before the next sentence.

And please attach the file to this bug report.

Thanks!

Eric Weddington
Comment 7 Vidya Praveen 2012-02-02 06:17:35 UTC
(In reply to comment #6)
> Hi Vidya,
> Please resubmit patch with:
> - Email address in the Changelog entry set to your Atmel address. If you don't
> have a personal FSF contribution form signed off, then you need to submit it
> under your Atmel address, because we (Atmel) have a corporate FSF assignment on
> file.
> - Fix comments to be complete sentences, with capital letter on start, period
> at end of sentence and space after period and before the next sentence.
> And please attach the file to this bug report.
> Thanks!
> Eric Weddington

Thanks Eric. But is it ok if I use this gmail address for all the communications but have the Atmel ID mentioned in the ChangeLog? 

I'll attach the patch to this report, but should I place the ChangeLog as comment here or should I put it in the beginning of the patch file? 

VP
Comment 8 Vidya Praveen 2012-02-02 07:46:59 UTC
Created attachment 6188 [details]
Correct the condition for qualifying candidates for relaxation

Log Message:

2012-02-01  Vidya Praveen (vidya.praveen@atmel.com)

	PR bfd/13410
	* bfd/elf32-avr.c (elf32_avr_relax_section): Correct the 
	condition for qualifying the candidates for relaxation.
Comment 9 Vidya Praveen 2012-02-02 08:38:19 UTC
Corrected the log message

2012-02-02  Vidya Praveen (vidya.praveen@atmel.com)

	PR bfd/13410
	* bfd/elf32-avr.c (elf32_avr_relax_section): Correct the 
	condition that qualifies the candidates for relaxation.

Eric,
Please let me know if the resubmission is OK. Thanks

VP
Comment 10 cvs-commit@gcc.gnu.org 2012-02-02 18:02:17 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	eweddington@sourceware.org	2012-02-02 18:02:10

Modified files:
	bfd            : ChangeLog elf32-avr.c 

Log message:
	2012-02-02  Vidya Praveen (vidya.praveen@atmel.com)
	
	PR bfd/13410
	* bfd/elf32-avr.c (elf32_avr_relax_section): Correct the
	condition that qualifies the candidates for relaxation.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/ChangeLog.diff?cvsroot=src&r1=1.5598&r2=1.5599
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf32-avr.c.diff?cvsroot=src&r1=1.52&r2=1.53
Comment 11 Eric Weddington 2012-02-02 18:05:34 UTC
Vidya,

You did fine, putting the patch in the bug report as attachment, and ChangeLog in comment. Yes, you can use your gmail account for communication, but then use your atmel account for the ChangeLog entry.

Patch committed, after addition of a couple of periods at end of comment sentences.

Thanks for fixing this bug, Vidya.
Comment 12 Georg-Johann Lay 2012-02-02 19:29:31 UTC
The patch reads:

+  /* If 'shrinkable' is FALSE, do not shrink by deleting bytes while
+     relaxing. Such shrinking can cause issues for the sections such 
+     as .vectors and .jumptables. Instead the unused bytes should be 
+     filled with nop instructions. */
+  bfd_boolean shrinkable = TRUE;
+
+  if (!strcmp (sec->name,".vectors")
+      || !strcmp (sec->name,".jumptables"))
+    shrinkable = FALSE;

Shouln't this also cover .jumptables* and .progmem.gcc* ?

avr-gcc puts its jumptables into .progmem.gcc_sw_table for instance.

Relaxing must not do harm to them.
Comment 13 Vidya Praveen 2012-02-03 08:30:31 UTC
(In reply to comment #12)
> The patch reads:
> +  /* If 'shrinkable' is FALSE, do not shrink by deleting bytes while
> +     relaxing. Such shrinking can cause issues for the sections such 
> +     as .vectors and .jumptables. Instead the unused bytes should be 
> +     filled with nop instructions. */
> +  bfd_boolean shrinkable = TRUE;
> +
> +  if (!strcmp (sec->name,".vectors")
> +      || !strcmp (sec->name,".jumptables"))
> +    shrinkable = FALSE;
> Shouln't this also cover .jumptables* and .progmem.gcc* ?
> avr-gcc puts its jumptables into .progmem.gcc_sw_table for instance.
> Relaxing must not do harm to them.

I didn't analyze the which other sections that might require this, however I assumed such requirements will be there. This is one of the reason why I moved out the check outside the loop, so the such additions are easy and in one place. Are there any other sections that might require to be here?
Comment 14 Vidya Praveen 2012-02-03 08:40:08 UTC
(In reply to comment #11)
> Vidya,
> You did fine, putting the patch in the bug report as attachment, and ChangeLog
> in comment. Yes, you can use your gmail account for communication, but then use
> your atmel account for the ChangeLog entry.
> Patch committed, after addition of a couple of periods at end of comment
> sentences.
> Thanks for fixing this bug, Vidya.

Thanks Eric.
Comment 15 Georg-Johann Lay 2012-02-03 12:55:47 UTC
(In reply to comment #13)
> (In reply to comment #12)
> > +  if (!strcmp (sec->name,".vectors")
> > +      || !strcmp (sec->name,".jumptables"))
> > +    shrinkable = FALSE;
> > Shouln't this also cover .jumptables* and .progmem.gcc* ?
> > avr-gcc puts its jumptables into .progmem.gcc_sw_table for instance.
> > Relaxing must not do harm to them.
> 
> I didn't analyze the which other sections that might require this, however I
> assumed such requirements will be there. This is one of the reason why I moved
> out the check outside the loop, so the such additions are easy and in one
> place. Are there any other sections that might require to be here?

My bad, I answered too fast. Swtich-case lookup tables from avr-gcc have always 16-bit entries, even on the big devices. Sorry for the noise.