Enable ASAN/LSAN/TSAN for *-apple-ios-macabi by danakj · Pull Request #115644 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation35 Commits2 Checks0 Files changed

Conversation

This file contains hidden or 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

[ Show hidden characters]({{ revealButtonHref }})

danakj

The -macabi targets are iOS running on MacOS, and they use the runtime libraries for MacOS, thus they have the same sanitizers available as the *-apple-darwin targets.

This is based on the work of aacf321.

Closes #113935.

@rustbot

r? @cjgillot

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-testsuite

Area: The testsuite used to check the correctness of rustc

S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

T-bootstrap

Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

T-compiler

Relevant to the compiler team, which will review and decide on the PR/issue.

labels

Sep 7, 2023

@rustbot

These commits modify compiler targets.
(See the Target Tier Policy.)

This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp.

@rust-log-analyzer

This comment has been minimized.

@danakj

Along the way to writing this CL, I added the targets to the compiletest/src/util.rs file but didn't add them to the spec/ files yet. At that point trying to build for asan still failed with the unsupported message (expected) but compiletests all passed. So I am not sure that this is being tested, any thoughts?

@danakj

I have verified that building Chromium with Rust in the build, targeting ios-macabi with ASAN does link and run after this change.

@danakj

This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp.

I think this may be a false positive, we're pointing to the asan libs and causing them to be built maybe instead of not when targeting *-apple-ios-macabi, but let me know if I am wrong and this does require touching the download-ci-llvm-stamp please.

thomcc

Choose a reason for hiding this comment

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

Surprised this works at all. Shouldn't this mean the sanitizer runtimes aren't present and thus things won't load (and possibly even fail to link)? Actually, when this was tested, it was linked into chromium, so it's likely the chromium C++ build provided the sanitizer runtime.

After this is fixed, a smoke-test against pure-rust use should be done.

But in general I'm in favor of allowing this, so long as it works (once fixed and smoke tested). And like, it's an unstable feature on a tier 3 target too, so there's relatively low risk to accept.

@tmiasko

At that point trying to build for asan still failed with the unsupported message (expected) but compiletests all passed. So I am not sure that this is being tested, any thoughts?

With verbose-tests in config.toml compiletest will display a reason why tests was ignored. Sanitizers also need to be enabled in config.toml, otherwise tests with needs-sanitizer-support header will be ignored.

@danakj

Trying to do a stand-alone Rust test and ./x.py test --target aarch64-apple-ios-macabi fails without any of my changes.

Building test helpers for aarch64-apple-ios-macabi
running: "xcrun" "--show-sdk-path" "--sdk" "macosx"
exit status: 0
running: "clang" "-O0" "-fPIC" "--target=arm64-apple-ios13.0-macabi" "-isysroot" "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk" "-fembed-bitcode" "-o" "/Users/danakj/s/rust/build/aarch64-apple-ios-macabi/native/rust-test-helpers/rust_test_helpers.o" "-c" "/Users/danakj/s/rust/tests/auxiliary/rust_test_helpers.c"
cargo:warning=clang: error: invalid version number in '--target=arm64-apple-ios13.0-macabi'
exit status: 1


error occurred: Command "clang" "-O0" "-fPIC" "--target=arm64-apple-ios13.0-macabi" "-isysroot" "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk" "-fembed-bitcode" "-o" "/Users/danakj/s/rust/build/aarch64-apple-ios-macabi/native/rust-test-helpers/rust_test_helpers.o" "-c" "/Users/danakj/s/rust/tests/auxiliary/rust_test_helpers.c" with args "clang" did not execute successfully (status code exit status: 1).

I just copied the config.example.toml and added verbose-tests=true and sanitizers=true.

However ./x.py install --target aarch64-apple-ios-macabi does work. So I can write my own test to verify the runtime is working at least.

I did:

cargo new --bin asan-test

Made a config.toml:

$ cat .cargo/config.toml
[build]
target = "aarch64-apple-ios-macabi"

[target.aarch64-apple-ios-macabi]
rustflags = [
    "-Zsanitizer=address",
    "-Clinker=clang",
    "-Clink-args=-isysroot",
    "-Clink-arg=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk"
    ]

And build and run it:

$ RUSTC=$HOME/rustbin/bin/rustc RUSTFLAGS=--sysroot=$HOME/rustbin cargo run
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
     Running `target/aarch64-apple-ios-macabi/debug/asan-test`
Hello, world!

@danakj

