Skip to content

bpo-30345: Add -g to LDFLAGS to ease debug #7709

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

Merged
merged 5 commits into from
Jun 19, 2018
Merged

bpo-30345: Add -g to LDFLAGS to ease debug #7709

merged 5 commits into from
Jun 19, 2018

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Jun 15, 2018

When using PGO+LTO and compile without LDFLAGS=-g, gdb fails to get
function arguments: add -g to LDFLAGS to ease debug.

https://bugs.python.org/issue30345

When using PGO+LTO and compile without LDFLAGS=-g, gdb fails to get
function arguments: add -g to LDFLAGS to ease debug.
@vstinner
Copy link
Member Author

cc @Traceur759

@vstinner
Copy link
Member Author

Hum, who can review this change? :-) @pitrou @ned-deily @serhiy-storchaka @methane @doko42 modified configure.ac recently. Also @encukou maybe?

@ned-deily ned-deily self-requested a review June 15, 2018 21:21
configure Outdated
@@ -6716,6 +6716,9 @@ then
else
OPT="-g $WRAP -O3 -Wall"
fi
# bpo-30345: When using PGO+LTO and compile without LDFLAGS=-g,
# gdb fails to get function arguments.
LDFLAGS="$LDFLAGS -g"
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be added only in debug mode? I afraid that adding -g in release mode can disable some optimizations.

Copy link
Member Author

Choose a reason for hiding this comment

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

We already always pass -g to CFLAGS, even in release mode.

Extract of gcc manual page:

       -g  Produce debugging information in the operating system's native
           format (stabs, COFF, XCOFF, or DWARF).  GDB can work with this
           debugging information.

           On most systems that use stabs format, -g enables use of extra
           debugging information that only GDB can use; this extra information
           makes debugging work better in GDB but probably makes other
           debuggers crash or refuse to read the program.  If you want to
           control for certain whether to generate the extra information, use
           -gstabs+, -gstabs, -gxcoff+, -gxcoff, or -gvms (see below).

It's unrelated to optimizations. It's just an option to ask to include also debug symbols in the produced binary file (like ELF file on Linux).

If you want to get ride of them, just run "strip python". Linux vendors are able to distribute these debug symbols in a different package, like python-debuginfo on Fedora.

I'm not 100% sure, that's why I'm asking for a review :-)

Copy link
Member

Choose a reason for hiding this comment

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

Thank you, now I got this.

@pitrou
Copy link
Member

pitrou commented Jun 18, 2018

I think it's ok to add -g to link flags, I just don't understand why it's added in a single code path in configure.ac.
(also, what exactly is $ac_cv_prog_cc_g?)

@methane
Copy link
Member

methane commented Jun 19, 2018

AFAIK, -g is required only when LTO because code generation is happen in link time.
So I think it's safer that adding -g here in configure.ac.

   CFLAGS="$CFLAGS $LTOFLAGS"
-  LDFLAGS="$LDFLAGS $LTOFLAGS"
+  LDFLAGS="$LDFLAGS -g $LTOFLAGS"

LINKCC is CC for most case, but not always.

@methane
Copy link
Member

methane commented Jun 19, 2018

(also, what exactly is $ac_cv_prog_cc_g?)

I think it means whether CC supports -g or not. And we use CC for LINKCC by default on most platform. (except AIX).

@vstinner
Copy link
Member Author

"AFAIK, -g is required only when LTO because code generation is happen in link time. So I think it's safer that adding -g here in configure.ac."

Done.

@vstinner
Copy link
Member Author

"(also, what exactly is $ac_cv_prog_cc_g?) I think it means whether CC supports -g or not."

Hum, why, I cannot add -g for all platforms?

vstinner added 2 commits June 19, 2018 17:46
Only add -g if it's supported by the C compiler.
@methane
Copy link
Member

methane commented Jun 19, 2018

I don't know linker without -g support.
If AIX supports -g, I feel worrying about minor linker/platform is worthless.

@vstinner
Copy link
Member Author

I modified my PR to respect $ac_cv_prog_cc_g.

@methane
Copy link
Member

methane commented Jun 19, 2018

I'm sorry, I meant why ac_cv_prog_cc_g is used in general.

LTO is used only with clang and gcc. So no need to worry about others when adding -g only with LTO.

@methane
Copy link
Member

methane commented Jun 19, 2018

I'm OK to merge this with or without $ac_cv_prog_cc_g check.

@vstinner vstinner merged commit 06fe77a into python:master Jun 19, 2018
@vstinner vstinner deleted the ldflags_g branch June 19, 2018 16:25
@miss-islington
Copy link
Contributor

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @vstinner, I could not cleanly backport this to 3.6 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 06fe77a84bd29d51506ab2ff703ae585a6121af2 3.6

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 20, 2018
Add -g to LDFLAGS when compiling with LTO to get debug symbols.
(cherry picked from commit 06fe77a)

Co-authored-by: Victor Stinner <[email protected]>
@bedevere-bot
Copy link

GH-7824 is a backport of this pull request to the 3.7 branch.

@bedevere-bot
Copy link

GH-7826 is a backport of this pull request to the 3.6 branch.

vstinner added a commit that referenced this pull request Jun 22, 2018
Add -g to LDFLAGS when compiling with LTO to get debug symbols.
(cherry picked from commit 06fe77a)

Co-authored-by: Victor Stinner <[email protected]>
vstinner added a commit that referenced this pull request Jun 22, 2018
Add -g to LDFLAGS when compiling with LTO to get debug symbols.
vstinner added a commit that referenced this pull request Jun 22, 2018
Add -g to LDFLAGS when compiling with LTO to get debug symbols.
@ned-deily ned-deily removed their request for review June 22, 2018 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants