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

ci: build wheels with debug info #1599

Merged
merged 1 commit into from
Mar 26, 2024
Merged

ci: build wheels with debug info #1599

merged 1 commit into from
Mar 26, 2024

Conversation

lidavidm
Copy link
Member

@lidavidm lidavidm commented Mar 7, 2024

Fixes #1583.

@lidavidm
Copy link
Member Author

lidavidm commented Mar 8, 2024

Looks like I should get to #847 first

@lidavidm lidavidm force-pushed the gh-1583 branch 2 times, most recently from 2a3c884 to 2821347 Compare March 8, 2024 16:46
@lidavidm
Copy link
Member Author

Comparing with main:

  • Linux wheels: 44 MB vs 44 MB
  • macOS: 41 MB vs 41 MB
  • Windows: 38 MB vs 21 MB

So Windows gains about 80% but the other platforms are largely unchanged

I think that's still about reasonable, though. Once I confirm that I can get stacktraces with this, I can then look at splitting debuginfo.

@lidavidm
Copy link
Member Author

  • backward-cpp is pretty easy to integrate
  • our linker script is hiding something necessary for it to print function names in the back trace
  • we need to link another library if we also want to get file names/line numbers but all appear to be GPL/LGPL

@lidavidm
Copy link
Member Author

@lidavidm
Copy link
Member Author

Hmm, and libunwind doesn't seem to help :/

libbacktrace supposedly can do this, but it's not integrated into backward-cpp

@lidavidm
Copy link
Member Author

Or enabling one of libbfd/libdwarf/etc. but then we run back into licensing issues

@lidavidm
Copy link
Member Author

Ok, so I'm not sure backward-cpp will be useful here. But we can at least build with debug info.

@lidavidm lidavidm marked this pull request as ready for review March 18, 2024 18:15
@lidavidm lidavidm requested a review from kou as a code owner March 18, 2024 18:15
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

FYI:

For Linux, we can get source locations from backtrace()/backtrace_symbols() output later with .so and addr2line.

For Windows, I have a backtrace implementation which is licensed by LGPL 2.1 but I can re-license it to Apache-2.0 because I'm the author.

@lidavidm
Copy link
Member Author

Oh, interesting. I'll give it another shot tomorrow then. backward-cpp uses backtrace_symbols by default, but when I saw it couldn't give the symbol name if we hide the symbols via linker script I was a little disappointed.

I haven't tested the behavior on Windows, I will try that too.

@kou
Copy link
Member

kou commented Mar 19, 2024

but when I saw it couldn't give the symbol name if we hide the symbols via linker script I was a little disappointed.

backtrace_symbols() shows an address even when the symbol name can't be resolved. addr2line can show the corresponding source file/line by the address.

FYI: https://github.com/groonga/groonga/blob/1caae5659a84b08c787e3466682aba424a141609/tools/parse-backtrace.rb#L264

@lidavidm
Copy link
Member Author

Hmm, addr2line doesn't work

#5    Object "/home/lidavidm/Code/arrow-adbc/python/adbc_driver_manager/adbc_driver_manager/_lib.cpython-311-x86_64-linux-gnu.so", at 0x7fc95b0c1158, in 
#4    Object "/home/lidavidm/Code/arrow-adbc/python/adbc_driver_sqlite/adbc_driver_sqlite/libadbc_driver_sqlite.so", at 0x7fc8cb94b4cb, in 
#3    Object "/home/lidavidm/Code/arrow-adbc/python/adbc_driver_sqlite/adbc_driver_sqlite/libadbc_driver_sqlite.so", at 0x7fc8cb94b3af, in 
$ addr2line -e /home/lidavidm/Code/arrow-adbc/python/adbc_driver_manager/adbc_driver_manager/_lib.cpython-311-x86_64-linux-gnu.so 0x7fc95b0c1158
??:0

Presumably because this is a virtual address, but we don't have the base. I remember catchsegv/libSegFault would print that, maybe we can improve backward.cpp...

