From d46b6889451de69a9cf7808a042d8292fc2c341a Mon Sep 17 00:00:00 2001
From: Erik Desjardins <erikdesjardins@users.noreply.github.com>
Date: Tue, 5 Mar 2024 20:55:42 -0500
Subject: [PATCH] adjust stack-protector test (which inappropriately depends on
 IR types)

---
 .../stack-protector-heuristics-effect.rs      | 55 +++++--------------
 1 file changed, 15 insertions(+), 40 deletions(-)

diff --git a/tests/assembly/stack-protector/stack-protector-heuristics-effect.rs b/tests/assembly/stack-protector/stack-protector-heuristics-effect.rs
index e63adc88ff501..8e32d170244a7 100644
--- a/tests/assembly/stack-protector/stack-protector-heuristics-effect.rs
+++ b/tests/assembly/stack-protector/stack-protector-heuristics-effect.rs
@@ -11,6 +11,11 @@
 //@ compile-flags: -C opt-level=2 -Z merge-functions=disabled
 //@ min-llvm-version: 17.0.2
 
+// NOTE: the heuristics for stack smash protection inappropriately rely on types in LLVM IR,
+// despite those types having no semantic meaning. This means that the `basic` and `strong`
+// settings do not behave in a coherent way. This is a known issue in LLVM.
+// See comments on https://github.com/rust-lang/rust/issues/114903.
+
 #![crate_type = "lib"]
 
 #![allow(incomplete_features)]
@@ -39,23 +44,9 @@ pub fn array_char(f: fn(*const char)) {
     f(&b as *const _);
     f(&c as *const _);
 
-    // Any type of local array variable leads to stack protection with the
-    // "strong" heuristic. The 'basic' heuristic only adds stack protection to
-    // functions with local array variables of a byte-sized type, however. Since
-    // 'char' is 4 bytes in Rust, this function is not protected by the 'basic'
-    // heuristic
-    //
-    // (This test *also* takes the address of the local stack variables. We
-    // cannot know that this isn't what triggers the `strong` heuristic.
-    // However, the test strategy of passing the address of a stack array to an
-    // external function is sufficient to trigger the `basic` heuristic (see
-    // test `array_u8_large()`). Since the `basic` heuristic only checks for the
-    // presence of stack-local array variables, we can be confident that this
-    // test also captures this part of the `strong` heuristic specification.)
-
     // all: __stack_chk_fail
     // strong: __stack_chk_fail
-    // basic-NOT: __stack_chk_fail
+    // basic: __stack_chk_fail
     // none-NOT: __stack_chk_fail
     // missing-NOT: __stack_chk_fail
 }
@@ -163,26 +154,11 @@ pub fn local_string_addr_taken(f: fn(&String)) {
     f(&x);
 
     // Taking the address of the local variable `x` leads to stack smash
-    // protection with the `strong` heuristic, but not with the `basic`
-    // heuristic. It does not matter that the reference is not mut.
-    //
-    // An interesting note is that a similar function in C++ *would* be
-    // protected by the `basic` heuristic, because `std::string` has a char
-    // array internally as a small object optimization:
-    // ```
-    // cat <<EOF | clang++ -O2 -fstack-protector -S -x c++ - -o - | grep stack_chk
-    // #include <string>
-    // void f(void (*g)(const std::string&)) {
-    //     std::string x;
-    //     g(x);
-    // }
-    // EOF
-    // ```
-    //
+    // protection. It does not matter that the reference is not mut.
 
     // all: __stack_chk_fail
     // strong: __stack_chk_fail
-    // basic-NOT: __stack_chk_fail
+    // basic: __stack_chk_fail
     // none-NOT: __stack_chk_fail
     // missing-NOT: __stack_chk_fail
 }
@@ -233,8 +209,8 @@ pub fn local_large_var_moved(f: fn(Gigastruct)) {
     // Even though the local variable conceptually doesn't have its address
     // taken, it's so large that the "move" is implemented with a reference to a
     // stack-local variable in the ABI. Consequently, this function *is*
-    // protected by the `strong` heuristic. This is also the case for
-    // rvalue-references in C++, regardless of struct size:
+    // protected. This is also the case for rvalue-references in C++,
+    // regardless of struct size:
     // ```
     // cat <<EOF | clang++ -O2 -fstack-protector-strong -S -x c++ - -o - | grep stack_chk
     // #include <cstdint>
@@ -248,7 +224,7 @@ pub fn local_large_var_moved(f: fn(Gigastruct)) {
 
     // all: __stack_chk_fail
     // strong: __stack_chk_fail
-    // basic-NOT: __stack_chk_fail
+    // basic: __stack_chk_fail
     // none-NOT: __stack_chk_fail
     // missing-NOT: __stack_chk_fail
 }
@@ -261,9 +237,9 @@ pub fn local_large_var_cloned(f: fn(Gigastruct)) {
     // A new instance of `Gigastruct` is passed to `f()`, without any apparent
     // connection to this stack frame. Still, since instances of `Gigastruct`
     // are sufficiently large, it is allocated in the caller stack frame and
-    // passed as a pointer. As such, this function is *also* protected by the
-    // `strong` heuristic, just like `local_large_var_moved`. This is also the
-    // case for pass-by-value of sufficiently large structs in C++:
+    // passed as a pointer. As such, this function is *also* protected, just
+    // like `local_large_var_moved`. This is also the case for pass-by-value
+    // of sufficiently large structs in C++:
     // ```
     // cat <<EOF | clang++ -O2 -fstack-protector-strong -S -x c++ - -o - | grep stack_chk
     // #include <cstdint>
@@ -275,10 +251,9 @@ pub fn local_large_var_cloned(f: fn(Gigastruct)) {
     // EOF
     // ```
 
-
     // all: __stack_chk_fail
     // strong: __stack_chk_fail
-    // basic-NOT: __stack_chk_fail
+    // basic: __stack_chk_fail
     // none-NOT: __stack_chk_fail
     // missing-NOT: __stack_chk_fail
 }