Add the ability to run tests on the standalone Dart VM under Address Sanitizer, Memory Sanitizer or Thread Sanitizer.#2575
Conversation
PR HealthChangelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. This check can be disabled by tagging the PR with |
739b45e to
c10e70a
Compare
71c3dbd to
123dfdb
Compare
natebosch
left a comment
There was a problem hiding this comment.
Can you edit the first comment on the PR to add some details about this change and the motivation? We can use the first comment to decide the content of the final commit message when this lands.
| process.exitCode.then((exitCode) async { | ||
| if (exitCode != 0) { | ||
| // At least don't hang. | ||
| exit(exitCode); |
There was a problem hiding this comment.
We have had difficulty in the past using a direct exit call from the test runner, I don't think we should land another.
What are the scenarios where this would hang?
There was a problem hiding this comment.
When the sanitizers exit the child, the parent was hanging. This is hacky, and probably the real issue is the consumer of the socket created between parent and child is failing to react to a disconnect, but I haven't found that code yet.
There was a problem hiding this comment.
Is the repro for this the one in #2577 ?
I started digging on that before the holidays but I hadn't made much progress. I'll spend some more time on it and see if I can make any headway. I'd prefer if we can land this without a potential call to exit but if we need something in the short term we can maybe leave it as a TODO.
There was a problem hiding this comment.
Yeah. Anything that crashes or directly exits with -c exe should reproduce the issue. This is exit code forwarding is at least limited to -c exe and so won't affect the default way of running.
123dfdb to
f7b2b78
Compare
ebf452d to
879394d
Compare
879394d to
1c4ebf3
Compare
db84acc to
2fc1eb8
Compare
…Sanitizer, Memory Sanitizer or Thread Sanitizer.
2fc1eb8 to
b2f4c84
Compare
Should no longer hang following dart-lang#2606
| void main() { | ||
| group( | ||
| 'asan sanitized', | ||
| skip: !File('$sdkDir/bin/dartaotruntime_asan').existsSync(), |
There was a problem hiding this comment.
Is there an SDK version number we can check or is it not pinned down yet? I'm a little worried about an SDK refactor skipping these tests with no one noticing.
There was a problem hiding this comment.
3.11. But which SDKs have which sanitizers is also architecture specific (mostly a function of what clang supports), such that I think testing for its presence is the least brittle option. E.g., Linux RISCV64 lacks MSAN and Linux ARM lacks all.
(Though it looks like this repo doesn't test on Linux ARM64 or Mac X64, even though we support those platforms and there are standard GitHub runners for them.)
There was a problem hiding this comment.
(Though it looks like this repo doesn't test on Linux ARM64 or Mac X64, even though we support those platforms and there are standard GitHub runners for them.)
Likely an oversight - happy to take PRs expanding the architectures we test on.
But which SDKs have which sanitizers is also architecture specific
OK, we'll run the the file checks for now in that case.
No longer forwarding the exact exitCode value of `6`.
|
Exit changes LGTM |
This is useful for finding issues when using foreign libraries through dart:ffi, such as use-after-free, use of uninitialized memory and data races.
This is also useful for pure Dart programs that might have data races through the use of shared fields.
Only available on 64-bit Linux.
https://clang.llvm.org/docs/AddressSanitizer.html
https://clang.llvm.org/docs/MemorySanitizer.html
https://clang.llvm.org/docs/ThreadSanitizer.html