"New-style" linker script process is not workable for multi-target apps

[multiplemonomials]

I was doing some testing, and while I didn't notice this initially looking at the PR, it seems like there's a pretty serious issue. Since ARMmbed/mbed-os#14199, the current architecture of mbed_set_linker_script() cannot properly work for the case where there are multiple different targets defined in the app and these targets link to different sets of Mbed OS modules.

This was noticed also in ARMmbed/mbed-os#14220, but that apparent CMake error is just masking the real issue.

Currently the flow of information is like this:

• mbed_set_linker_script() (called for each MCU) instructs the linker script preprocessor to read the file ${CMAKE_BINARY_DIR}/compile_time_defs.txt at build time, without knowing what's in it.
• User app links to mbed-core and some modules
• mbed_configure_app_target() takes the user app, aggregates all compile defs in it, and writes them to ${CMAKE_BINARY_DIR}/compile_time_defs.txt

But, if there are multiple different user apps, there are multiple sets of compile definitions! How does mbed_set_linker_script() know which one to read? It can't, because it's called before any user app targets have even been created.

A band-aid-fix for this is to just assume that all compile definitions used by the linker script are exactly the same between targets, so any compile_defs file can be used. That's what I did for now in my PR.

However, if this isn't an assumption we can make, fixing this properly would require making it so mbed_set_linker_script() is called in mbed_configure_app_target() instead of in the subdirs.

A related issue, also introduced in ARMmbed/mbed-os#14199, is massive target pollution due to a new linker script target being created for each MCU. This is what the CLion menu looks like now:

It will be next to impossible to actually find the target you want once all MCUs have been added to it.

Maybe you are command line people and don't care about what is essentially an IDE issue, but the deluge of targets will also affect the make help help message.

The fix for this is, again, is to call mbed_set_linker_script() once for each application (call it through mbed_set_post_build()) instead of once for each MCU. Plus, this way the linker script could be generated by a PRE_LINK action instead of having its own target.

Another way that would also work would be if you knew that only the definitions in some specific list (that is global for the entire build, not per-target) were actually used by the linker script. Then you could use just those definitions to create a single compile_defs.txt that is used everywhere, and a single target to create one shared linker script. This is how I did it on mbed-cmake!

[ladislas]

@multiplemonomials can you look at ARMmbed/mbed-os#14272

I think they're related.

[multiplemonomials]

That PR is a good cleanup, but it doesn't really address this issue I don't think.

[multiplemonomials]

Just submitted this PR which at least adds a workaround for some of these issues: ARMmbed/mbed-os#14274

[0xc0170]

Thanks for the report, we will review. @rwalton-arm is looking at fixing some of the areas mentioned here so we might fix it once for all.

I've noticed this is under mbed-tools, didn't we enable discussion for mbed-os and created CMake group there? I'll check

[ladislas]

I've noticed this is under mbed-tools, didn't we enable discussion for mbed-os and created CMake group there? I'll check

See @donatieng's response https://forums.mbed.com/t/mbed-os-working-groups/11730/21

[rwalton-arm]

Interesting read. Here are my thoughts.

Really the issue here (in my view) is that we create this global "compile_defs" to pass config definitions to the C preprocessor, so we can configure the linker scripts using the Mbed config system. This means we're fighting against CMake's preferred way of doing things (using target properties and propagating usage requirements upward) and we need this non-obvious backward dependency on the application in the mbed-os library. I'd rather we got our flow of dependencies and layering violations in order, instead of adding even more macros to work around that core issue.

The "old way" of setting the linker script was that each mbed-target set a global property, and we gated the add_subdirectory(<target>) call with some logic that checked some "labels" produced by mbed-tools from targets.json, so we only added the directory selected by mbed_config.cmake. Now that we do the semantically correct thing of adding the subdirectories unconditionally to make the targets available and use target_link_libraries to select the target to build (removing our dependency on much of targets.json) this global property would be set by every target and we'd just end up with the value the target that happened to be added to the project last set it to.

I have another PR up that changes mbed_generate_options_for_linker to pull the config definitions from mbed-core to populate the compile_defs file. We should only need config macros from mbed_config.cmake in the compile_defs file, we shouldn't need all the definitions the application may have attached to it.

Note: there was also a bug added by one of the recent PRs, which meant the compile_defs file wasn't being read at all when the linker script was set. Because our dependency chain isn't correct and previously we were setting these things in top-down fashion, order of operations between application and mbed-os has become important when it shouldn't be.

I can take a look at the additional target bloat. PRE_LINK may be a cleaner way of setting the linker script without adding the intermediate linker script target.

Another related issue: because we declare all targets as INTERFACE targets, it makes finding the targets actually being built even more difficult (all our targets seem to be marked as PHONY because we don't build actual libraries, which makes make help more difficult to parse).

Ideally we'd fix our weak linking issues so we could build libraries, which I think would solve quite a few problems we currently have, especially around unnecessary recompilation for projects that define multiple executable targets. We'd also remove any of the old style CMake-isms, like this compile_defs global file, and the way we set the core and toolchain. Then we should be able to integrate easily into a wide range of project setups without additional tooling we shouldn't need.

[multiplemonomials]

Oh, sweet! I didn't see that PR, that seems to fix most of the issues I was running into.

After that, the only thing left to fix is the target pollution. Since there's now a single compile defs file, it should be possible to have only one target to create the linker script, right?

[nat45928]

I'll second all of the things mentioned by @multiplemonomials. I use repos which build multiple executables quite a bit, and the new tooling makes that development style more painful.

I'd love to see some of the changes mentioned in @rwalton-arm's comment (#188 (comment)) especially with relation to the current INTERFACE target usage and weak linking issues.

I'm glad to see the Mbed team taking the time to tackle the build system, it's a big thing to overhaul but will greatly improve the Mbed experience!