[PATCH] D75695: [StackProtector] Catch direct out-of-bounds when checking address-takenness

[Paul Robinson via Phabricator via llvm-commits]

probinson added a comment.

I have to admit I didn't thoroughly look at the test, although I have to wonder if the loop cases in the test really provide any additional coverage?  They're using phis as input, but you already have phi cases.



================
Comment at: llvm/lib/CodeGen/StackProtector.cpp:194
+      // If the GEP offset is out-of-bounds, or is non-constant and so has to be
+      // assumed to be out-of-bounds, then any memory access that would use it
+      // would also have to be out-of-bounds meaning stack protection is
----------------
potentially out-of-bounds


================
Comment at: llvm/lib/CodeGen/StackProtector.cpp:195
+      // assumed to be out-of-bounds, then any memory access that would use it
+      // would also have to be out-of-bounds meaning stack protection is
+      // required.
----------------
s/would also have to be/could also be/


================
Comment at: llvm/test/CodeGen/Generic/stack-guard-oob.ll:6
+; RUN: llc -mtriple=x86_64 -O0 < %s | FileCheck %s
+
+; CHECK-LABEL: in_bounds
----------------
REQUIRES: aarch64-registered-target
REQUIRES: arm-registered-target
REQUIRES: x86-registered-target



================
Comment at: llvm/test/CodeGen/Generic/stack-guard-oob.ll:7
+
+; CHECK-LABEL: in_bounds
+; CHECK-NOT: __stack_chk_guard
----------------
When CHECK-LABEL specifies an actual label, it's best to include the appropriate punctuation; so, `CHECK-LABEL: in_bounds:` and so on throughout.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75695/new/

https://reviews.llvm.org/D75695