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

danakj · 2023-09-07T14:19:35Z

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 · 2023-09-07T14:19:44Z

r? @cjgillot

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

rustbot · 2023-09-07T14:19:48Z

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.

danakj · 2023-09-07T14:37:43Z

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 · 2023-09-07T17:50:44Z

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

danakj · 2023-09-07T18:05:46Z

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

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.

src/bootstrap/compile.rs

tmiasko · 2023-09-10T14:39:16Z

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 · 2023-09-11T15:16:35Z

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 · 2023-09-12T17:41:13Z

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

thomcc

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

cjgillot · 2023-09-16T08:57:07Z

Thanks for the review @thomcc.
@bors r=cjgillot,thomcc

bors · 2023-09-16T08:57:09Z

📌 Commit 1365771 has been approved by cjgillot,thomcc

It is now in the queue for this repository.

bors · 2023-09-17T17:06:53Z

⌛ Testing commit 1365771 with merge 02bd00b...

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 · 2023-09-17T18:54:09Z

☀️ Test successful - checks-actions
Approved by: cjgillot,thomcc
Pushing 02bd00b to master...

bors · 2023-09-17T18:55:09Z

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

rust-timer · 2023-09-17T20:43:49Z

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 · 2023-09-18T00:47:29Z

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

tmiasko · 2023-09-18T07:06:06Z

@bors retry

422 Update is not a fast forward

bors · 2023-09-18T08:54:27Z

⌛ Testing commit 1365771 with merge cf21090...

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 · 2023-09-18T10:44:12Z

☀️ Test successful - checks-actions
Approved by: cjgillot,thomcc
Pushing cf21090 to master...

bors · 2023-09-18T10:45:12Z

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

rust-timer · 2023-09-18T12:05:29Z

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%)

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.

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

danakj · 2023-09-18T13:40:13Z

422 Update is not a fast forward

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

danakj · 2023-09-18T15:20:14Z

@bors retry

bors · 2023-09-18T15:20:16Z

@danakj: 🔑 Insufficient privileges: not in try users

cjgillot · 2023-09-18T17:16:27Z

@bors r=cjgillot,thomcc

bors · 2023-09-18T17:16:29Z

📌 Commit b7e98e1 has been approved by cjgillot,thomcc

It is now in the queue for this repository.

bors · 2023-09-19T01:47:26Z

⌛ Testing commit b7e98e1 with merge 19dd953...

bors · 2023-09-19T03:52:39Z

☀️ Test successful - checks-actions
Approved by: cjgillot,thomcc
Pushing 19dd953 to master...

rust-timer · 2023-09-19T05:14:53Z

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%)