@kou
Copy link
Member

kou commented Mar 19, 2024

Presumably because this is a virtual address, but we don't have the base.

I think so. backtrace_symbols() shows both of a virtual address and a relative (?) address.

@lidavidm
Copy link
Member Author

Maybe backward.hpp could use adjustment then...I'll keep digging. Thanks!

@lidavidm
Copy link
Member Author

The other problem is that it seems to not do anything on Windows, which was the main motivation in the first place

@lidavidm
Copy link
Member Author

Hmm so from the backtrace_symbols manpage the default output is something like ./prog(myfunc3+0x5c) [0x80487f0]. But we can't get the myfunc3+0x5c in the first place because our symbols are hidden :/

@lidavidm
Copy link
Member Author

I suppose on Linux, we can print /proc/self/maps after the backtrace. There seems to be a macOS-equivalent API and Windows-equivalent API, too. But on Windows I still can't get it to actually print any backtrace in the first place...

@lidavidm
Copy link
Member Author

Ah, ok. Segfault will print the stack trace, something like abort() or assert(false) is handled by the windows runtime libraries.

@lidavidm
Copy link
Member Author

Ok! If I print /proc/self/maps, then I can use addr2line and recover the line number properly. So I'll need to patch backward-cpp a bit.

On Windows, it already resolves line number properly, so long as the debug info is available...So I just need to package that up properly. However I can't get windows to find the pdb file unless I put it in PWD. For the time being I think that's still tenable.

@lidavidm
Copy link
Member Author

TODOs

  • Confirm this works with the PostgreSQL crash
  • Make this an optional thing instead of automatic (I'm currently leaning towards including a secret magic library that you import)

@kou
Copy link
Member

kou commented Mar 20, 2024

Hmm so from the backtrace_symbols manpage the default output is something like ./prog(myfunc3+0x5c) [0x80487f0]. But we can't get the myfunc3+0x5c in the first place because our symbols are hidden :/

FYI: If backtrace_symbols() can't resolve myfunc3, the part has only address that can be used for addr2line directly. If we have myfunc3+0x5c, we need to convert myfunc3 to address. We can do it by nm --demangle XXX.so | grep myfunc3 | cut -d' ' -f1. We can use it for addr2line: addr2line --exe=XXX.so $(myfunc3_address + 0x5c)

@lidavidm lidavidm marked this pull request as draft March 20, 2024 13:39
@lidavidm lidavidm force-pushed the gh-1583 branch 2 times, most recently from fd61eb5 to a119769 Compare March 20, 2024 13:53
@lidavidm lidavidm marked this pull request as ready for review March 20, 2024 15:14
@zeroshade
Copy link
Member

Currently in the Go-based drivers, the CMakeLists.txt specifies LDFLAGS of -s -w but only for CONFIG:Release. RelWithDebugInfo won't count for that right? We wouldn't want to keep those options when we want to maintain the debug info

@lidavidm
Copy link
Member Author

Currently in the Go-based drivers, the CMakeLists.txt specifies LDFLAGS of -s -w but only for CONFIG:Release. RelWithDebugInfo won't count for that right? We wouldn't want to keep those options when we want to maintain the debug info

Yeah, it's comparing specifically against "Release" so RelWithDebugInfo shouldn't count.

@lidavidm lidavidm force-pushed the gh-1583 branch 2 times, most recently from f891515 to b9ccea0 Compare March 26, 2024 16:38
@zeroshade
Copy link
Member

That's what I thought, just wanted to confirm.

@lidavidm lidavidm force-pushed the gh-1583 branch 2 times, most recently from f736dd0 to 1ab68a4 Compare March 26, 2024 18:32
@lidavidm lidavidm merged commit e29c031 into apache:main Mar 26, 2024
61 checks passed
@lidavidm lidavidm deleted the gh-1583 branch March 26, 2024 19:32
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.

python: consider a backtrace handler for debugging
3 participants