More robust stack protector testing #147115

cezarbbb · 2025-09-28T03:31:51Z

I've added some tests related to the stack protector. These tests were originally in the LLVM stack protector test project.
These tests were written for the "Stabilize stack-protector" proposal, and therefore removed the "stack-protector=basic" test option, as this stack protector was considered ineffective in Rust.
For the proposal, see: #146369
For the discussion, see zulip: https://rust-lang.zulipchat.com/#narrow/channel/233931-t-compiler.2Fmajor-changes/topic/Proposal.20for.20Adapt.20Stack.20Protector.20for.20Ru.E2.80.A6.20compiler-team.23841

I have opened an issue to discuss the 'abi_mismatch' issue I encountered while writing tests for the coexistence of 'stack-protector' and 'safe-stack': #149340

r? @wesleywiser (feel free to reassign)
cc @nikic, @rcvalle, @davidtwco, @arielb1, @Darksonn, @Noratrieb, @SparrowLii

rustbot · 2025-09-28T03:31:56Z

wesleywiser is currently at their maximum review capacity.
They may take a while to respond.

SparrowLii · 2025-10-25T02:00:24Z

I'm looking this and gonna give some comments, expecting in the next week

SparrowLii

Please split the PR into multiple commits according to the logical organization of the test, which makes it easier for others to review.

And if your tests are coming from llvm tests, could you write something like a sheet to explain if these result in rustc are all expected?

View changes since this review

SparrowLii · 2025-10-31T08:54:51Z

tests/assembly-llvm/stack-protector/stack-protector-heuristics-effect-2.rs

+    fn dummy(_: ...) -> i32;
+
+    static STR: [u8; 1];
+}


Could you add some comments about why we need these extern functions?

The names of these functions have no special meaning; they simply correspond to the corresponding test functions in LLVM, because parameter passing checks are also an important part of checking whether stack protector is working properly (e.g., sspstrong).

SparrowLii · 2025-10-31T08:57:18Z

tests/assembly-llvm/stack-protector/stack-protector-heuristics-effect-2.rs

+// #[repr(C)]
+// struct A {
+//     data: [u8; 2],
+// }


Why are these code left here

Edited. Because it became unnecessary due to unnecessary testing.

SparrowLii · 2025-10-31T08:59:52Z

tests/assembly-llvm/stack-protector/stack-protector-heuristics-effect-2.rs

+}
+
+// Note: test2
+// struct -> flat aggregate -> array


These codes too, please add some comments to explain why keep these codes

The corresponding comments have been removed. The following explains why these tests are considered unnecessary.

test2/4: The struct structures in test2 and test4 will be "flat aggregated" into arrays, becoming identical to those in test1 and test3.

test17: There is no such type in rustc corresponding to '%struct.vec = type { <4 x i32> }' in LLVM.

test22: [2 x i8] in struct will be automatically optimized to i16 and will not trigger the sspstrong.

test23: [2 x i8] nested in several layers of structs and unions: same as test22.

test24: Variable sized alloca (VLA): see https://github.com/rust-lang/rfcs/pull/1909

test28/29/30/31: These tests were originally used to verify whether the 'basic' mode worked correctly. Since the community has decided to remove the 'basic' mode, these tests will also become useless.

SparrowLii · 2025-10-31T09:02:39Z

tests/assembly-llvm/stack-protector/stack-protector-heuristics-effect-2.rs

