AddressSanitizer support #3560
Merged
AddressSanitizer support #3560
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
|
I'll look at the runtime changes. |
Svetlitski
force-pushed
the
address-sanitizer-via-emit
branch
5 times, most recently
from
f996e58 to
fb6e329
Compare
gretay-js
reviewed
Feb 11, 2025
Svetlitski
force-pushed
the
address-sanitizer-via-emit
branch
4 times, most recently
from
ff9afd3 to
9f534d1
Compare
…Sanitizer integration
…ccesses by performing the load of `shadow_value` as part of the `cmp` instruction
… was invalid because its component registers had been clobbered
… convention used for the error-reporting stubs
…ield initialization stores
Svetlitski
force-pushed
the
address-sanitizer-via-emit
branch
from
9f534d1 to
32aac6b
Compare
gretay-js
approved these changes
Feb 13, 2025
|
I've read the runtime parts. This is ok once the runtime5 implementation has been tested. |
|
@mshinwell Tested things with runtime5 internally; everything checks out |
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.
This PR supersedes the earlier attempt at adding this functionality in #3033
Implement a new compiler configuration which integrates support for AddressSanitizer. When the compiler is configured with
--enable-address-sanitizerthe OCaml runtime is built against AddressSanitizer, and codegen is altered to instrument all loads and stores in accordance with the AddressSanitizer algorithm.The original implementation of this in #3033 ran into issues where certain files triggered pathological behavior in the register allocator when AddressSanitizer was enabled, resulting in the files taking up to several hours to compile where they had taken just seconds before. Having discussed this problem with @xclerc and @gretay-js and not finding an easy way to mitigate the issues in the register allocator, I've worked around this by completely reimplementing the AddressSanitizer instrumentation in a phase after register allocation. This avoids the pathological compile-times, but does come with some downsides, namely that the code is more complex and the runtime penalty is higher (internal testing on realistic benchmark I performed showed a 2.6x slowdown with my implementation in #3033, versus a 3x slowdown with this PR's implementation). There's definitely room for improvement in the runtime performance, but I'd rather get this out the door now since it works, and follow up with optimizations in future PRs.