ARMmbed / mbed-os

Arm Mbed OS is a platform operating system designed for the internet of things
https://mbed.com
Other
4.67k stars 2.98k forks source link

Client Lite fails to compile with Mbed OS 6.0.0 Alpha 3 #12781

Closed JanneKiiskila closed 4 years ago

JanneKiiskila commented 4 years ago

Description of defect

Client Lite 1.0.0 with mbed_app.json fixes (hence the fork) does not compile with Mbed OS 6.0.0 Alpha 3 using the baremetal profiles. It compiles still with Alpha 2.

Target(s) affected by this defect ?

NRF52840_DK + BG96 cellular

Toolchain(s) (name and version) displaying this defect ?

gcc-arm-none-eabi-9-2019-q4-major

What version of Mbed-os are you using (tag or sha) ?

mbed-os-6.0.0-alpha-3

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

1.10.1

How is this defect reproduced ?

git clone git@github.com:JanneKiiskila/pelion-client-lite-example.git
cd pelion-client-lite-example
git checkout mbed-os-6.0.0-alpha3
mbed deploy
mbed compile -m NRF52840_DK -t GCC_ARM --profile profiles/pico_lte_size.json --profile mbed-os/tools/profiles/extensions/minimal-printf.json
or 

mbed compile -m NRF52840_DK -t GCC_ARM --profile profiles/pico_lte_size.json

Replace the `mbed_cloud_dev_credentials.c` file with one you get from Pelion portal from your own account before compiling, though.

This will fail with the following error:

[Warning] setup.cpp@429,9: unused variable 'ret' [-Wunused-variable] Link: client-lite-example-internal arm-none-eabi-cpp: fatal error: '-c' is not a valid option to the preprocessor compilation terminated. [ERROR] arm-none-eabi-cpp: fatal error: '-c' is not a valid option to the preprocessor compilation terminated.



So, for some reason the `-c` option is no longer OK? 

This still works with the Alpha 2. 

If we leave out the --profile options, the target binary becomes 19 kilobytes larger, so that's not an option.

Even if you leave out the --profile minimal-printf, it fails.
JanneKiiskila commented 4 years ago

@0xc0170 @bulislaw

0xc0170 commented 4 years ago

The first thing comes to my mind: --profile mbed-os/tools/profiles/extensions/minimal-printf.json - this is default in alpha3 (PR https://github.com/ARMmbed/mbed-os/pull/12233), would that make difference?

However I dont understand why cpp is used - is it to run preprocessor on the linker file ? as we are at linking phase. Is the output coming from verbose output?

I am running it locally.

ciarmcom commented 4 years ago

Internal Jira reference: https://jira.arm.com/browse/MBOTRIAGE-2624

JanneKiiskila commented 4 years ago

Nope, mbed compile -m NRF52840_DK -t GCC_ARM --profile profiles/pico_lte_size.json fails as well.

JanneKiiskila commented 4 years ago

FAIL 03) d9becd449a Merge pull request #11856 from fkjagodzinski/gcc_build-enable_lto_for_release PASS 08) 4d0c3463a0 Fix Code style and reorder class members to get besser packing

That's hash that breaks it.

JanneKiiskila commented 4 years ago

It seems to be that fact that the "-c", was moved out of the common block and moved separately to each of the lines (.c/.cpp) instead that triggers this. @fkjagodzinski - can you explain in a bit more detail what is reason for breaking out the "-c"?

I also do have to admit that I don't know how the --profíle that I give on the command line for mbed cli interacts with the tools/profiles/develop.json file? Is some information from that used, too or is there some Python magic that now fails?

JanneKiiskila commented 4 years ago

The -c option seems to imply not linking.

-c Compile or assemble the source files, but do not link. The linking stage simply
is not done. The ultimate output is in the form of an object file for each source
file.
By default, the object file name for a source file is made by replacing the suffix
‘.c’, ‘.i’, ‘.s’, etc., with ‘.o’.
Unrecognized input files, not requiring compilation or assembly, are ignored.

according to: https://gcc.gnu.org/onlinedocs/gcc.pdf (page 34).

0xc0170 commented 4 years ago

I can't compile mbed anymore at the moment to be able to reproduce, not able to investigate (antivirus thinks I am doing something wrong compiling this example 🙄 ) 😞

JanneKiiskila commented 4 years ago

I think it's this expansion of the json blocks that now causes the -c to be there twice;

https://github.com/ARMmbed/mbed-os/blob/mbed-os-6.0.0-alpha-3/tools/toolchains/gcc.py#L154-L155

    self.ld += self.flags['ld'] + self.flags['common']

or linker can't accept the "-c", which now comes in via the common -block.

It seems I can work around this issue by modifying the pico-profile to have the "-c" similarly broken out.

That's at least my guess for now.

0xc0170 commented 4 years ago

updated tools/toolchains/gcc.py to use common flags at link time,

Yes, that is correct. My understanding due to linking process suggestions for LTO to use the same flags, -c was moved out as it could not be applied to a linker. The rest of the flags are applied to ld as per suggestion (taken from the PR 11856 comment):

I left this commit to handle the -flto extension, where it is recommended that you compile all the files participating in the same link with the same options and also specify those options at link time.

To fix it, moving -c from common should do the trick.

That PR should have been been "breaking" and I'll fix that now in PR #11856.

fkjagodzinski commented 4 years ago

It seems to be that fact that the "-c", was moved out of the common block and moved separately to each of the lines (.c/.cpp) instead that triggers this. @fkjagodzinski - can you explain in a bit more detail what is reason for breaking out the "-c"?

Sure; this flag -c was moved out of common in a PR that added the the LTO extension (mbed-os/tools/profiles/extensions/lto.json). This happened because common flags are recommended to be used also for the link step with the LTO. Here is the commit introducing this change https://github.com/ARMmbed/mbed-os/pull/11856/commits/a5240da3b4f20a0ba78bed57e65efc4c1fc3f753. You can also take a look at the last release note in https://github.com/ARMmbed/mbed-os/pull/11856.


The release note I mentioned above:

  • The common flags defined for the GCC_ARM toolchain are now appended to the ld flags during a mbed-cli build. Previously, the common flags were appended only to asm, c and cxx flags. Having the same flags for the compiler and the linker is required when using the link-time optimizer (which is the case for the lto build profile extension). Any options unrecognized by the linker should be moved from common to asm, c or cxx.