Mbed-os: Build No Longer Fails if 'target.restrict_size' Exceeded

[mattbrown015]

Description

It appears Add support for nrf52_dk bootloader #8097 has broken things for us! :-(

We have "target.restrict_size": "0x35800" set in our mbed_app.json and previously our build would fail if we exceeded the size. For example:

Using ROM regions application, post_application in this build.
  Region application: size 0x35800, offset 0x8000000
  Region post_application: size 0xa800, offset 0x8035800
...
Merging Regions
  Filling region application with .\BUILD\TEST_TARGET\GCC_ARM\application.bin
[ERROR] Contents of region application does not fit
[mbed] ERROR: "c:\python27\python.exe" returned error.
...

This is no longer the case, I suspect because of tools: fix up build_api padding logic and clean up unused code which removes:

if part_size > region.size:
    raise ToolException("Contents of region %s does not fit"
                         % region.name)

Build output now look likes:

Using ROM regions application, post_application in this build.
  Region application: size 0x35800, offset 0x8000000
  Region post_application: size 0xa800, offset 0x8035800
...
Merging Regions
  Filling region application with .\BUILD\PENGUIN\GCC_ARM\temperature-sensor_application.bin
Space used after regions merged: 0x359cc
Merging Regions
  Filling region application with .\BUILD\PENGUIN\GCC_ARM\temperature-sensor_application.bin
Space used after regions merged: 0x359cc

Our app started crashing and it has taken a couple of hours to realise why because the '0x359cc' wasn't noticed. Primarily because we were under the impression we would get a build failure if it exceeded '0x35800'.

We've been hit with a _double whammy_ this week because it is changes to Mbed OS that have caused our code to grow in size! MPU API #8335 added 824 bytes of code, see MPU API Increases Code Size and Cannot be Completely Removed #9007.

We shall spend the rest of the afternoon removing some cruft so that our app fits again. :-)

Issue request type

[ ] Question
[ ] Enhancement
[X] Bug

[ciarmcom]

Internal Jira reference: https://jira.arm.com/browse/MBOCUSTRIA-280

[cmonr]

@JammuKekkonen @naveenkaje What was the reason that the check was removed?
I didn't find anything in the PR's comments.

[naveenkaje]

I suspect that got removed because we refactored how the padding is done now where we iterate through the segments and pad the gaps. I sumbitted https://github.com/ARMmbed/mbed-os/pull/9021 to address the issue that @mattbrown015 is seeing.

[theotherjimmy]

@cmonr It got removed because that's no longer how we size regions. We size regions based only on the segments within the intelhex file that are port of ROM.

[theotherjimmy]

@mattbrown015 It sounds like you're using a target that incorrectly reports that it supports bootloader mode. Check that your linker script takes the defines MBED_APP_START and MBED_APP_SIZE into account.

[mattbrown015]

Hi @theotherjimmy,

Our target is a STM32L4. We've created a custom target based on NUCLEO_L432KC which supports a bootloader. We're not using a bootloader and haven't changed much in our target.

I can see the standard STM32 provided linker script references MBED_APP_START and MBED_APP_SIZE.

Are you saying that I should be defining MBED_APP_SIZE in mbed_app.json?

I'm off work for a couple of days so can't try for myself at the moment.

Thanks.

[theotherjimmy]

Are you saying that I should be defining MBED_APP_SIZE in mbed_app.json?

No. You are using "managed bootloader mode" so you will already see MBED_APP_SIZE passed to your linker script. You should use them in your linker script to restrict the size of your application so that you get a linker failure when your bootloader does not fit.

[mattbrown015]

I've had a look at this now and got a better handle on the Mbed OS build system. I can see you running the preprocessor to generate .link_script.ld.

I think probably I'm abusing the bootloader configuration slightly. We're not using a bootloader and _managed_ or _unmanaged_ bootloader mode is irrelevant to me. All I wanted to do was restricted the amount of flash allocated to .text/.data/.bss without writing my own linker script. I still don't want to create my own linker script, that just feels like something else that I would need to maintain.

I can see in .link_script.ld that the flash memory length is what I want:

  FLASH (rx) : ORIGIN = 0x8000000, LENGTH = 0x35800

rather than:

  FLASH (rx) : ORIGIN = 0x08000000, LENGTH = 256k

So my question now is, why isn't the link failing?

I now appreciate this probably isn't your problem but perhaps you understand these things better than me.

