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

Compile libunwind in release mode #102

Merged
merged 1 commit into from
Jan 27, 2025
Merged

Compile libunwind in release mode #102

merged 1 commit into from
Jan 27, 2025

Conversation

etan-status
Copy link
Contributor

Default build is DEBUG. Switch to RelWithDebInfo.

**CMAKE_BUILD_TYPE**:STRING
  Sets the build type for ``make`` based generators. Possible values are
  Release, Debug, RelWithDebInfo and MinSizeRel. On systems like Visual Studio
  the user sets the build type with the IDE settings.

Default build is `DEBUG`. Switch to `RelWithDebInfo`.

```
**CMAKE_BUILD_TYPE**:STRING
  Sets the build type for ``make`` based generators. Possible values are
  Release, Debug, RelWithDebInfo and MinSizeRel. On systems like Visual Studio
  the user sets the build type with the IDE settings.
```
@etan-status etan-status enabled auto-merge (squash) January 27, 2025 12:11
@etan-status etan-status merged commit 616e5f4 into master Jan 27, 2025
20 checks passed
@etan-status etan-status deleted the dev/etan/bs-rel branch January 27, 2025 12:18
@arnetheduck
Copy link
Member

How about using {.compile.} directives instead of cmake to build libunwind? cc @etan-status

@etan-status
Copy link
Contributor Author

Goal was to update libunwind to latest version as the used version was 4 years old, with minimal changes otherwise. The Makefile was not changed in how it works, as in, it's not like {.compile.} suddenly becomes a better choice (if it was, it was for the last 4 years).

As part of the bumps, there were 2 build system changes upstream, with the standalone build that was used historically initially deprecated and later removed. There's a ton of upstream CMake scripts in vendor/libunwind/{cmake, llvm/cmake, llvm/utils/lit-llvm, runtime} that would have to be analyzed and monitored on further bumps. Switching to {.compile.} would bring a continuous maintenance burden that would have to be analyzed, as we would have to keep in sync with upstream CMake changes.

Further, he libunwind stuff is quite intertwined even into the projects that use nim-libbacktrace, see USE_VENDORED_LIBUNWIND and disable_libbacktrace. It is configured differently for crosscompiling via entry_point.sh via Docker, and it seems to only be pulled in for macOS / Windows, while on Linux the preinstalled libgcc version is used. That means that any efforts to optimize the build system would not impact our primary platform. The sections also are filled with comments suggesting more pitfalls, changing how the build works is possibly non-trivial.

There's also the #48 pointer authentication issue which is only present in a specific combination. Changing any of the variables gets rid of the problem. This should probably be addressed before doing more changes, to have a stable basis to compare against.

  1. Nim ≥ 2.0
  2. CPP backend (--exceptions:cpp is the only one that actually even compiles with that, setjmp and goto fail to compile with CPP mode)
  3. macOS
  4. ARM

And, then there's a more overarching question, whether we should even continue to use libunwind / libbacktrace at all. Because if compiled with -d:release the stacktraces start missing crucial info, such as the line where a Defect was originally thrown, unless -d:release is also paired with either -d:disable_libbacktrace (making exceptions / stacktraces slow but correct), or with -d:disableLTO (which prevents some optimization from happening). As is, stack traces from release builds aren't too useful as is. That should be discussed before investing more into libunwind wrapping.

@arnetheduck
Copy link
Member

it was for the last 4 years

this is more of a legacy decision that indeed isn't tied to the cmake upgrade specifically - in particular, it's a tradeoff between the complexity of the native build system and what nim is capable of doing and bugs in nim 4 years ago. {.compile.} has some minor downsides but makes it a lot easier to cross-compile and to keep C compiler options in sync - in particular, at the time this was written it wasn't possible to pass compile flags to a single C compilation - this exists now.

Re C++, the funny thing is that there's usually a compiler-provided copy of libunwind linked to the binary already since it's also used for exception unwinding - sometimes, the OS provides yet another version of the same thing, it's a bit of a mess.

should even continue to use

yes - the native nim solution is insanely/unusably slow

start missing crucial info

Many of these are actually nim compiler bugs where it generates invalid #line directives or forgets to line-break the C code etc - nevertheless, optimization and lto in particular indeed has a tendency to collapse similar functions which messes up the debugging info since some redundant function copies are removed - without the "native" nim stack frame cruft, they become identical.

I guess the problem here is that there are many overlapping bugs and platform-specific behaviors in stack frame unwinding and fixing one does not necessarily lead to an overall improvement until a sufficient number have been quashed.

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