New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add minimal debug info to release and develop profiles. #5708

Merged
merged 1 commit into from Jan 8, 2018

Conversation

Projects
None yet
6 participants
@SeppoTakalo
Contributor

SeppoTakalo commented Dec 14, 2017

This allows minimal debugging and allows tools like
mbed-os-linker-report to work properly.

Because debugging info is kept in .elf file and not flashed to device
there is no side effects to flash sizes.

Add minimal debug info to release and develop profiles.
This allows minimal debugging and allows tools like
mbed-os-linker-report to work properly.

Because debugging info is kept in .elf file and not flashed to device
there is no side effects to flash sizes.

@0xc0170 0xc0170 requested a review from theotherjimmy Dec 14, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Dec 14, 2017

Because debugging info is kept in .elf file and not flashed to device
there is no side effects to flash sizes.

Before there were proposed similar changes, the one thing that we had to consider is that how does this affect the online compiler (increasing debug info results in bigger final size of the build).

@sg-

This comment has been minimized.

Member

sg- commented Dec 16, 2017

Why only GCC? Does IAR and ARM compilers already give the needed info?

@adbridge

This comment has been minimized.

Contributor

adbridge commented Jan 2, 2018

@SeppoTakalo Could you address @sg- comment please ?

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jan 2, 2018

If size is a concern, we should also include here web team. As I noted above, if builds get bigger, might affect the online compiler. What are the number - for mbed 2 and mbed 5 ?

I would like to also get this aligned to all 3 toolchains but also be certain that this is fine for the online compiler (cc @immunda @th3james)

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jan 2, 2018

@0xc0170 This patch will not affect the online compiler one bit. The online compiler produces either a .bin binary or a .hex intel hex binary, neither of which contain debug information regardless of the compiler fags.

@theotherjimmy

@SeppoTakalo Why -g1 and not -g2 or -g3? ARM and IAR do not alread have a similar flag. Why only GCC?

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented Jan 3, 2018

-g1 is selected because "develop" and "release" builds are optimised and therefore cannot be accurately debugged. Therefore only minimal debug info should be provided.

I did not change IAR or ARM profiles because my initial goal was to get default build working with mbed-os-linker-report tool.
I might be worthwhile for including minimal debug information on all builds to allow at least function level stepping in debugger.

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented Jan 3, 2018

According to http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0493e/CHDBHCEA.html
the parameters --debug and --no_debug debug controls the same as -g in GCC. And default is --debug therefore now after adding -g1 GCC matches what ARM compiler has already been doing.

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented Jan 3, 2018

Also, according to IAR C/C++ Development
Guide
there is a command line option --strip

Syntax       --strip
Description  By default, the linker retains the debug information from the input object files in the
             output executable image. Use this option to remove that information.

This option is not used in our build profiles. Therefore having -g1 in GCC also matches what IAR builds have been doing.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jan 3, 2018

@0xc0170 This patch will not affect the online compiler one bit. The online compiler produces either a .bin binary or a .hex intel hex binary, neither of which contain debug information regardless of the compiler fags.

I was pointing at something else, not at the output of online compiler (that is clear it won't be affected), but what is happening in the background - we need to first build it (object files, then executable file), thus increasing a size of the workspace (but as Seppo noted above, ARMCC already has symbols in, so no change there).

As noted above by updates, ARMCC and IAR provide debug symbols - no change there, this is just for GCC to be aligned.

@0xc0170

0xc0170 approved these changes Jan 3, 2018

@theotherjimmy

Thanks for the explanation @SeppoTakalo

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jan 3, 2018

/morph build

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Jan 3, 2018

@mbed-ci

This comment has been minimized.

mbed-ci commented Jan 3, 2018

Build : SUCCESS

Build number : 799
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5708/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jan 4, 2018

Aborted, need to verify one patch that will make tests green again

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jan 5, 2018

/morph test

@mbed-ci

This comment has been minimized.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jan 5, 2018

/morph test

@mbed-ci

This comment has been minimized.

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Jan 8, 2018

@theotherjimmy theotherjimmy merged commit 31f7ea7 into ARMmbed:master Jan 8, 2018

11 checks passed

AWS-CI uVisor Build & Test Verification build successful.
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Local events testing has passed
Details
travis-ci/littlefs Local littlefs testing has passed
Details
travis-ci/mbed2 Local mbed2 testing has passed
Details
travis-ci/tools Local tools testing has passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment