Skip to content

native: turn on creation of debug symbols (CFLAGS += -g)#13423

Merged
cgundogan merged 1 commit intoRIOT-OS:masterfrom
kaspar030:turn_on_native_debug
Feb 20, 2020
Merged

native: turn on creation of debug symbols (CFLAGS += -g)#13423
cgundogan merged 1 commit intoRIOT-OS:masterfrom
kaspar030:turn_on_native_debug

Conversation

@kaspar030
Copy link
Copy Markdown
Contributor

@kaspar030 kaspar030 commented Feb 20, 2020

Contribution description

I know there are a bunch of "make-debug-..." targets, but all other platforms create the debug info by default. native is an exception. I'm regularly finding my self manually adding the "-g" to CFLAGS, after finding a bug, then recompile, retrigger, ...

So this PR enables "-g" by default for native.

Testing procedure

Issues/PRs references

@kaspar030 kaspar030 added Platform: native Platform: This PR/issue effects the native platform Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Feb 20, 2020
@kaspar030 kaspar030 requested a review from miri64 February 20, 2020 14:45
@cgundogan
Copy link
Copy Markdown
Member

cgundogan commented Feb 20, 2020 •
edited
Loading

shouldn't this be an OPTIONAL_CFLAG so that we can disable this on the ci? I don't think the extra overhead of including debug symbols is wanted for the thousands of jobs that compile for native?

@miri64
Copy link
Copy Markdown
Member

miri64 commented Feb 20, 2020

Why not use -g3 as with make all-debug?

@kaspar030
Copy link
Copy Markdown
Contributor Author

kaspar030 commented Feb 20, 2020

shouldn't this be an OPTIONAL_CFLAG so that we can disable this on the ci? I don't think the extra overhead of including debug symbols is wanted for the thousands of jobs that compile for native?

Right. Using CFLAGS_DBG now, which will be overridden by CI.

Why not use -g3 as with make all-debug?

I thought that was default anywas. Changed to -g3.

cgundogan
cgundogan approved these changes Feb 20, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@cgundogan cgundogan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks IMO good and is ready to be squashed!

@kaspar030 kaspar030 force-pushed the turn_on_native_debug branch from 4005359 to dbd8654 Compare February 20, 2020 16:07
@cgundogan
Copy link
Copy Markdown
Member

cgundogan commented Feb 20, 2020

Without PR

% make all debug
Reading symbols from RIOT/examples/gnrc_networking/bin/native/gnrc_networking.elf...
(No debugging symbols found in RIOT/examples/gnrc_networking/bin/native/gnrc_networking.elf)

With PR

% make all debug
Reading symbols from RIOT/examples/gnrc_networking/bin/native/gnrc_networking.elf...

@cgundogan cgundogan added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Feb 20, 2020
@cgundogan cgundogan merged commit 86fcc35 into RIOT-OS:master Feb 20, 2020
@leandrolanzieri leandrolanzieri added the Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer label Feb 21, 2020
@leandrolanzieri leandrolanzieri added this to the Release 2020.04 milestone Feb 21, 2020
@miri64
Copy link
Copy Markdown
Member

miri64 commented Feb 21, 2020

Should we maybe add a deprecation message to make all-debug now?

@kaspar030 kaspar030 deleted the turn_on_native_debug branch February 21, 2020 10:55
@miri64 miri64 mentioned this pull request Feb 21, 2020
Merged
@miri64 miri64 mentioned this pull request Jul 28, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cgundogan cgundogan cgundogan approved these changes

@miri64 miri64 Awaiting requested review from miri64

@aabadie aabadie Awaiting requested review from aabadie

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

Assignees

No one assigned

Labels

Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Platform: native Platform: This PR/issue effects the native platform Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines

Projects

None yet

Milestone

Release 2020.04

Development

Successfully merging this pull request may close these issues.

4 participants

@kaspar030 @cgundogan @miri64 @leandrolanzieri