Actually abort in -Zpanic-abort-tests #120326

tmandry · 2024-01-25T00:13:04Z

When a test fails in panic=abort, it can be useful to have a debugger or other tooling hook into the abort() call for debugging. Doing this some other way would require it to hard code details of Rust's panic machinery.

There's no reason we couldn't have done this in the first place; using a single exit code for "success" or "failed" was just simpler. Now we are aware of the special exit codes for posix and windows platforms, logging a special error if an unrecognized code is used on those platforms, and falling back to just "failure" on other platforms.

This continues to account for #[should_panic] inside the test process itself, so there's no risk of misinterpreting a random call to abort() as an expected panic. Any exit code besides TR_OK is logged as a test failure.

As an added benefit, this would allow us to support panic=immediate_abort (but not #[should_panic]), without noise about unexpected exit codes when a test fails.

rustbot · 2024-01-25T00:13:12Z

r? @Mark-Simulacrum

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

tmandry · 2024-01-25T00:30:39Z

Testing windows support..

@bors try

tmandry · 2024-01-25T00:31:05Z

r? libs

Actually abort in panic-abort-tests When a test fails in panic=abort, it can be useful to have a debugger or other tooling hook into the `abort()` call for debugging. There's no reason we couldn't have done this in the first place; using a single exit code for "success" or "failed" was just simpler. Now we are aware of the special exit codes for posix and windows platforms, logging a special error if an unrecognized code is used on those platforms, and falling back to just "failure" on other platforms. This continues to account for `#[should_panic]` inside the test process itself, so there's no risk of misinterpreting a random call to `abort()` as an expected panic. Any exit code besides `TR_OK` is logged as a test failure.

bors · 2024-01-25T00:31:52Z

⌛ Trying commit bb332f3 with merge 11f3f41...

bors · 2024-01-25T01:19:02Z

💔 Test failed - checks-actions

tmandry · 2024-01-26T01:08:13Z

@bors try

bors · 2024-01-26T01:09:23Z

⌛ Trying commit 9610404 with merge 765d6e6...

Actually abort in -Zpanic-abort-tests When a test fails in panic=abort, it can be useful to have a debugger or other tooling hook into the `abort()` call for debugging. Doing this some other way would require it to hard code details of Rust's panic machinery. There's no reason we couldn't have done this in the first place; using a single exit code for "success" or "failed" was just simpler. Now we are aware of the special exit codes for posix and windows platforms, logging a special error if an unrecognized code is used on those platforms, and falling back to just "failure" on other platforms. This continues to account for `#[should_panic]` inside the test process itself, so there's no risk of misinterpreting a random call to `abort()` as an expected panic. Any exit code besides `TR_OK` is logged as a test failure. As an added benefit, this would allow us to support panic=immediate_abort (but not `#[should_panic]`), without noise about unexpected exit codes when a test fails.

bors · 2024-01-26T02:40:57Z

☀️ Try build successful - checks-actions
Build commit: 765d6e6 (765d6e6ebd889c8e1783ca96400086e9a96b2bc1)

bors · 2024-02-06T04:15:44Z

⌛ Testing commit 48f664b with merge ff95e52...

bors · 2024-02-06T06:19:56Z

☀️ Test successful - checks-actions
Approved by: cuviper
Pushing ff95e52 to master...

rust-timer · 2024-02-06T08:30:00Z

Finished benchmarking commit (ff95e52): 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)	1.9%	[1.9%, 1.9%]	1
Regressions ❌ (secondary)	3.2%	[3.2%, 3.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.9%	[1.9%, 1.9%]	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: 662.71s -> 662.079s (-0.10%)
Artifact size: 308.15 MiB -> 308.06 MiB (-0.03%)

Update Android in CI We are currently using a 10+ year old Android image, and it has caused trouble when working on rust-lang#120326. Our current NDK (25) only supports API 19+, so we were already out of spec. This PR: 1. Bumps the API used by the emulator in CI to 21, as per [NDK-26's release notes](https://github.com/android/ndk/wiki/Changelog-r26) deprecating 19 and 20 as targets. 2. Activates aarch64 testing on the emulator, since the base image is now a 64-bit image. 3. Bumps the NDK to 26b

We were running testing on API 18, which was already out of support for NDK 25, and some of the ancient behavior in that image was causing trouble when developing `rustc` features (rust-lang#120326). We'll be updating to NDK 26 (latest LTS) shortly, so rather than updating to API 19, then again to API 21 in a bit, just jump straight to 21. Fixes: rust-lang#120567

In updating our image in rust-lang#120326 we were forced off the 32-bit image. This means it is not too difficult to add testing for 64-bit code as well. Since this architecture is more used, and on later images, the only supported kind of executable, we should start testing this now.

…um,workingjubilee Update Android in CI We are currently using a 10+ year old Android image, and it has caused trouble when working on rust-lang#120326. Our current NDK (25) only supports API 19+, so we were already out of spec. This PR: 1. Bumps the API used by the emulator in CI to 21, as per [NDK-26's release notes](https://github.com/android/ndk/wiki/Changelog-r26) deprecating 19 and 20 as targets. 2. Activates aarch64 testing on the emulator, since the base image is now a 64-bit image. 3. Bumps the NDK to 26b