-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
implement a ubsan runtime for better error messages #22488
Open
Rexicon226
wants to merge
18
commits into
ziglang:master
Choose a base branch
from
Rexicon226:ubsan-rt
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+764
−20
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
also seems to work around aarch64 LLVM miscompilation 🤔
Unlike `compiler-rt`, `ubsan` uses the standard library quite a lot. Using a similar approach to how `compiler-rt` is handled today, where it's compiled into its own object and then linked would be sub-optimal as we'd be introducing a lot of code bloat. This approach always "imports" `ubsan` if the ZCU, if it exists. If it doesn't such as the case where we're compiling only C code, then we have no choice other than to compile it down to an object and link. There's still a tiny optimization we can do in that case, which is when compiling to a static library, there's no need to construct an archive with a single object. We'd only go back and parse out ubsan from the archive later in the pipeline. So we compile it to an object instead and link that to the static library. TLDR; - `zig build-exe foo.c` -> build `libubsan.a` and links - `zig build-obj foo.c` -> doesn't build anything, just emits references to ubsan runtime - `zig build-lib foo.c -static` -> build `ubsan.o` and link it - `zig build-exe foo.zig bar.c` -> import `ubsan-rt` into the ZCU - `zig build-obj foo.zig bar.c` -> import `ubsan-rt` into the ZCU - `zig build-lib foo.zig bar.c` -> import `ubsan-rt` into the ZCU
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
closes #5163
So far, I've opted to copy the error messages that LLVM's ubsan runtime uses verbatim. I think they make sense, but we might be able to improve some of them.
As discussed on the Zulip, we've opted for the following strategies.
Regarding the logic for including the runtime, since
ubsan-rt
pulls in a lot of the standard library for the stack traces, I've set it up to do the following:zig build-exe foo.c
-> buildlibubsan.a
and linkszig build-obj foo.c
-> doesn't build anything, just emits references to ubsan runtimezig build-lib foo.c -static
-> buildubsan.o
and link itzig build-exe foo.zig bar.c
-> importubsan-rt
into the ZCUzig build-obj foo.zig bar.c
-> importubsan-rt
into the ZCUzig build-lib foo.zig bar.c
-> importubsan-rt
into the ZCURegarding what defaults to use for including ubsan:
Debug
mode, the full runtime is included, with only a couple of specific checks disabled for reasons explained in Compilation.zigReleaseSafe
mode, the full runtime is disabled opting instead to trap on detected UB.ReleaseFast
andReleaseSmall
, the ubsan runtime is not included in any capacity.@mlugg has a proposal for allowing enabling other configurations of the ubsan, maybe he'll open it up as a proposal :)