Bug 14058 - Internal overflow error, on >128kB FLASH with no relaxation
Summary: Internal overflow error, on >128kB FLASH with no relaxation
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-03 18:55 UTC by Jan Waclawek
Modified: 2012-07-24 22:25 UTC (History)
2 users (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Waclawek 2012-05-03 18:55:25 UTC
If there are gs()'d labels both below and above the 0x20000 boundary, during linking without -relax an "internal overflow error" is thrown.

A simple minimum example is (filename y3a.S):
.text
.global   main
main:
  ret

.global K1
.org 0x100
.L1:
K1:
  nop
.global K2
.org 0x20002
.L2:
K2:
  nop

.section .progmem
.word gs(.L2)
.word gs(.L1) 

Using
"c:\Program Files\Atmel\AVRTools\WinAVR20100110\bin\avr-gcc" y3a.S -Os -DF_CPU=14745600UL -mmcu=atmega2561 -Wa,-adhlns=y3a.lst -Wl,-Map=y3a.map,--cref,--debug-stubs -o y3a.elf 

yields
Adding stub with destination 0x20108 to the hash table.
(Pre-Alloc run: 1)
Adding stub with destination 0x206 to the hash table.
(Pre-Alloc run: 1)
Allocating 1 entries in the AMT
Building one Stub. Address: 0x20110, Offset: 0x0
Final Stub section Size: 4
LD: Using jump stub (at 0x20000) with destination 0x2010c for reloc at address 0xcc.
C:\Users\OM7ZZ\AppData\Local\Temp/cc4xOnGI.o:(.progmem+0x0): warning: internal error: out of range error 

Confirmed to persist on avr-ld v2.22 too.

Further discussion at http://www.avrfreaks.net/index.php?name=PNphpBB2&file=viewtopic&p=707285#707285
Comment 1 Georg-Johann Lay 2012-05-03 20:13:06 UTC
See the comments in PR13812

If you use such big programs, .trampolines gets shifted out of it's 128KiB section.

Notice that the code will malfunction then because the assumption is that .trampolines is located in the 128 KiB segment where EIND points to; at least that is the assumption of GCC which you are obviously using.

Moreover, the actual position or .trampolines must not be messed up by code or linker script by shifting it out of place.

The default linker scripts are just a default; it won't and cannot any setup imagineable. If you need special arrangement for you code, you may want to arrange code and data according to your needs.

The inconvenience could be reduced by a more descriptive error message, yes, and the default ld script be arranged to suit better this situation. But then other issues might pop up more frequently like shifting .low_text or .progmem out of place.

For some explanation on linker stubs and gs(), see also

3.17.3.1 EIND and Devices with more than 128 Ki Bytes of Flash

http://gcc.gnu.org/onlinedocs/gcc/AVR-Options.html

*** This bug has been marked as a duplicate of bug 13812 ***
Comment 2 Jan Waclawek 2012-05-03 20:45:27 UTC
(In reply to comment #1)
> If you use such big programs, .trampolines gets shifted out of it's 128KiB
> section.

No, they don't; I posted relevant portions of disassembly in the link given above. Adding to that, relevant portion of .map:

 .trampolines   0x000000d0        0x4 linker stubs
 *(.trampolines*)
                0x000000d4                __trampolines_end = .

The key difference to the testcase in 13812 is that here .text is big, not .progmem. A quick look at the linker script reveals that .trampolines are above .progmem and below .text.

Please re-read the original report and the post at the given link.
Comment 3 Georg-Johann Lay 2012-05-03 21:57:19 UTC
(In reply to comment #2)

> Please re-read [...] the post at the given link.

Please make bug reports that are self-contained.

If there is information missing to reproduce the issue or to understand it, please add the information to render the bug report self-contained.

If the PR is already self-contained and includes all information, a link is simply distracting and confusing and superfluous.

Thanks.
Comment 4 Georg-Johann Lay 2012-05-03 22:03:18 UTC
Here is a reduced example:

; == foo.ss ==
.text

K1:  nop

.space 0x1fff6

K2:  nop

.section .progmem, "a", @progbits
.word gs(K1)
.word gs(K2)

== Assemble ==

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

== Link ==

$ avr-ld -m avr6 foo.o -o foo.elf -Map=foo.map

With .space 0x1fff4 the linker passes.

Is using linker stubs supported without --relax at all?
Comment 5 Jan Waclawek 2012-05-03 22:29:01 UTC
> Is using linker stubs supported without --relax at all?

While it is indeed possible to "administratively" suppress this bug by disallowing stubs without --relax, I believe this bug deserves to be fixed at some point as there may be "legal" reasons to avoid using --relax at times.

This said, I understand that it may be a difficult task, so the vague error message could be replaced by a more descriptive one (perhaps also suggesting usage of --relax) as an interim solution.
Comment 6 Jan Waclawek 2012-05-03 22:37:12 UTC
Hi Johann,

I think what you wrote there is a bit unfair.

The original bug report IMO contained everything needed to reproduce the bug and to assess it as different from the "trampolines high" case, without the need to read the avrfreaks.net thread. You shouldn't have edited out the "original post" from the quote in your response; that really changed the meaning of that sentence altogether.

Also, do you really believe I would post a bug report without searching for existing duplicate bug report? I thought my work, however informally presented, doesn't indicate such approach.


Moreover, I also believe that absolute placement of labels through .org (however deprecated it is for "real life" usage) is more illustrative of the underlying reason than your .space solution. Also the usage of --debug-stubs switch is not coincidental.

In the avrfreaks.net thread, there is further information I inferred from reading the sources, which might be useful for those who would attempt to fix the bug. It may be incorrect, of course, as I did not have time  to dig deep enough (read: I have to feed my family and as I said elsewhere already, I just tried to help others with their bugs and my code does not suffer from this bug due to my coding habits); that's why I did not push it futher. I believe those who are genuinely interested don't mind reading the post or whatever further information is available.


Have a nice day (or night)!

Jan






----- Original Message ---------------

Subject: [Bug ld/14058] Internal overflow error, on >128kB FLASH with norelaxation
   From: "gjl at gcc dot gnu.org" <sourceware-bugzilla@sourceware.org>
   Date: Thu, 03 May 2012 21:57:19 +0000
     To: konfera@efton.sk

>http://sourceware.org/bugzilla/show_bug.cgi?id=14058
>
>--- Comment #3 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2012-05-03 21:57:19 UTC ---
>(In reply to comment #2)
>
>> Please re-read [...] the post at the given link.
>
>Please make bug reports that are self-contained.
>
>If there is information missing to reproduce the issue or to understand it,
>please add the information to render the bug report self-contained.
>
>If the PR is already self-contained and includes all information, a link is
>simply distracting and confusing and superfluous.
>
>Thanks.
>
>-- 
>Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
>------- You are receiving this mail because: -------
>You reported the bug.
Comment 7 Georg-Johann Lay 2012-05-04 13:42:39 UTC
(In reply to comment #6)

> The original bug report IMO contained everything needed to reproduce the bug
> and to assess it as different from the "trampolines high" case, without the
> need to read the avrfreaks.net thread.
>
> [...]
> 
> Moreover, I also believe that absolute placement of labels through .org
> (however deprecated it is for "real life" usage) is more illustrative of the
> underlying reason than your .space solution. Also the usage of --debug-stubs
> switch is not coincidental.

The reduced example from comment #4 removes external dependencies like

* avr-gcc
* crtmxxx.o from AVR-LibC
* Command options passed like -Tdata=

Please note that GCC and AVR-LibC are not part of binutils, so that I worked out an example without these external dependencies.

Also note that /we/ want something from the binutils developers (I am not one of them), in particular to fix problems. They do not want something from /us/. It's us who need their help, knowledge, experience, time and pacience.

To help the developes fix a problem, we should not use external stuff, just as an #include <avr/io.h> or #include "lcd.h" might deter a GCC developer from tracking an avr-gcc issue. We should always strive to have no external dependencies or references and self-contained and comprehensible reports.

For a binutils developer, the difference between two targets is just to switch from --target=foo to --target=bar. Many stuff in binutils is /not/ AVR-related, so we basically exclude all developers that would help with avr-binutils problems that are not familiar with avr-gcc or want to install MS-windows distributions or build AVR-Libc. I just stripped external stuff in order to /not/ exclude any binutils developers.

As I played with the test case, I used .space just because I don't know the semantics of .org; in particular you linked against crt so that the very code startes after 0x100 and then there was an .org 0x100 in the code. So no drama here.

Concerning the relatedness to PR13812 it might very well be the case that I am plain wrong with my conclusions/"analysis" from there. It might be just as well be the case that it's completely unrelated to "shifting stubs out of segment" and instead is caused by "using gs() targeting different segments" as from this PR.
Comment 8 Jan Waclawek 2012-05-06 15:23:17 UTC
The bug is consequence of performing a relaxation-related optimisation step - reduction of the stubs table (.trampolines section) through removing stubs targeting labels which are directly reachable (i.e. are in the "segment" pointed by the default EIND) when --relax has not been specified. This step is performed in the elf32_avr_size_stubs() function in [binutils]/bfd/elf32-avr.c.

The stubs table is first created through calling elf32_avr_size_stubs() from avr_elf_${EMULATION_NAME}_before_allocation() in [binutils]/ld/emultempl/avrelf.em. It is called with the is_prealloc_run parameter set to TRUE, which results in the complete table being built containing all the targets of gs(). This step yields the maximal size of the stubs table (.trampolines section), which is then used in the initial output sections allocation.

When no relax, elf32_avr_size_stubs() is called second time from  avr_elf_after_allocation() (avrelf.em). The purpose of this is to fill the stubs table with instructions for jumps to the target addresses, which thus have to be known already (from the previous allocation). elf32_avr_size_stubs() is called with is_prealloc_run parameter set to FALSE, but that results in marking those stubs, which jump to "reachable" targets, as unused (note, that this is the only purpose of the is_prealloc_run parameter, so its name is misleading), thus the table's size might get reduced. As this results in moving the target labels in the subsequent address fixing step of the linker, the stub target addresses no longer match the real target addresses, which is then detected in avr_final_link_relocate() at labels R_AVR_LO8_LDI_GS:, R_AVR_HI8_LDI_GS: and R_AVR_16_PM: and the error is subsequently thrown.

(When --relax, the change of stubs section size is detected when calling elf32_avr_size_stubs() from elf32_avr_relax_section() and if changed, through setting *again invokes one more relaxation iteration which takes care of fixing the changed target addresses).

The fix is surprisingly simple:

+++ avrelf.em   Sun May  6 17:06:25 2012
@@ -152,7 +152,7 @@
     {
       /* If relaxing, elf32_avr_size_stubs will be called from
         elf32_avr_relax_section.  */
-      if (!elf32_avr_size_stubs (link_info.output_bfd, &link_info, FALSE))
+      if (!elf32_avr_size_stubs (link_info.output_bfd, &link_info, TRUE))
        einfo ("%X%P: can not size stub section: %E\n");
     }

So now, elf32_avr_size_stubs() for the non-relax case won't drop any entry thus the target addresses will remain correct.

It would be perhaps good also to rename the is_prealloc_run parameter of elf32_avr_size_stubs() to something more appropriate, e.g. no_resize or such.

Please note, that while this fixes a genuine bug, it also removes a "beneficial side effect" of the bug for those applications, which don't use --relax and at the same time don't have gs() targets above 0x20000 - so far they benefited from complete removal of the unneeded stubs table (which went unnoticed as then there were no incorrect stubs targets anymore). So, they now will experience both growth in size (as the stubs table is not removed anymore) and decreased execution speed (as indirect jumps will go through the stubs). As many of these chips are in fact used for relatively small application and use the big flash only for extensive data, this would impair a fair number of applications. In spite of that, I don't think it's necessary to implement any other fix for this other than change in documentation, which should recommend using the --no-stubs switch for those cases.
Comment 9 Sourceware Commits 2012-07-24 22:23:27 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	eweddington@sourceware.org	2012-07-24 22:23:21

Modified files:
	ld             : ChangeLog 
	ld/emultempl   : avrelf.em 

Log message:
	2012-07-24  Jan Waclawek <konfera@efton.sk>
	
	PR ld/14058
	* emultempl/avrelf.em (avr_elf_after_allocation): Call
	elf32_avr_size_stubs with is_prealloc_run as TRUE.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/ChangeLog.diff?cvsroot=src&r1=1.2468&r2=1.2469
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/emultempl/avrelf.em.diff?cvsroot=src&r1=1.11&r2=1.12
Comment 10 Eric Weddington 2012-07-24 22:25:44 UTC
Fixed with patch from Jan Waclawek.