[PATCH] D55428: [Docs] Expand -fstack-protector and -fstack-protector-all info

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre added inline comments.


================
Comment at: include/clang/Driver/Options.td:1634-1636
+  HelpText<"Uses a stronger heuristic to apply stack protectors to functions "
+           "that include arrays of any size (and any type), "
+           "as well as any calls to alloca or the taking of an address from a local variable">;
----------------
I'm not sure what's the policy for related options but I feel the description should stand on its own. I'd therefor start by:

"Enable stack protectors for some functions potentially vulnerable to stack smashing. Compared to -fstack-protector, this uses a stronger heuristic (....)"

If the policy is to avoid such repeatition then please ignore this comment.


================
Comment at: include/clang/Driver/Options.td:1638
 def fstack_protector : Flag<["-"], "fstack-protector">, Group<f_Group>,
-  HelpText<"Enable stack protectors for functions potentially vulnerable to stack smashing">;
+  HelpText<"Enable stack protectors for some functions potentially vulnerable to stack smashing. "
+           "Namely those containing a char (or 8bit integer) array or constant sized calls to "
----------------
Not a native english speaker but I feel that "potentially" is redundant given you said it enables stack protector for *some* functions. Perhaps rewrite it along the lines of:

"Enable stack protectors for some of the functions vulnerable to stack smashing based on simple heuristic"

with a better word than "simple". This conveys both that not all functions are protected and suggests that a better heuristic is possible. You can then easily refer the reader to -fstack-protector-strong and -fstack-protector-all in a following sentence.


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

https://reviews.llvm.org/D55428