I'm guessing that the problem is related to strings. I can see our debug strings appear in the top of flash so it is always a string that overflows 0x35800.

In turn I'm guessing that it is something to do with the placement of the constants strings in the STM32 linker script. I'm using GCC. The STM32 linker script appears to place .text, .data, .bss and .rodata in FLASH but there are a great many wildcards ('.', '.', etc) that I don't understand.

[theotherjimmy]

So my question now is, why isn't the link failing?

Good question; I don't know. I expect the link to fail with that linker script.

If you provide me with a bit more specifics on the target (like which linker script in mbed-os is used) I might be able to help more.

Stuff that's not directly related to your question below the line.

---

I think probably I'm abusing the bootloader configuration slightly.

Nope. You're using the "I'm building a bootloader" use case of managed bootloader mode. The only "abuse" here is that you're not building a "bootloader", but the tools really don't care about that at all.

We're not using a bootloader and managed or unmanaged bootloader mode is irrelevant to me.

Perhaps you're not using a bootloader, or even making one. That just means that the tools mode you're using is named poorly (my bad), not that it's semantics are irrelevant.

[mattbrown015]

Our custom target inherits from NUCLEO_L432KC so the mbed-os linker script is STM32L432XX.ld:

mbed-os/targets/TARGET_STM/TARGET_STM32L4/TARGET_STM32L432xC/device/TOOLCHAIN_GCC_ARM/STM32L432XX.ld

[theotherjimmy]

My intuition is that the AT here: https://github.com/ARMmbed/mbed-os/blob/master/targets/TARGET_STM/TARGET_STM32L4/TARGET_STM32L432xC/device/TOOLCHAIN_GCC_ARM/STM32L432XX.ld#L88 may be working against us here and placing the contents of .data sections that overflow the end of your application's region. You should be able to check this by running arm-none-eabi-nm BUILD/<your-target>/GCC_ARM/<your-app>_application.elf | sort and looking for addresses > 0x08035800 and < 0x20000000. You should specifically look for the _edata symbol: https://github.com/ARMmbed/mbed-os/blob/master/targets/TARGET_STM/TARGET_STM32L4/TARGET_STM32L432xC/device/TOOLCHAIN_GCC_ARM/STM32L432XX.ld#L120 Which will indicate that the data section is what overflowed rom.

[mattbrown015]

I think it is something to do with the initialisation values for the .data section. _sidata is the start address of the initialisation values.

Currently the app is linking because I found some savings. To make it nearly overflow again I added a massive 'const char[]' which gets located in the middle of FLASH and pushes everything else up. I've got it so adding 1 more char to the string makes the link fail.

In the symbol list 080357fc _sidata which can't be correct, there's got to be more than 4 bytes of initialisation!

...
08035238 T _ctype_
0803533c T __hexdig
0803543c t __EH_FRAME_BEGIN__
0803543c t __FRAME_END__
0803557c R __exidx_start
080357fc R __etext
080357fc R __exidx_end
080357fc R _sidata
20000188 D __data_start__
20000188 D __dso_handle
20000188 D _sdata
2000018c d _ZZL24M24SR_BuildIBlockCommandtP6C_APDUhPtPhE11BlockNumber
20000190 d _ZN11adc_manager12_GLOBAL__N_14hadcE
...

After some fumbling around I made this change:

index 34f44d070a..5ccba9bdd5 100644
--- a/targets/TARGET_STM/TARGET_STM32L4/TARGET_STM32L432xC/device/TOOLCHAIN_GCC_ARM/STM32L432XX.ld
+++ b/targets/TARGET_STM/TARGET_STM32L4/TARGET_STM32L432xC/device/TOOLCHAIN_GCC_ARM/STM32L432XX.ld
@@ -82,10 +82,10 @@ SECTIONS
     } > FLASH
     __exidx_end = .;

-    __etext = .;
-    _sidata = .;
+    __etext = LOADADDR( .data );
+    _sidata = LOADADDR( .data );

-    .data : AT (__etext)
+    .data :
     {
         __data_start__ = .;
         _sdata = .;
@@ -119,7 +119,7 @@ SECTIONS
         __data_end__ = .;
         _edata = .;

-    } > SRAM1
+    } > SRAM1 AT> FLASH

     .bss :
     {

This appears to ensure the link will fail if there isn't room for the initialisation values, _sidata won't go above 08034afc:

0803487c R __exidx_start
08034afc A __etext
08034afc R __exidx_end
08034afc A _sidata
20000188 D __data_start__
20000188 D __dso_handle
20000188 D _sdata

I had a look at a couple of other ST linker scripts, selected at random, and they all looked the same.

On the other hand, the SiLabs linker script that I looked at did } > RAM AT > FLASH.

