Re: [avr-gcc-list] Linker script patch with __flashN size checking. [Was: Handling __flash1 and .trampolines]

[Erik Christiansen]

On 16.12.12 15:03, Georg-Johann Lay wrote:
> Attached you find my latest draft patch.
,,,

> == Caveats, new problems and problems it does not solve ==
> 
> - It is not possible to bump the location only if there is a need to
> do so, i.e. if .progmemN is non-empty.  This wastes memory and blows
> the unconditionally blows binaries as it bumps data and text orphans
> to 0x30000 or higher, cf. [1].

That is only a limitation of that script attempt.
Please test my appended tested linker script.
It does not have these problems.

Please check the test results, also appended. You will see that it is
quite possible to "bump the location only if there is a need", given
sufficient experience with linker scripts.

> - The .progmemN stuff is handled within .text so that there is no
> need to specify -j .flashN with objcopy.

That is not a linking or locating requirement. Invocation of objcopy is
properly done in makefiles, and the benefits of a well structured linker
script ought not be lost just because a commandline parameter is needed.
If so, we could never have a -Dxxxx on avr-gcc.

> - If the case of an overflow there is just a fixed text massage
> without an indication on how much the sections overflow.  The user
> must trial-and-error change the source until she finds a
> configuration that passes all assertions, cf. [1] again.

That is just a limitation of your approach. With my linker script
mirroring the desired memory model, the user can immediately read the
size of overflow of any of the sections, just by looking in the map
file. Please see the test results at the end of this post for examples.
Run the test for yourself. (It uses your nifty test harness. :-):

   $ avr-gcc -T avr6.x-new -Wl,-Map,flash.map -o flash.elf -DSTUBS=10 \
     -DP0=0x20000 -DTEXT=0x20000 -mmcu=atmega2560 flash.sx

   Error: .lowtext (128KiB limit) overflow. Try shrinking .progmem?

AND IN flash.map:

    0x0002010c     _elowtext = .
    0x00000000     x = ASSERT ((. <= 0x20000), Error: .lowtext (128KiB limit)
                   overflow. Try shrinking .progmem?)

The amount of overflow is 10c bytes. There is no "trial-and-error
change" required, is there?

Our custom error message only mentions .progmem because that is the part
of .lowtext which is most amenable to user discretion. It can be harder
to shrink .trampolines, AIUI. (In the event of .progmem bloat, whack in
a __flashN, high enough to dodge 128KiB, and shovel some ,progmem.data
over to .progmem2.data, and the job is done.)

> - There is no test for progmem.  I don't know how .progmem is used.
> If it's common practice to use GET_FAR_ADDRESS or similar kludge with
> progmem, there must be no assertion or otherwise it will break
> existing code.  I guess Jan does know?

Please specify the requirement, and I will implement it.
My script already tests the low 128KiB limit. That is what was just done
above. And .progmem is in that 128 KiB, along with .trampolines, etc.
It is the sum of these which overflow the 128 KiB limit, not just
.progmem, is it not? In that case it is already handled, AIUI.
(And readily tweaked, if new requirements arise.)

> - size will report insanely high values because the location is
> bumped unconditionally, i.e. even for a trivial program the size will
> be at least 0x30000.

A naive linker script often has problems.

Please check my linker script against the appended tests and results.
In each case, the code is well compacted, without the holes which are
giving you problems. Look at the output of avr-objdump -h.

> - It is no more possible to (with the new flash.sx)
> 
>      -Ttext=0x20000 -DBOOT=10 -Wl,--section-start=.bootloader=0x28000

Although no effort has been expended on it, my script handles that
previously untested requirement:

$ avr-gcc -T avr6.x-new -Wl,-Map,flash.map -o flash.elf -DSTUBS=10
-DTEXT=0x20000 -DBOOT=10 -Wl,--section-start=.bootloader=0x28000
-mmcu=atmega2560 flash.sx

