GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-30345: Add -g to LDFLAGS to ease debug by vstinner · Pull Request #7709 · python/cpython

Conversation

@vstinner

@vstinner vstinner commented Jun 15, 2018 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vstinner
bpo-30345: Add -g to LDFLAGS to ease debug
9017194 
When using PGO+LTO and compile without LDFLAGS=-g, gdb fails to get
function arguments: add -g to LDFLAGS to ease debug.
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Jun 15, 2018
@bedevere-bot bedevere-bot added the awaiting merge label Jun 15, 2018
@vstinner

vstinner commented Jun 15, 2018

Copy link
Copy Markdown
Member Author

cc @Traceur759
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner

vstinner commented Jun 15, 2018

Copy link
Copy Markdown
Member Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ned-deily
ned-deily self-requested a review June 15, 2018 21:21
serhiy-storchaka
serhiy-storchaka reviewed Jun 16, 2018
View reviewed changes
Comment thread configure Outdated
fi
# bpo-30345: When using PGO+LTO and compile without LDFLAGS=-g,
# gdb fails to get function arguments.
LDFLAGS="$LDFLAGS -g"

@serhiy-storchaka serhiy-storchaka Jun 16, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@vstinner vstinner Jun 18, 2018

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

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 :-)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@serhiy-storchaka serhiy-storchaka Jun 19, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
methane
methane approved these changes Jun 18, 2018
View reviewed changes
@pitrou

pitrou commented Jun 18, 2018

Copy link
Copy Markdown
Member

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?)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@methane

methane commented Jun 19, 2018

Copy link
Copy Markdown
Member

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@methane

methane commented Jun 19, 2018

Copy link
Copy Markdown
Member

(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).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
Only add to LDFLAGS when using LTO
52fcc19
@vstinner

vstinner commented Jun 19, 2018

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner

vstinner commented Jun 19, 2018

Copy link
Copy Markdown
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?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vstinner added 2 commits June 19, 2018 17:46
@vstinner
Respect $ac_cv_prog_cc_g
0913c6c 
Only add -g if it's supported by the C compiler.
@vstinner
Merge branch 'master' into ldflags_g
3e52bfb
@methane

methane commented Jun 19, 2018

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
Update NEWS entry
4972939
@vstinner

vstinner commented Jun 19, 2018

Copy link
Copy Markdown
Member Author

I modified my PR to respect $ac_cv_prog_cc_g.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@methane

methane commented Jun 19, 2018

Copy link
Copy Markdown
Member

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@methane

methane commented Jun 19, 2018

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
vstinner merged commit 06fe77a into python:master Jun 19, 2018
@vstinner
vstinner deleted the ldflags_g branch June 19, 2018 16:25
@bedevere-bot bedevere-bot removed the awaiting merge label Jun 19, 2018
@vstinner vstinner added needs backport to 3.6 labels Jun 20, 2018
@miss-islington

miss-islington commented Jun 20, 2018

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington

miss-islington commented Jun 20, 2018

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington

miss-islington commented Jun 20, 2018

Copy link
Copy Markdown
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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington miss-islington assigned vstinner Jun 20, 2018
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 20, 2018
@vstinner @miss-islington
bpo-30345: Add -g to LDFLAGS for LTO (pythonGH-7709)
cf1c387 
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

bedevere-bot commented Jun 20, 2018

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed needs backport to 3.7 labels Jun 20, 2018
@bedevere-bot

bedevere-bot commented Jun 20, 2018

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vstinner added a commit that referenced this pull request Jun 22, 2018
@miss-islington @vstinner
bpo-30345: Add -g to LDFLAGS for LTO (GH-7709) (GH-7824)
1bb9dd3 
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
@vstinner
bpo-30345: Add -g to LDFLAGS for LTO (GH-7709) (GH-7826)
7839288 
Add -g to LDFLAGS when compiling with LTO to get debug symbols.
vstinner added a commit that referenced this pull request Jun 22, 2018
@vstinner
bpo-30345: Add -g to LDFLAGS for LTO (GH-7709) (GH-7825)
319cfb5 
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

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments
@methane methane methane approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@vstinner vstinner

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@vstinner @pitrou @methane @miss-islington @bedevere-bot @serhiy-storchaka @the-knights-who-say-ni
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.