[theotherjimmy]

Thanks so much @mattbrown015 That really helps with my understanding of the linker scripts!

[theotherjimmy]

@maclobdell Could you inform STM of this?

[theotherjimmy]

@mattbrown015 For the time being, you can put the mbed-os linker script location into a .mbedignore file and add the modified linker script into your project. That should get things working, while watching for the official solution in this issue.

[mattbrown015]

@theotherjimmy, yes now we know what's going on I'm happy and we can work with what we've got! :-)

Shall we wait for STM to have a look before I close this issue?

[theotherjimmy]

Yes, let's wait for a PR to fix this. This issue now has a workaround, but is not resolved.

[naveenkaje]

@mattbrown015 https://github.com/ARMmbed/mbed-os/pull/9021 could you please let us know if this helps to resolve your issue?

[mattbrown015]

Hi @naveenkaje ,

I'm sorry, I'm in the middle of something else right now, I'm not sure when I will be to try this.

I thought we'd agreed above that the problem was with ST's linker scripts and we were going to see what they had to say. I haven't seen anything from ST.

[cmonr]

@ARMmbed/team-st-mcd Thoughts?

[LMESTM]

@mattbrown015 et all
we'd be keen to help and update the linker scripts if needed.
Could you maybe make a more precise explanation of what is being achieved in https://github.com/ARMmbed/mbed-os/issues/9012#issuecomment-447020640
?
We're also not specialists of GC Clinker scripts and would need some description before we would deploy this massively to the STM32 targets.
Also I understand that #9021 has introduced a check again right ?

[mattbrown015]

Hi @LMESTM,

I'm not sure I can say much more than what I've already said above. The reason the comment is a bit vague is that I don't understand linker scripts that well either. I tried those changes because of what I saw in other scripts not because I understood them particular well. I agree we need to find someone who does understand before rolling out a big change.

I think the link should fail if data section initialisation data doesn't fit into the flash memory. #9021 is about the build tools doing something that could cause the Mbed build to fail but I'm not sure that should be a replacement for the linker script being correct and the link failing if it runs out of flash.

Unfortunately I've been moved onto another project that doesn't use Mbed OS. The Mbed OS project I was working on has been frozen while we do the final testing before release.

[0xc0170]

@mprse @deepikabhavnani Can you please review

[jeromecoutant]

Seems this issue is stuck ?

[0xc0170]

@ARMmbed/mbed-os-tools

[mark-edgeworth]

Thanks to all on this thread for the detail here. I don't yet know when we will be able to look at it as we have a huge backlog of issues to deal with.

[jeromecoutant]

@mark-edgeworth
Check https://github.com/ARMmbed/mbed-os/issues/9012#issuecomment-447020640

[jeromecoutant]

ping @mark-edgeworth

@adbridge I understand issue is not related to ST device but more on tool scripts.
Could you remove the label ?
Thx

[jeromecoutant]

adbridge assigned mark-edgeworth 7 days ago
mark-edgeworth removed their assignment 4 days ago

...

[0xc0170]

@jeromecoutant We use internal tracking for assignment.

[LDong-Arm]

We tried to reproduce the setup with

• the current mbed-os master, and the previous release when it was raised (5.11, in Decemeber 2018)
• GCC 9 (current), GCC 6 (recommended back then)
• NUCLEO_L432KC, and a customer target that inherits it
• target.restrict_size overriden to be smaller than the application - also we enlarged the application with a very large char array

But we were unable to reproduce the issues discussed - the linker correctly reported

./BUILD/NUCLEO_L432KC/GCC_ARM/myapp_application.elf section `.text' will not fit in region `FLASH'"

meaning the size check was okay for us.

Please tell us if we missed something. Otherwise we would close this issue as "cannot reproduce". Feel free to raise a new issue if you need further help, thanks.

[0xc0170]

As stated above, the reporter moved to a different project and has not been updated for some time.

I'll close this, should be reopened if the issue is still valid. Thanks @LDong-Arm

[mattbrown015]

Closing feels like the correct thing to do!

It was a long time ago and so much has changed it probably isn't relevant any more.

Thanks,
Matt