+
+// CHECK-LABEL: test3{{:|\[}}
+#[no_mangle]
+pub fn test3(a: *const u8) {


Can you make these test function names more meaningful? Or add some comments to explain

Each function has been commented accordingly.

SparrowLii · 2025-11-05T02:00:27Z

tests/assembly-llvm/stack-protector/stack-protector-heuristics-effect-2.rs

+// test3: array [4 x i8]
+// CHECK-LABEL: test3{{:|\[}}
+#[no_mangle]
+pub fn test3(a: *const u8) {


Since we delete some unneccesary tests, I think these function names are no need to be consistent with ones in llvm's tests. You can just name test2 or test_array4_i8.

Same for all functiosn below

SparrowLii

LGTM, I'd like to r+ this after fixing nits for test function names

View changes since this review

rust-bors · 2025-11-20T01:18:04Z

💔 Test for 63674d8 failed: CI. Failed jobs:

try - x86_64-gnu-llvm-21-3 (web logs, extended logs)

cezarbbb · 2025-11-20T01:39:30Z

The job x86_64-gnu-llvm-21-3 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

This is strange, as the KNOWN_DIRECTIVE_NAMES list shows an entry for "ignore-wasm-bare", but I changed it to "ignore-wasm32" to avoid a panic.

SparrowLii · 2025-11-21T08:04:19Z

@bors try jobs=test-various,x86_64-gnu-llvm-21-3

More robust stack protector testing try-job: test-various try-job: x86_64-gnu-llvm-21-3

rust-bors · 2025-11-21T08:35:15Z

💔 Test for 29e7f4d failed: CI. Failed jobs:

try - x86_64-gnu-llvm-21-3 (web logs, extended logs)

SparrowLii · 2025-11-26T01:28:17Z

For safe stack problem I think you can raise a new issue about it, with a FIXME in this PR. We can merge the other tests first.

cezarbbb · 2025-11-26T02:28:04Z

For safe stack problem I think you can raise a new issue about it, with a FIXME in this PR. We can merge the other tests first.

Done. See in PR description

SparrowLii · 2025-11-26T03:40:18Z

@bors try jobs=test-various,x86_64-gnu-llvm-21-3

More robust stack protector testing try-job: test-various try-job: x86_64-gnu-llvm-21-3

rust-bors · 2025-11-26T05:46:38Z

☀️ Try build successful (CI)
Build commit: 5d45ca5 (5d45ca5cfb5741d8af80231deda48d3323725af3, parent: 80d8f292d82d735f83417221dd63b0dd2bbb8dd2)

SparrowLii · 2025-11-26T05:54:16Z

@cezarbbb Can you squash the commits?

rustbot · 2025-11-26T09:11:50Z

⚠️ Warning ⚠️

This PR is based on an upstream commit that is older than 28 days.

It's recommended to update your branch according to the rustc-dev-guide.

SparrowLii · 2025-11-27T00:23:34Z

@bors r+

bors · 2025-11-27T00:23:37Z

📌 Commit be28e7f has been approved by SparrowLii

It is now in the queue for this repository.

Rollup of 12 pull requests Successful merges: - #147115 (More robust stack protector testing) - #148048 (Stabilize `maybe_uninit_write_slice`) - #148641 (Add a diagnostic attribute for special casing const bound errors for non-const impls) - #149074 (Add Command::get_env_clear) - #149097 (num: Implement `uint_gather_scatter_bits` feature for unsigned integers) - #149131 (optimize `slice::Iter::next_chunk`) - #149190 (Forbid `CHECK: br` and `CHECK-NOT: br` in codegen tests (suggest `br {{.*}}` instead)) - #149239 (clarify float min/max behavios for NaNs and signed zeros) - #149243 (Fix typo and clarify bootstrap change tracker entry) - #149301 (Motor OS: make decode_error_kind more comprehensive) - #149306 (bootstrap: Miri now handles jemalloc like everything else) - #149325 (rustdoc: add regression test for #140968) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #147115 - cezarbbb:stable_ssp, r=SparrowLii More robust stack protector testing I've added some tests related to the stack protector. These tests were originally in the LLVM stack protector test project. These tests were written for the "Stabilize stack-protector" proposal, and therefore removed the "stack-protector=basic" test option, as this stack protector was considered ineffective in Rust. For the proposal, see: #146369 For the discussion, see zulip: https://rust-lang.zulipchat.com/#narrow/channel/233931-t-compiler.2Fmajor-changes/topic/Proposal.20for.20Adapt.20Stack.20Protector.20for.20Ru.E2.80.A6.20compiler-team.23841 I have opened an issue to discuss the 'abi_mismatch' issue I encountered while writing tests for the coexistence of 'stack-protector' and 'safe-stack': #149340 r? `@wesleywiser` (feel free to reassign) cc `@nikic,` `@rcvalle,` `@davidtwco,` `@arielb1,` `@Darksonn,` `@Noratrieb,` `@SparrowLii`

rustbot assigned wesleywiser Sep 28, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 28, 2025