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

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.

rustbot · 2025-10-10T12:14:27Z

⚠️ 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-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

cezarbbb · 2025-11-05T06:21:10Z

Edited.

SparrowLii · 2025-11-05T11:56:08Z

@bors r+

bors · 2025-11-05T11:56:12Z

📌 Commit c49f4e1 has been approved by SparrowLii

It is now in the queue for this repository.

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 r? `@wesleywiser` (feel free to reassign) cc `@nikic,` `@rcvalle,` `@davidtwco,` `@arielb1,` `@Darksonn,` `@Noratrieb,` `@SparrowLii`

bors · 2025-11-05T17:21:40Z

⌛ Testing commit c49f4e1 with merge 41d1d86...

bors · 2025-11-05T17:55:07Z

💔 Test failed - checks-actions

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

This comment has been minimized.

Sign in to view

rustbot added has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 10, 2025

cezarbbb force-pushed the stable_ssp branch from 5fabb95 to 6acf262 Compare October 10, 2025 12:01

This comment has been minimized.

Sign in to view

rustbot removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. has-merge-commits PR has merge commits, merge with caution. labels Oct 10, 2025

This comment has been minimized.

Sign in to view

rustbot added has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 10, 2025

cezarbbb force-pushed the stable_ssp branch from d1dadab to 6acf262 Compare October 10, 2025 12:14

rustbot removed has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 10, 2025

rcvalle added the PG-exploit-mitigations Project group: Exploit mitigations label Oct 13, 2025

SparrowLii self-assigned this Oct 25, 2025

SparrowLii reviewed Oct 31, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

cezarbbb added 3 commits November 4, 2025 10:33

Check the coexistence of stack-protector and safe-stack.

8d0b5a9

tests for linux.

3086cb2

tests for windows 32bit.

8e59eb3

tests for windows 64bit.

670871a

cezarbbb force-pushed the stable_ssp branch from f53752a to 670871a Compare November 4, 2025 03:19

SparrowLii reviewed Nov 5, 2025

View reviewed changes

SparrowLii approved these changes Nov 5, 2025

View reviewed changes

test function name order correction

c49f4e1

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 Nov 5, 2025

This comment has been minimized.

Sign in to view

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 5, 2025

This comment has been minimized.

Sign in to view

cezarbbb force-pushed the stable_ssp branch from b155fb3 to a6f5cca Compare November 6, 2025 02:40

This comment has been minimized.

Sign in to view

add -Cunsafe-allow-abi-mismatch=sanitizer to silence ABI mismatch error

c9d0ee1

cezarbbb force-pushed the stable_ssp branch from a6f5cca to c9d0ee1 Compare November 6, 2025 04:14

More robust stack protector testing #147115

Are you sure you want to change the base?

More robust stack protector testing #147115

Conversation

cezarbbb commented Sep 28, 2025 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Sep 28, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rustbot commented Oct 10, 2025

Uh oh!

SparrowLii commented Oct 25, 2025

Uh oh!

SparrowLii left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cezarbbb Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

SparrowLii left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cezarbbb commented Nov 5, 2025

Uh oh!

SparrowLii commented Nov 5, 2025

Uh oh!

bors commented Nov 5, 2025

Uh oh!

bors commented Nov 5, 2025

Uh oh!

This comment has been minimized.

bors commented Nov 5, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

cezarbbb commented Sep 28, 2025 •

edited by rustbot

Loading

SparrowLii left a comment •

edited by rustbot

Loading

cezarbbb Nov 3, 2025 •

edited

Loading

SparrowLii left a comment •

edited by rustbot

Loading