Skip to content

Remove druntime's libunwind dependency#4691

Merged
JohanEngelen merged 3 commits intoldc-developers:masterfrom
JohanEngelen:rm_libunwind_dep
Jul 3, 2024
Merged

Remove druntime's libunwind dependency#4691
JohanEngelen merged 3 commits intoldc-developers:masterfrom
JohanEngelen:rm_libunwind_dep

Conversation

@JohanEngelen
Copy link
Member

No description provided.

@JohanEngelen
Copy link
Member Author

FYI: this is one of the stumbling blocks when trying to statically link with musl libc

// REQUIRED_ARGS(linux freebsd dragonflybsd): -L-export-dynamic
// LDC (required for Win32 and -O): REQUIRED_ARGS(windows32mscoff): -link-defaultlib-debug
// LDC (FreeBSD's libexecinfo apparently doesn't like elided frame pointers): REQUIRED_ARGS(freebsd): -link-defaultlib-debug -frame-pointer=all
// LDC (our _UnwindBacktrace implementation - for musl libc - requires this): REQUIRED_ARGS(linux): -link-defaultlib-debug
Copy link
Member Author

Choose a reason for hiding this comment

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

It'd be worthwhile to figure out what is causing the need for -link-defaultlib-debug on all these platforms. Perhaps we can solve this by preventing optimization (with magic UDA) of specific functions in druntime (or remove some other delta between optimized vs debug builds of specific functions).

Copy link
Member Author

Choose a reason for hiding this comment

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

(make new issue for that?)

Copy link
Member

Choose a reason for hiding this comment

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

Win32 backtraces are a catastrophe AFAIK, but I couldn't care less. For FreeBSD, it's the elided frame pointers as per the comment. For musl, no idea, but NOT testing the release variant for every Linux distro would IMO be a test regression.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah...
add the complexity of musl-specific parameters?

Copy link
Member

Choose a reason for hiding this comment

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

I don't know, that doesn't sound sexy at least. In the short term, I'd probably remove this test file for musl CI and file an issue, unless you wanna dig into the problem and try to fix it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I was already digging into it but it will cost more time than simple thing I tried. So let's remove it from CI, and file a separate issue to be solved in time.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed it by running the tests manually for now

@kinke
Copy link
Member

kinke commented Jul 3, 2024

Please let me know if you wanna include this in v1.39, then I'll wait with the release.

@JohanEngelen
Copy link
Member Author

Please let me know if you wanna include this in v1.39, then I'll wait with the release.

It would actually really be nice to get this in yes (minus the test change --> remove tests from musl CI for the time being).

@JohanEngelen JohanEngelen merged commit 7ad1a98 into ldc-developers:master Jul 3, 2024
@JohanEngelen JohanEngelen deleted the rm_libunwind_dep branch July 3, 2024 21:43
thewilsonator pushed a commit to dlang/dmd that referenced this pull request Jan 23, 2025
kinke added a commit that referenced this pull request Feb 15, 2025
…(dlang/dmd!20758)

Based on:
* #4639
* #4691

Co-authored-by: Martin Kinkelin <noone@nowhere.com>
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.

2 participants