Skip to content

Commit 9890981

Browse files
Rollup merge of #148849 - saethlin:windows-stack-protectors, r=wesleywiser
Set -Cpanic=abort in windows-msvc stack protector tests I ran into a test failure with the 32-bit windows test on #117192, one of the tests has been incorrectly passing (until my change!) because it is picking up the stack protector from another function. I've tried to prevent that happening again by adding CHECK-DAGs for the start and end of each function. I've also done my best to correct the comments, some were based on the fact that we used to run these tests with unwinding panics, but LLVM doesn't add protectors to function with SEH funclets so it's must more straightforward for these tests to use `-Cpanic=abort`.
2 parents 663d843 + a5f677b commit 9890981

File tree

3 files changed

+79
-61
lines changed

3 files changed

+79
-61
lines changed

tests/assembly-llvm/stack-protector/stack-protector-heuristics-effect-windows-32bit.rs

Lines changed: 33 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
//@ [strong] compile-flags: -Z stack-protector=strong
88
//@ [basic] compile-flags: -Z stack-protector=basic
99
//@ [none] compile-flags: -Z stack-protector=none
10-
//@ compile-flags: -C opt-level=2 -Z merge-functions=disabled
10+
//@ compile-flags: -C opt-level=2 -Z merge-functions=disabled -Cpanic=abort -Cdebuginfo=1
1111

1212
#![crate_type = "lib"]
1313
#![allow(internal_features)]
@@ -39,6 +39,8 @@ pub fn array_char(f: fn(*const char)) {
3939
// basic: __security_check_cookie
4040
// none-NOT: __security_check_cookie
4141
// missing-NOT: __security_check_cookie
42+
43+
// CHECK-DAG: .cv_fpo_endproc
4244
}
4345

4446
// CHECK-LABEL: array_u8_1
@@ -55,6 +57,8 @@ pub fn array_u8_1(f: fn(*const u8)) {
5557
// basic-NOT: __security_check_cookie
5658
// none-NOT: __security_check_cookie
5759
// missing-NOT: __security_check_cookie
60+
61+
// CHECK-DAG: .cv_fpo_endproc
5862
}
5963

6064
// CHECK-LABEL: array_u8_small:
@@ -72,6 +76,8 @@ pub fn array_u8_small(f: fn(*const u8)) {
7276
// basic-NOT: __security_check_cookie
7377
// none-NOT: __security_check_cookie
7478
// missing-NOT: __security_check_cookie
79+
80+
// CHECK-DAG: .cv_fpo_endproc
7581
}
7682

7783
// CHECK-LABEL: array_u8_large:
@@ -88,6 +94,8 @@ pub fn array_u8_large(f: fn(*const u8)) {
8894
// basic: __security_check_cookie
8995
// none-NOT: __security_check_cookie
9096
// missing-NOT: __security_check_cookie
97+
98+
// CHECK-DAG: .cv_fpo_endproc
9199
}
92100

93101
#[derive(Copy, Clone)]
@@ -107,6 +115,8 @@ pub fn array_bytesizednewtype_9(f: fn(*const ByteSizedNewtype)) {
107115
// basic: __security_check_cookie
108116
// none-NOT: __security_check_cookie
109117
// missing-NOT: __security_check_cookie
118+
119+
// CHECK-DAG: .cv_fpo_endproc
110120
}
111121

112122
// CHECK-LABEL: local_var_addr_used_indirectly
@@ -134,6 +144,8 @@ pub fn local_var_addr_used_indirectly(f: fn(bool)) {
134144
// basic-NOT: __security_check_cookie
135145
// none-NOT: __security_check_cookie
136146
// missing-NOT: __security_check_cookie
147+
148+
// CHECK-DAG: .cv_fpo_endproc
137149
}
138150

139151
// CHECK-LABEL: local_string_addr_taken
@@ -143,28 +155,15 @@ pub fn local_string_addr_taken(f: fn(&String)) {
143155
f(&x);
144156

145157
// Taking the address of the local variable `x` leads to stack smash
146-
// protection with the `strong` heuristic, but not with the `basic`
147-
// heuristic. It does not matter that the reference is not mut.
148-
//
149-
// An interesting note is that a similar function in C++ *would* be
150-
// protected by the `basic` heuristic, because `std::string` has a char
151-
// array internally as a small object optimization:
152-
// ```
153-
// cat <<EOF | clang++ -O2 -fstack-protector -S -x c++ - -o - | grep stack_chk
154-
// #include <string>
155-
// void f(void (*g)(const std::string&)) {
156-
// std::string x;
157-
// g(x);
158-
// }
159-
// EOF
160-
// ```
161-
//
158+
// protection. It does not matter that the reference is not mut.
162159

163160
// all: __security_check_cookie
164-
// strong-NOT: __security_check_cookie
165-
// basic-NOT: __security_check_cookie
161+
// strong: __security_check_cookie
162+
// basic: __security_check_cookie
166163
// none-NOT: __security_check_cookie
167164
// missing-NOT: __security_check_cookie
165+
166+
// CHECK-DAG: .cv_fpo_endproc
168167
}
169168