Pkgsrc changes: * Remove NetBSD-8 support (embedded LLVm requires newer C++ than what is in -8; it's conceivable that this could still build with an external LLVM) * undo powerpc 9.0 file naming tweak, since we no longer support -8. * Remove patch to LLVM for powerpc now included by upstream. * Minor adjustments, checksum changes etc. Upstream changes: Version 1.74.1 (2023-12-07) =========================== - [Resolved spurious STATUS_ACCESS_VIOLATIONs in LLVM] (rust-lang/rust#118464) - [Clarify guarantees for std::mem::discriminant] (rust-lang/rust#118006) - [Fix some subtyping-related regressions] (rust-lang/rust#116415) Version 1.74.0 (2023-11-16) ========================== Language -------- - [Codify that `std::mem::Discriminant<T>` does not depend on any lifetimes in T] (rust-lang/rust#104299) - [Replace `private_in_public` lint with `private_interfaces` and `private_bounds` per RFC 2145] (rust-lang/rust#113126) Read more in [RFC 2145](https://rust-lang.github.io/rfcs/2145-type-privacy.html). - [Allow explicit `#[repr(Rust)]`] (rust-lang/rust#114201) - [closure field capturing: don't depend on alignment of packed fields] (rust-lang/rust#115315) - [Enable MIR-based drop-tracking for `async` blocks] (rust-lang/rust#107421) Compiler -------- - [stabilize combining +bundle and +whole-archive link modifiers] (rust-lang/rust#113301) - [Stabilize `PATH` option for `--print KIND=PATH`] (rust-lang/rust#114183) - [Enable ASAN/LSAN/TSAN for `*-apple-ios-macabi`] (rust-lang/rust#115644) - [Promote loongarch64-unknown-none* to Tier 2] (rust-lang/rust#115368) - [Add `i686-pc-windows-gnullvm` as a tier 3 target] (rust-lang/rust#115687) Libraries --------- - [Implement `From<OwnedFd/Handle>` for ChildStdin/out/err] (rust-lang/rust#98704) - [Implement `From<{&,&mut} [T; N]>` for `Vec<T>` where `T: Clone`] (rust-lang/rust#111278) - [impl Step for IP addresses] (rust-lang/rust#113748) - [Implement `From<[T; N]>` for `Rc<[T]>` and `Arc<[T]>`] (rust-lang/rust#114041) - [`impl TryFrom<char> for u16`] (rust-lang/rust#114065) - [Stabilize `io_error_other` feature] (rust-lang/rust#115453) - [Stabilize the `Saturating` type] (rust-lang/rust#115477) - [Stabilize const_transmute_copy] (rust-lang/rust#115520) Stabilized APIs --------------- - [`core::num::Saturating`] (https://doc.rust-lang.org/stable/std/num/struct.Saturating.html) - [`impl From<io::Stdout> for std::process::Stdio`] (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStdout%3E-for-Stdio) - [`impl From<io::Stderr> for std::process::Stdio`] (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStderr%3E-for-Stdio) - [`impl From<OwnedHandle> for std::process::Child{Stdin, Stdout, Stderr}`] (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStderr%3E-for-Stdio) - [`impl From<OwnedFd> for std::process::Child{Stdin, Stdout, Stderr}`] (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStderr%3E-for-Stdio) - [`std::ffi::OsString::from_encoded_bytes_unchecked`] (https://doc.rust-lang.org/stable/std/ffi/struct.OsString.html#method.from_encoded_bytes_unchecked) - [`std::ffi::OsString::into_encoded_bytes`] (https://doc.rust-lang.org/stable/std/ffi/struct.OsString.html#method.into_encoded_bytes) - [`std::ffi::OsStr::from_encoded_bytes_unchecked`] (https://doc.rust-lang.org/stable/std/ffi/struct.OsStr.html#method.from_encoded_bytes_unchecked) - [`std::ffi::OsStr::as_encoded_bytes`] (https://doc.rust-lang.org/stable/std/ffi/struct.OsStr.html#method.as_encoded_bytes) - [`std::io::Error::other`] (https://doc.rust-lang.org/stable/std/io/struct.Error.html#method.other) - [`impl TryFrom<char> for u16`] (https://doc.rust-lang.org/stable/std/primitive.u16.html#impl-TryFrom%3Cchar%3E-for-u16) - [`impl<T: Clone, const N: usize> From<&[T; N]> for Vec<T>`] (https://doc.rust-lang.org/stable/std/vec/struct.Vec.html#impl-From%3C%26%5BT;+N%5D%3E-for-Vec%3CT,+Global%3E) - [`impl<T: Clone, const N: usize> From<&mut [T; N]> for Vec<T>`] (https://doc.rust-lang.org/stable/std/vec/struct.Vec.html#impl-From%3C%26mut+%5BT;+N%5D%3E-for-Vec%3CT,+Global%3E) - [`impl<T, const N: usize> From<[T; N]> for Arc<[T]>`] (https://doc.rust-lang.org/stable/std/sync/struct.Arc.html#impl-From%3C%5BT;+N%5D%3E-for-Arc%3C%5BT%5D,+Global%3E) - [`impl<T, const N: usize> From<[T; N]> for Rc<[T]>`] (https://doc.rust-lang.org/stable/std/rc/struct.Rc.html#impl-From%3C%5BT;+N%5D%3E-for-Rc%3C%5BT%5D,+Global%3E) These APIs are now stable in const contexts: - [`core::mem::transmute_copy`] (https://doc.rust-lang.org/beta/std/mem/fn.transmute_copy.html) - [`str::is_ascii`] (https://doc.rust-lang.org/beta/std/primitive.str.html#method.is_ascii) - [`[u8]::is_ascii`] (https://doc.rust-lang.org/beta/std/primitive.slice.html#method.is_ascii) Cargo ----- - [fix: Set MSRV for internal packages] (rust-lang/cargo#12381) - [config: merge lists in precedence order] (rust-lang/cargo#12515) - [fix(update): Clarify meaning of --aggressive as --recursive] (rust-lang/cargo#12544) - [fix(update): Make `-p` more convenient by being positional] (rust-lang/cargo#12545) - [feat(help): Add styling to help output ] (rust-lang/cargo#12578) - [feat(pkgid): Allow incomplete versions when unambigious] (rust-lang/cargo#12614) - [feat: stabilize credential-process and registry-auth] (rust-lang/cargo#12649) - [feat(cli): Add '-n' to dry-run] (rust-lang/cargo#12660) - [Add support for `target.'cfg(..)'.linker`] (rust-lang/cargo#12535) - [Stabilize `--keep-going`] (rust-lang/cargo#12568) - [feat: Stabilize lints] (rust-lang/cargo#12648) Rustdoc ------- - [Add warning block support in rustdoc] (rust-lang/rust#106561) - [Accept additional user-defined syntax classes in fenced code blocks] (rust-lang/rust#110800) - [rustdoc-search: add support for type parameters] (rust-lang/rust#112725) - [rustdoc: show inner enum and struct in type definition for concrete type] (rust-lang/rust#114855) Compatibility Notes ------------------- - [Raise minimum supported Apple OS versions] (rust-lang/rust#104385) - [make Cell::swap panic if the Cells partially overlap] (rust-lang/rust#114795) - [Reject invalid crate names in `--extern`] (rust-lang/rust#116001) - [Don't resolve generic impls that may be shadowed by dyn built-in impls] (rust-lang/rust#114941) 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.

rustbot assigned cjgillot Sep 7, 2023

This comment has been minimized.

Sign in to view

danakj force-pushed the catalyst-asan branch from 2449892 to dcbe9f7 Compare September 7, 2023 14:35

thomcc requested changes Sep 10, 2023

View reviewed changes

src/bootstrap/compile.rs Outdated Show resolved Hide resolved

danakj force-pushed the catalyst-asan branch from 206a662 to 1365771 Compare September 11, 2023 14:33

danakj requested a review from thomcc September 12, 2023 17:41

thomcc approved these changes Sep 15, 2023

View reviewed changes

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 added the merged-by-bors This PR was explicitly merged by bors. label Sep 17, 2023

tmiasko removed the merged-by-bors This PR was explicitly merged by bors. label Sep 18, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 18, 2023

danakj added 2 commits September 18, 2023 09:38

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

4eb1b52

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.

Document that the macabi sanitizers are shared with darwin

b7e98e1

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

danakj force-pushed the catalyst-asan branch from 1365771 to b7e98e1 Compare September 18, 2023 13:41

bors merged commit 19dd953 into rust-lang:master Sep 19, 2023
11 checks passed

rustbot added this to the 1.74.0 milestone Sep 19, 2023

bors mentioned this pull request Sep 19, 2023

compiletest: load supported sanitizers from target spec #115695

Merged

danakj deleted the catalyst-asan branch September 19, 2023 15:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

danakj commented Sep 7, 2023 •

edited

Loading

rustbot commented Sep 7, 2023

rustbot commented Sep 7, 2023

This comment has been minimized.

danakj commented Sep 7, 2023

danakj commented Sep 7, 2023

danakj commented Sep 7, 2023

thomcc left a comment

tmiasko commented Sep 10, 2023

danakj commented Sep 11, 2023 •

edited

Loading

danakj commented Sep 12, 2023 •

edited

Loading

thomcc left a comment

cjgillot commented Sep 16, 2023

bors commented Sep 16, 2023

bors commented Sep 17, 2023

bors commented Sep 17, 2023

bors commented Sep 17, 2023

rust-timer commented Sep 17, 2023

danakj commented Sep 18, 2023

tmiasko commented Sep 18, 2023

bors commented Sep 18, 2023

bors commented Sep 18, 2023

bors commented Sep 18, 2023

rust-timer commented Sep 18, 2023

danakj commented Sep 18, 2023

danakj commented Sep 18, 2023

bors commented Sep 18, 2023

cjgillot commented Sep 18, 2023

bors commented Sep 18, 2023

bors commented Sep 19, 2023

bors commented Sep 19, 2023

rust-timer commented Sep 19, 2023

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

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

Conversation

danakj commented Sep 7, 2023 • edited Loading

rustbot commented Sep 7, 2023

rustbot commented Sep 7, 2023

This comment has been minimized.

danakj commented Sep 7, 2023

danakj commented Sep 7, 2023

danakj commented Sep 7, 2023

thomcc left a comment

Choose a reason for hiding this comment

tmiasko commented Sep 10, 2023

danakj commented Sep 11, 2023 • edited Loading

danakj commented Sep 12, 2023 • edited Loading

thomcc left a comment

Choose a reason for hiding this comment

cjgillot commented Sep 16, 2023

bors commented Sep 16, 2023

bors commented Sep 17, 2023

bors commented Sep 17, 2023

bors commented Sep 17, 2023

rust-timer commented Sep 17, 2023

Overall result: no relevant changes - no action needed

danakj commented Sep 18, 2023

tmiasko commented Sep 18, 2023

bors commented Sep 18, 2023

bors commented Sep 18, 2023

bors commented Sep 18, 2023

rust-timer commented Sep 18, 2023

Overall result: no relevant changes - no action needed

danakj commented Sep 18, 2023

danakj commented Sep 18, 2023

bors commented Sep 18, 2023

cjgillot commented Sep 18, 2023

bors commented Sep 18, 2023

bors commented Sep 19, 2023

bors commented Sep 19, 2023

rust-timer commented Sep 19, 2023

Overall result: no relevant changes - no action needed

danakj commented Sep 7, 2023 •

edited

Loading

danakj commented Sep 11, 2023 •

edited

Loading

danakj commented Sep 12, 2023 •

edited

Loading