Bump for review? Oh there's github UI to do this.

thomcc

Choose a reason for hiding this comment

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

Looks fine to me. Not a compiler/* reviewer so I won't r+ it.

@cjgillot

@bors

📌 Commit 1365771 has been approved by cjgillot,thomcc

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Sep 16, 2023

@bors

bors added a commit to rust-lang-ci/rust that referenced this pull request

Sep 17, 2023

@bors

Enable ASAN/LSAN/TSAN for *-apple-ios-macabi

The -macabi targets are iOS running on MacOS, and they use the runtime libraries for MacOS, thus they have the same sanitizers available as the *-apple-darwin targets.

This is based on the work of rust-lang@aacf321.

Closes rust-lang#113935.

@bors

@bors

👀 Test was successful, but fast-forwarding failed: 422 Update is not a fast forward

@rust-timer

Finished benchmarking commit (02bd00b): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌ (primary) - - 0
Regressions ❌ (secondary) 3.3% [3.2%, 3.5%] 2
Improvements ✅ (primary) - - 0
Improvements ✅ (secondary) -2.9% [-2.9%, -2.9%] 1
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 635.505s -> 635.947s (0.07%)
Artifact size: 318.39 MiB -> 318.50 MiB (0.03%)

@danakj

Thank you so much. Rust for iOS chromium is coming up. 😊

@tmiasko

@bors retry

422 Update is not a fast forward

@bors

bors added a commit to rust-lang-ci/rust that referenced this pull request

Sep 18, 2023

@bors

Enable ASAN/LSAN/TSAN for *-apple-ios-macabi

The -macabi targets are iOS running on MacOS, and they use the runtime libraries for MacOS, thus they have the same sanitizers available as the *-apple-darwin targets.

This is based on the work of rust-lang@aacf321.

Closes rust-lang#113935.

@bors

@bors

👀 Test was successful, but fast-forwarding failed: 422 Update is not a fast forward

@rust-timer

Finished benchmarking commit (cf21090): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌ (primary) 2.4% [2.4%, 2.4%] 1
Regressions ❌ (secondary) 3.2% [2.7%, 3.4%] 5
Improvements ✅ (primary) - - 0
Improvements ✅ (secondary) - - 0
All ❌✅ (primary) 2.4% [2.4%, 2.4%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 633.229s -> 636.08s (0.45%)
Artifact size: 317.99 MiB -> 317.94 MiB (-0.02%)

@danakj

The -macabi targets are iOS running on MacOS, and they use the runtime libraries for MacOS, thus they have the same sanitizers available as the *-apple-darwin targets.

@danakj

Do not rename and resign the darwin sanitizers a second time for macabi.

@danakj

422 Update is not a fast forward

Hm I tried rebasing but there's no conflicts. I will upload the rebase anyhow.

@danakj

@bors

@danakj: 🔑 Insufficient privileges: not in try users

@cjgillot

@bors

📌 Commit b7e98e1 has been approved by cjgillot,thomcc

It is now in the queue for this repository.

@bors

@bors

@bors bors mentioned this pull request

Sep 19, 2023

@rust-timer

Finished benchmarking commit (19dd953): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌ (primary) - - 0
Regressions ❌ (secondary) - - 0
Improvements ✅ (primary) - - 0
Improvements ✅ (secondary) -3.8% [-3.8%, -3.8%] 1
All ❌✅ (primary) - - 0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌ (primary) - - 0
Regressions ❌ (secondary) 1.6% [1.6%, 1.6%] 1
Improvements ✅ (primary) -1.4% [-1.4%, -1.4%] 1
Improvements ✅ (secondary) - - 0
All ❌✅ (primary) -1.4% [-1.4%, -1.4%] 1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 631.066s -> 633.02s (0.31%)
Artifact size: 317.89 MiB -> 317.91 MiB (0.00%)

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request

Jan 8, 2024

@he32

Pkgsrc changes:

Upstream changes:

Version 1.74.1 (2023-12-07)

Version 1.74.0 (2023-11-16)

Language

Compiler

Libraries

Stabilized APIs

These APIs are now stable in const contexts:

Cargo

Rustdoc

Compatibility Notes

Internal Changes

These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools.

None this cycle.

Labels

A-testsuite

Area: The testsuite used to check the correctness of rustc

merged-by-bors

This PR was explicitly merged by bors.

S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

T-bootstrap

Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

T-compiler

Relevant to the compiler team, which will review and decide on the PR/issue.