[PATCH] D29027: [Stack Protection] Add remark for reasons why Stack Protection has been applied

[Richard Smith via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: include/clang/Basic/DiagnosticFrontendKinds.td:229
+def remark_ssp_applied_reason
+    : Remark<"SSP applied to function due to %select{an unknown reason|a "
+             "call to alloca|a stack allocated buffer or struct containing a "
----------------
Can this "unknown reason" case actually happen? It seems like a bug that SSP insertion would not know why it's doing what it's doing.


================
Comment at: include/clang/Basic/DiagnosticFrontendKinds.td:229
+def remark_ssp_applied_reason
+    : Remark<"SSP applied to function due to %select{an unknown reason|a "
+             "call to alloca|a stack allocated buffer or struct containing a "
----------------
rsmith wrote:
> Can this "unknown reason" case actually happen? It seems like a bug that SSP insertion would not know why it's doing what it's doing.
Rather than the potentially-opaque initialism SSP, could you say "stack protector" here? That would match the flag name better.


================
Comment at: include/clang/Basic/DiagnosticFrontendKinds.td:230
+    : Remark<"SSP applied to function due to %select{an unknown reason|a "
+             "call to alloca|a stack allocated buffer or struct containing a "
+             "buffer|the address of a local variable being taken|a function "
----------------
Do we actually know that the cause is a call to `alloca` rather than a VLA?


================
Comment at: include/clang/Basic/DiagnosticFrontendKinds.td:232
+             "buffer|the address of a local variable being taken|a function "
+             "attribute or use of -fstack-protector-all}0">,
+      InGroup<SSPReason>;
----------------
These two cases seem very easy to tell apart: if `-fstack-protector-all` is specified, use that diagnostic, otherwise the LLVM attribute must have been from a source-level attribute.


================
Comment at: include/clang/Basic/DiagnosticGroups.td:911
+// function.
+def SSPReason : DiagGroup<"ssp-reason">;
----------------
The flags to control this are `-fstack-protector*`, so `-Rstack-protector` (or something starting `-Rstack-protector`) should be used here.


https://reviews.llvm.org/D29027