170169
pub trait SelfByRef {
@@ -194,6 +193,8 @@ pub fn local_var_addr_taken_used_locally_only(factory: fn() -> i32, sink: fn(i32
194193
// basic-NOT: __security_check_cookie
195194
// none-NOT: __security_check_cookie
196195
// missing-NOT: __security_check_cookie
196+
197+
// CHECK-DAG: .cv_fpo_endproc
197198
}
198199

199200
pub struct Gigastruct {
@@ -231,6 +232,8 @@ pub fn local_large_var_moved(f: fn(Gigastruct)) {
231232
// basic: __security_check_cookie
232233
// none-NOT: __security_check_cookie
233234
// missing-NOT: __security_check_cookie
235+
236+
// CHECK-DAG: .cv_fpo_endproc
234237
}
235238

236239
// CHECK-LABEL: local_large_var_cloned
@@ -260,6 +263,8 @@ pub fn local_large_var_cloned(f: fn(Gigastruct)) {
260263
// basic: __security_check_cookie
261264
// none-NOT: __security_check_cookie
262265
// missing-NOT: __security_check_cookie
266+
267+
// CHECK-DAG: .cv_fpo_endproc
263268
}
264269

265270
extern "C" {
@@ -300,6 +305,8 @@ pub fn alloca_small_compile_time_constant_arg(f: fn(*mut ())) {
300305
// basic-NOT: __security_check_cookie
301306
// none-NOT: __security_check_cookie
302307
// missing-NOT: __security_check_cookie
308+
309+
// CHECK-DAG: .cv_fpo_endproc
303310
}
304311

305312
// CHECK-LABEL: alloca_large_compile_time_constant_arg
@@ -312,6 +319,8 @@ pub fn alloca_large_compile_time_constant_arg(f: fn(*mut ())) {
312319
// basic-NOT: __security_check_cookie
313320
// none-NOT: __security_check_cookie
314321
// missing-NOT: __security_check_cookie
322+
323+
// CHECK-DAG: .cv_fpo_endproc
315324
}
316325

317326
// CHECK-LABEL: alloca_dynamic_arg
@@ -324,14 +333,14 @@ pub fn alloca_dynamic_arg(f: fn(*mut ()), n: usize) {
324333
// basic-NOT: __security_check_cookie
325334
// none-NOT: __security_check_cookie
326335
// missing-NOT: __security_check_cookie
336+
337+
// CHECK-DAG: .cv_fpo_endproc
327338
}
328339

329340
// The question then is: in what ways can Rust code generate array-`alloca`
330341
// LLVM instructions? This appears to only be generated by
331342
// rustc_codegen_ssa::traits::Builder::array_alloca() through
332-
// rustc_codegen_ssa::mir::operand::OperandValue::store_unsized(). FWICT
333-
// this is support for the "unsized locals" unstable feature:
334-
// https://doc.rust-lang.org/unstable-book/language-features/unsized-locals.html.
343+
// rustc_codegen_ssa::mir::operand::OperandValue::store_unsized().
335344

336345
// CHECK-LABEL: unsized_fn_param
337346
#[no_mangle]
@@ -346,14 +355,11 @@ pub fn unsized_fn_param(s: [u8], l: bool, f: fn([u8])) {
346355
// alloca, and is therefore not protected by the `strong` or `basic`
347356
// heuristics.
348357

349-
// We should have a __security_check_cookie call in `all` and `strong` modes but
350-
// LLVM does not support generating stack protectors in functions with funclet
351-
// based EH personalities.
352-
// https://github.com/llvm/llvm-project/blob/37fd3c96b917096d8a550038f6e61cdf0fc4174f/llvm/lib/CodeGen/StackProtector.cpp#L103C1-L109C4
353358
// all-NOT: __security_check_cookie
354359
// strong-NOT: __security_check_cookie
355-
356360
// basic-NOT: __security_check_cookie
357361
// none-NOT: __security_check_cookie
358362
// missing-NOT: __security_check_cookie
363+
364+
// CHECK-DAG: .cv_fpo_endproc
359365
}

0 commit comments

Comments
 (0)