The compilation proceeds error-free, and .bootloader is correctly
located. The low 128 KiB (.lowtext) is good, and .hightext (the stuff
which is allowed to move up when __flashN is invoked, immediately
follows. Then .data. No wasted space - all looks as it should be:

   Idx Name          Size      VMA       LMA       File off  Algn
  0 .data         00000000  00800200  0002016e  00020202  2**0
                  CONTENTS, ALLOC, LOAD, DATA
  1 .bootloader   0000000a  00028000  00028000  00020202  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
  2 .lowtext      0000010c  00000000  00000000  00000094  2**1
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
  3 .hightext     00020062  0000010c  0000010c  000001a0  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, CODE

If there is anything else you need, please ask.

>   if one wants to place an orphan.  The linker will throw:
> 
>      ld.exe: section .bootloader loaded at [00028000,00028009]
>      overlaps section .text loaded at [00020000,0002ffff]
> 
>   and similar without -Ttext=

Please provide test command lines. Orphan handling in my linker script
remains as in the old script, but I do not expect to be troubled by the
possible need for tweaking for new test cases.

> - Diagnostic messages are printed twice.

That is an aesthetic nuisance, I think. Ld's automatic erroring can be
awkward to make a Mona Lisa from, but we will persevere. Better that a
user is told twice that he has an error, than not at all.

> The script moves a comment up that says ctors / dtors is needed in
> the lower 128Ki.  I don't see a requirement for that.  ctors goes
> like that:
> 
> __do_global_ctors reads stubs locations from .ctors, i.e. .ctors
> contains an array of gs() stubs.  __do_global_ctors handles RAMPZ
> correctly while it reads from .ctors (by CALLing __tablejump_elpm__
> so that the ctor will RET to __do_global_ctors.  __tablejump_elpm__
> performs an EIJMP that targets a stub in .trampolines which JUMPs to
> the ctor function.
> 
> Ditto for dtors.
> 

Please run such tests on my working script. If any results are
unsatisfactory, it will not be difficult to tweak, I expect.

> == Conclusion ==
> 
> All in all, the current draft script is too intrusive and has too
> many negative side effects on existing code.

I'm sorry that the attempt is not working for you. It is hard to do
everything, especially when the skills in one area have not been
developed. We cannot do what you do on avr-gcc - the learning curve is
too frustrating for too long, before we become productive.

Please consider letting more experienced contributors handle the linker
scripts. It will ease your frustration, and enhance the community's
productivity.

> .progmemx will work smooth and can be added without problems, same
> for the more restrict .progmem pattern.  avr-gcc can be extended to
> put __memx into .progmemx.data instead of in .progmem.data which will
> both work with the old and a new script (but with a new script it
> will work better because no low precious locations are consumed).

We can do that when my handling of __flashN is thoroughly tested,
preferably by a number of users.
AIUI, neither is in use yet, anyway?

> The linker script language appears to be too weak to express what's
> needed and without disturbing existing code in an inappropriate way.

A tool is only as powerful as the wielder. Experience is required,
if good results are to be achieved.

> I extended and attached the flash.sx torture source with the
> following defines:
> 
> PX      : Put data into .progmemx (prior to .progmem1)
> BOOT    : Put data into some "ax" orphan section (.bootloader)
> CTORS   : Add some constructors

Thank you. That is great teamwork. I used the middle one to test the
.bootloader case.

...

> 
> >file: avr6.patch
> >
> >--- avr6.x-2.23      2012-12-13 13:42:18.000000000 +1100
> >+++ avr6.x-new       2012-12-15 21:55:31.000000000 +1100
> >@@ -70,13 +70,12 @@
> >   .rel.plt       : { *(.rel.plt)            }
> >   .rela.plt      : { *(.rela.plt)           }
> >   /* Internal text space or external memory.  */
> >-  .text   :
> >+  .lowtext :
> 
> Why that? I don't think we suppose people to objcopy -j .lowtext or
> otherwise their binaries will miss essential parts.

I'll outline the benefits of a more structured linker script, mirroring
the desired memory model, in another post. This one is cluttered with
problems from your script attempt.
...

> >+    _elowtext = . ;
> >+    x = ASSERT (. <= 0x10000, "Error: .lowtext (128KiB limit) overflow. Try 
> >shrinking .progmem?") ;
> 
> Message does not fit assertion.

Fixed and tested in a later version, appended below.

> Anyway, I really don't know if users abuse .progmem to drop their
> non-volatile data that exceeds 64Ki and use GET_FAR_ADDRESS on that.

The .lowtext output section overflow detection picks up any cause,
including too many stubs in .trampolines. The test is a requirement
which you posted upthread. I have just implemented what you asked for.

> morepgmspace.h says nothing about how to locate data and is not
> supported by avr-libc, see avr-libc!/include/avr which only brings
> pgmspace.h.
> 
> The GCC docs [3] promise that
> 
>  "This attribute [progmem] works similar to the section attribute
>   but adds additional checking."
> 
> so that an additional sanity checking should be in order.

Much vagueness. Please specify a requirement.

...

> >   }  > text
> >-  .data       : AT (ADDR (.text) + SIZEOF (.text))
> >+
> >+  .data       : AT (_etext)
> 
> Why _etext?  This does not take into account orphans, or does it?

You might like to run a test of my script with orphans. It just does
what the old script did, in that regard.

If the equivalent syntax is disconcerting, I can use ADDR and SIZEOF to
the same effect. It was just convenient to use _etext while developing
the new script. Either way works equally well, so far.

Some lists don't accept attachments, so I've just appended the two files
in-line. Hopefully that's the preferred way.

Happy testing.

Erik

file: avr6.x_new

/* Default linker script, for normal executables */
OUTPUT_FORMAT("elf32-avr","elf32-avr","elf32-avr")
OUTPUT_ARCH(avr:6)
MEMORY
{
  text   (rx)   : ORIGIN = 0, LENGTH = 1024K
  data   (rw!x) : ORIGIN = 0x800200, LENGTH = 0xfe00
  eeprom (rw!x) : ORIGIN = 0x810000, LENGTH = 64K
  fuse      (rw!x) : ORIGIN = 0x820000, LENGTH = 1K
  lock      (rw!x) : ORIGIN = 0x830000, LENGTH = 1K
  signature (rw!x) : ORIGIN = 0x840000, LENGTH = 1K
}
SECTIONS
{
  /* Read-only sections, merged into text segment: */
  .hash          : { *(.hash)           }
  .dynsym        : { *(.dynsym)         }
  .dynstr        : { *(.dynstr)         }
  .gnu.version   : { *(.gnu.version)    }
  .gnu.version_d   : { *(.gnu.version_d)        }
  .gnu.version_r   : { *(.gnu.version_r)        }
  .rel.init      : { *(.rel.init)               }
  .rela.init     : { *(.rela.init)      }
  .rel.text      :
    {
      *(.rel.text)
      *(.rel.text.*)
      *(.rel.gnu.linkonce.t*)
    }
  .rela.text     :
    {
      *(.rela.text)
      *(.rela.text.*)
      *(.rela.gnu.linkonce.t*)
    }
  .rel.fini      : { *(.rel.fini)               }
  .rela.fini     : { *(.rela.fini)      }
  .rel.rodata    :
    {
      *(.rel.rodata)
      *(.rel.rodata.*)
      *(.rel.gnu.linkonce.r*)
    }
  .rela.rodata   :
    {
      *(.rela.rodata)
      *(.rela.rodata.*)
      *(.rela.gnu.linkonce.r*)
    }
  .rel.data      :
    {
      *(.rel.data)
      *(.rel.data.*)
      *(.rel.gnu.linkonce.d*)
    }
  .rela.data     :
    {
      *(.rela.data)
      *(.rela.data.*)
      *(.rela.gnu.linkonce.d*)
    }
  .rel.ctors     : { *(.rel.ctors)      }
  .rela.ctors    : { *(.rela.ctors)     }
  .rel.dtors     : { *(.rel.dtors)      }
  .rela.dtors    : { *(.rela.dtors)     }
  .rel.got       : { *(.rel.got)                }
  .rela.got      : { *(.rela.got)               }
  .rel.bss       : { *(.rel.bss)                }
  .rela.bss      : { *(.rela.bss)               }
  .rel.plt       : { *(.rel.plt)                }
  .rela.plt      : { *(.rela.plt)               }
  /* Internal text space or external memory.  */
  .lowtext :
  {
    *(.vectors)
    KEEP(*(.vectors))
    /* For data that needs to reside in the lower 64k of progmem.  */
    *(.progmem.gcc*)
    . = ALIGN(2);
     __trampolines_start = . ;
    /* The jump trampolines for the 16-bit limited relocs will reside here.  */
    *(.trampolines)
    *(.trampolines*)
     __trampolines_end = . ;
    /* For future tablejump instruction arrays for 3 byte pc devices.
       We don't relax jump/call instructions within these sections.  */
    *(.jumptables)
    *(.jumptables*)
    /* For code that needs to reside in the lower 128k progmem.  */
    *(.lowtext)
    *(.lowtext*)
     __ctors_start = . ;
     *(.ctors)
     __ctors_end = . ;
     __dtors_start = . ;
     *(.dtors)
     __dtors_end = . ;
    KEEP(SORT(*)(.ctors))
    KEEP(SORT(*)(.dtors))
    *(.progmem.data*)    /* Explicitly page 0 input sections */
    _elowtext = . ;
    x = ASSERT (. <= 0x20000, "Error: .lowtext (128KiB limit) overflow. Try 
shrinking .progmem?") ;
  } > text

  .flash1 0x10000 :
  { *(.progmem1.data*)    /* Page 1 */
  } > text

  .flash2 0x20000 :
  { *(.progmem2.data*)    /* Page 2 */
  } > text

  .flash3 0x30000 :
  { *(.progmem3.data*)    /* Page 3 */
  } > text

  .flash4 0x40000 :
  { *(.progmem4.data*)    /* Page 4 */
  } > text

  .flash5 0x50000 :
  { *(.progmem5.data*)    /* Page 5 */
  } > text

  .hightext :
  {
    /* The following is OK to follow preceding sections
       at any even byte address, on any page. */

    . = ALIGN(2) ;
    *(.init0)  /* Start here after reset.  */
    KEEP (*(.init0))
    *(.init1)
    KEEP (*(.init1))
    *(.init2)  /* Clear __zero_reg__, set up stack pointer.  */
    KEEP (*(.init2))
    *(.init3)
    KEEP (*(.init3))
    *(.init4)  /* Initialize data and BSS.  */
    KEEP (*(.init4))
    *(.init5)
    KEEP (*(.init5))
    *(.init6)  /* C++ constructors.  */
    KEEP (*(.init6))
    *(.init7)
    KEEP (*(.init7))
    *(.init8)
    KEEP (*(.init8))
    *(.init9)  /* Call main().  */
    KEEP (*(.init9))
    *(.text)
    . = ALIGN(2);
    *(.text.*)
    . = ALIGN(2);
    *(.fini9)  /* _exit() starts here.  */
    KEEP (*(.fini9))
    *(.fini8)
    KEEP (*(.fini8))
    *(.fini7)
    KEEP (*(.fini7))
    *(.fini6)  /* C++ destructors.  */
    KEEP (*(.fini6))
    *(.fini5)
    KEEP (*(.fini5))
    *(.fini4)
    KEEP (*(.fini4))
    *(.fini3)
    KEEP (*(.fini3))
    *(.fini2)
    KEEP (*(.fini2))
    *(.fini1)
    KEEP (*(.fini1))
    *(.fini0)  /* Infinite loop after program termination.  */
    KEEP (*(.fini0))
     _etext = . ;
  }  > text

  .data   : AT (_etext)
  {
     PROVIDE (__data_start = .) ;
    /* --gc-sections will delete empty .data. This leads to wrong start
       addresses for subsequent sections because -Tdata= from the command
       line will have no effect, see PR13697.  Thus, keep .data  */
    KEEP (*(.data))
    *(.data*)
    *(.rodata)  /* We need to include .rodata here if gcc is used */
    *(.rodata*) /* with -fdata-sections.  */
    *(.gnu.linkonce.d*)
    . = ALIGN(2);
     _edata = . ;
     PROVIDE (__data_end = .) ;
  }  > data
  .bss   : AT (ADDR (.bss))
  {
     PROVIDE (__bss_start = .) ;
    *(.bss)
    *(.bss*)
    *(COMMON)
     PROVIDE (__bss_end = .) ;
  }  > data
   __data_load_start = LOADADDR(.data);
   __data_load_end = __data_load_start + SIZEOF(.data);
  /* Global data not cleared after reset.  */
  .noinit  :
  {
     PROVIDE (__noinit_start = .) ;
    *(.noinit*)
     PROVIDE (__noinit_end = .) ;
     _end = . ;
     PROVIDE (__heap_start = .) ;
  }  > data
  .eeprom  :
  {
    *(.eeprom*)
     __eeprom_end = . ;
  }  > eeprom
  .fuse  :
  {
    KEEP(*(.fuse))
    KEEP(*(.lfuse))
    KEEP(*(.hfuse))
    KEEP(*(.efuse))
  }  > fuse
  .lock  :
  {
    KEEP(*(.lock*))
  }  > lock
  .signature  :
  {
    KEEP(*(.signature*))
  }  > signature

    x = ASSERT (SIZEOF(.flash1) <= 0x10000, "Error: __flash1 (64KiB limit) 
overflow. Need to shrink it.") ;
    x = ASSERT (SIZEOF(.flash2) <= 0x10000, "Error: __flash2 (64KiB limit) 
overflow. Need to shrink it.") ;
    x = ASSERT (SIZEOF(.flash3) <= 0x10000, "Error: __flash3 (64KiB limit) 
overflow. Need to shrink it.") ;
    x = ASSERT (SIZEOF(.flash4) <= 0x10000, "Error: __flash4 (64KiB limit) 
overflow. Need to shrink it.") ;
    x = ASSERT (SIZEOF(.flash5) <= 0x10000, "Error: __flash5 (64KiB limit) 
overflow. Need to shrink it.") ;

  /* Stabs debugging sections.  */
  .stab 0 : { *(.stab) }
  .stabstr 0 : { *(.stabstr) }
  .stab.excl 0 : { *(.stab.excl) }
  .stab.exclstr 0 : { *(.stab.exclstr) }
  .stab.index 0 : { *(.stab.index) }
  .stab.indexstr 0 : { *(.stab.indexstr) }
  .comment 0 : { *(.comment) }
  /* DWARF debug sections.
     Symbols in the DWARF debugging sections are relative to the beginning
     of the section so we begin them at 0.  */
  /* DWARF 1 */
  .debug          0 : { *(.debug) }
  .line           0 : { *(.line) }
  /* GNU DWARF 1 extensions */
  .debug_srcinfo  0 : { *(.debug_srcinfo) }
  .debug_sfnames  0 : { *(.debug_sfnames) }
  /* DWARF 1.1 and DWARF 2 */
  .debug_aranges  0 : { *(.debug_aranges) }
  .debug_pubnames 0 : { *(.debug_pubnames) }
  /* DWARF 2 */
  .debug_info     0 : { *(.debug_info) *(.gnu.linkonce.wi.*) }
  .debug_abbrev   0 : { *(.debug_abbrev) }
  .debug_line     0 : { *(.debug_line) }
  .debug_frame    0 : { *(.debug_frame) }
  .debug_str      0 : { *(.debug_str) }
  .debug_loc      0 : { *(.debug_loc) }
  .debug_macinfo  0 : { *(.debug_macinfo) }
  /* DWARF 3 */
  .debug_pubtypes 0 : { *(.debug_pubtypes) }
  .debug_ranges   0 : { *(.debug_ranges) }
  /* DWARF Extension.  */
  .debug_macro    0 : { *(.debug_macro) }
}

file: script_tests

                           Tests and Expected Results
                           --------------------------
Trampolines, etc., must be in lower 128 KiB:
   Just fits in:
   $ avr-gcc -T avr6.x-new -Wl,-Map,flash.map -o flash.elf -DSTUBS=10 \
     -DP0=0x1f000 -DTEXT=0x20000 -mmcu=atmega2560 flash.sx

   Idx Name          Size      VMA       LMA       File off  Algn
  0 .data         00000000  00800200  0003f16e  0003f1e2  2**0
                  CONTENTS, ALLOC, LOAD, DATA
  1 .lowtext      0001f10c  00000000  00000000  00000074  2**1
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
  2 .hightext     00020062  0001f10c  0001f10c  0001f180  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, CODE

   .data LMA butts up to .hightext, which butts up to .lowtext.

   Overflows:
   $ avr-gcc -T avr6.x-new -Wl,-Map,flash.map -o flash.elf -DSTUBS=10 \
     -DP0=0x20000 -DTEXT=0x20000 -mmcu=atmega2560 flash.sx

   Error: .lowtext (128KiB limit) overflow. Try shrinking .progmem?

Locate __flashN at 0xN0000:
   Just fits in:
   $ avr-gcc -T avr6.x-new -Wl,-Map,flash.map -o flash.elf -DSTUBS=10 \
     -DP1=0x10000 -DP3=0x87ff -DTEXT=0x20000 -mmcu=atmega2560 flash.sx

   Idx Name          Size      VMA       LMA       File off  Algn
  0 .data         00000000  00800200  00058862  00038a22  2**0
                  CONTENTS, ALLOC, LOAD, DATA
  1 .lowtext      0000010c  00000000  00000000  000000b4  2**1
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
  2 .flash1       00010000  00010000  00010000  000001c0  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  3 .flash3       000087ff  00030000  00030000  000101c0  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  4 .hightext     00020063  000387ff  000387ff  000189bf  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, CODE

   Note: .hightext (where input section .text goes) butts up to 000387ff, using
         all spare .flash3. (From the next even byte address, inside .hightext.)
         .data in flash does the same with .hightext, at 00058862.

   Overflows:
   $ avr-gcc -T avr6.x-new -Wl,-Map,flash.map -o flash.elf -DSTUBS=10 \
     -DP1=0x10001 -DP3=0x10001 -DTEXT=0x20000 -mmcu=atmega2560 flash.sx

   Error: __flash1 (64KiB limit) overflow. Need to shrink it
   Error: __flash3 (64KiB limit) overflow. Need to shrink it.

   Overlaps prior section:
   $ avr-gcc -T avr6.x-new -Wl,-Map,flash.map -o flash.elf -DSTUBS=10 \
     -DP0=0x10000 -DP1=0x87ff -DTEXT=0x20000 -mmcu=atmega2560 flash.sx

   section .flash1 loaded at [00010000,000187fe] overlaps section .lowtext
   loaded at [00000000,0001010b]
   flash.elf: section .flash1 vma 0x10000 overlaps previous sections

   but ld also says this, which isn't helpful:

   section .hightext vma 0x187ff overlaps previous sections

   Overlaps following section:
   $ avr-gcc -T avr6.x-new -Wl,-Map,flash.map -o flash.elf -DSTUBS=10 \
     -DP1=0x10001 -DP2=0x87ff -DTEXT=0x20000 -mmcu=atmega2560 flash.sx

   Error: __flash1 (64KiB limit) overflow. Need to shrink it.
   section .flash2 loaded at [00020000,000287fe] overlaps section .flash1 loaded
   at [00010000,00020000]
   flash.elf: section .flash2 vma 0x20000 overlaps previous sections

   and again:
   flash.elf: section .hightext vma 0x287ff overlaps previous sections

The bulk of code (.text -> .hightext) snugs down if there are no __flashN:
   $ avr-gcc -T avr6.x-new -Wl,-Map,flash.map -o flash.elf -DSTUBS=10 \
     -DTEXT=0x20000 -mmcu=atmega2560 flash.sx

   Idx Name          Size      VMA       LMA       File off  Algn
  0 .data         00000000  00800200  0002016e  000201e2  2**0
                  CONTENTS, ALLOC, LOAD, DATA
  1 .lowtext      0000010c  00000000  00000000  00000074  2**1
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
  2 .hightext     00020062  0000010c  0000010c  00000180  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, CODE


   .hightext now follows imediately from .lowtext.

   

-- 
Whenever people agree with me I always feel I must be wrong.                  
                                                   - Oscar Wilde