Skip to content
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

Document Tips for Debugging C Extensions #35100

Merged
merged 16 commits into from
Dec 10, 2020
Merged

Conversation

WillAyd
Copy link
Member

@WillAyd WillAyd commented Jul 2, 2020

Quick notes in case they are helpful to others. Can move to the Wiki as well if we don't want in the standard docs

setup.py Outdated
@@ -435,8 +433,14 @@ def run(self):
else:
extra_compile_args = ["-Werror"]
extra_link_args = []
if debugging_symbols_requested:
extra_compile_args.append("-g")
if not debugging_symbols_requested:
Copy link
Member Author

Choose a reason for hiding this comment

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

I guess Python by default (at least locally and looking at some of the CI builds) includes the -g flag as part of the CFLAGS that distutils uses to compile extensions, so in the current setup this does nothing.

According to SO we can override that by appending here, which might help reduce file size by removing those symbols:

https://stackoverflow.com/a/37952343/621736

I can also remove this from this PR if deemed too orthogonal. IIRC @xhochy or @TomAugspurger may have experience with stripping debug symbols from built distributions

Copy link
Contributor

Choose a reason for hiding this comment

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

Multibuild may do this by default now? I don't recall.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, multibuild includes this nowadays.

setup.py Outdated
# Strip debugging symbols (included by default)
extra_compile_args.append("-g0")
else:
# TODO: these should override the defaults provided by Python
Copy link
Member Author

Choose a reason for hiding this comment

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

distutils adds NDEBUG and -O3 by default it seems without a feasible way to remove those compilation flags. Appending these at the end should override those according to the SO link shared above

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't recommend building with -O0 actually as it hides a lot of problems. In case of -O0 memory is mored often zeroed and thus invalid memory access is not that fatal (in the higher -Ox cases you would get random data). This makes the detection of bugs a lot harder. It might be better to keep the optimization level here and add flags that make debugging easier. Personally I like -ggdb -fno-omit-frame-pointer. This gives better stacktraces and a bit more debugging information.

Copy link
Member Author

Choose a reason for hiding this comment

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

gdb suggests turning off optimizations:

https://sourceware.org/gdb/onlinedocs/gdb/Optimized-Code.html

There are certainly exceptions but I think as a general rule (especially for people that aren't super well versed in debugging the extensions yet) that no optimizations will be easier to follow

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the debug information with -O0 is definitely better, I would like to leave -fno-omit-frame-pointer somewhere here in the document as this is the option that provides the most debug information for the smallest performance hit and is really useful if you have an memory-out-of-bounds issue that wouldn't occur easily in the unoptimized version.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good. I think this is off by default with -O0 per the docs but doesn't hurt to add again

https://gcc.gnu.org/onlinedocs/gcc-3.4.4/gcc/Optimize-Options.html

@pep8speaks
Copy link

pep8speaks commented Jul 6, 2020

Hello @WillAyd! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-12-09 00:49:35 UTC


.. code-block:: sh

gdb --args python -m pytest
Copy link
Member

Choose a reason for hiding this comment

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

$ gdb --args python3 -m pytest
[...]
"~/.pyenv/shims/python3": not in executable format: File format not recognized
(gdb) run
Starting program:  -m pytest
No executable file specified.
Use the "file" or "exec-file" command.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you try gdb -ex r --args python3 -m pytest? Taking that from this link:

https://wiki.python.org/moin/DebuggingWithGdb

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea looks like gdb -ex r --args python3 -m pytest pandas/tests was working for me if you want to try on yours and confirm

Copy link
Member

Choose a reason for hiding this comment

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

$ gdb -ex r --args python3 -m pytest pandas/tests
[...]
"~/.pyenv/shims/python3": not in executable format: File format not recognized
Starting program:  -m pytest pandas/tests
No executable file specified.
Use the "file" or "exec-file" command.
(gdb) 

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you using pyenv for development or Conda?

Copy link
Member

Choose a reason for hiding this comment

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

you've already gone above and beyond helping me debug this; ill spend some more time on this and ping you if i find anything new

Copy link
Member Author

Choose a reason for hiding this comment

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

All good - I think this is generally helpful to hash out together so thanks for the input.

It looks like this might be specific to pyenv and how it manages the python executable:

https://stackoverflow.com/questions/48141135/cannot-start-dbg-on-my-python-c-extension

Copy link
Member

Choose a reason for hiding this comment

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

gdb -ex r --args bash pytest pandas/tests --skip-slow --skip-db tentatively looks like a winner

Copy link
Member

Choose a reason for hiding this comment

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

is the command from my previous comment specific to my case, or relevant to the document?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm trying to avoid adding too much detail here since this issue is more of a pyenv thing than a debugger issue

@simonjayhawkins simonjayhawkins added this to the 1.2 milestone Jul 24, 2020
@WillAyd
Copy link
Member Author

WillAyd commented Sep 10, 2020

Any other comments here? Otherwise plan on merging in a few days

@jbrockmendel
Copy link
Member

One question, no objections.

Using a debugger
================

You can create a script that hits the extension module you are looking to debug and place it in the project root. Thereafter launch a Python process under lldb:
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reference on lldb (link / how to install?)

Copy link
Member Author

Choose a reason for hiding this comment

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

I have only used on macOS where it comes bundled with the Xcode tools. Not sure about other systems

Copy link
Member

Choose a reason for hiding this comment

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

On my ubuntu machine it's not available (while gdb is). So I would at least link to some web page about it if we want to recommend it over gdb, I suppose most readers won't know what lldb is (I also didn't).
it seems you can install it from conda-forge.

Copy link
Member Author

Choose a reason for hiding this comment

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

To be clear I don't recommend one over the other - should just use whatever comes with your build system.

I'll try to reword

Copy link
Member

Choose a reason for hiding this comment

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

Did you settle on a wording here?

@jbrockmendel
Copy link
Member

any movement here?

@WillAyd
Copy link
Member Author

WillAyd commented Oct 19, 2020

Been tight on time lately so closing this temporarily to clean the queue - will come back to this at a later date

@WillAyd WillAyd closed this Oct 19, 2020
@WillAyd WillAyd reopened this Dec 9, 2020
@WillAyd WillAyd removed this from the 1.2 milestone Dec 9, 2020
@WillAyd
Copy link
Member Author

WillAyd commented Dec 9, 2020

Getting back around to this. Since there was some confusion around gdb vs lldb I added a blurb about which one to choose when. I also made it so all of the steps are shown for both debuggers

Copy link
Member

@jbrockmendel jbrockmendel left a comment

Choose a reason for hiding this comment

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

LGTM

@jreback jreback added this to the 1.2 milestone Dec 10, 2020
@jreback jreback merged commit ec8240a into pandas-dev:master Dec 10, 2020
@jreback
Copy link
Contributor

jreback commented Dec 10, 2020

thanks @WillAyd

meeseeksmachine pushed a commit to meeseeksmachine/pandas that referenced this pull request Dec 10, 2020
simonjayhawkins pushed a commit that referenced this pull request Dec 10, 2020
Co-authored-by: William Ayd <will_ayd@innobi.io>
@WillAyd WillAyd deleted the ext-debug branch April 12, 2023 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants