[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

The file is auto-generated by:
`path/to/clang-tblgen -gen-opt-docs -I clang/include/clang/Driver -I 
llvm/include/ clang/include/clang/Driver/ClangOptionDocs.td > 
clang/docs/ClangCommandLineReference.rst`

The description is retrieved from `DocBrief`. It `DocBrief` does not exist, 
`HelpText` provides a fallback. I think there needs to be a better way to 
organize detailed help messages. Inlined help is just too cumbersome. For 
attributes, there is a TableGen file `clang/include/clang/Basic/AttrDocs.td`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85239

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

[Peter Smith via Phabricator via cfe-commits]

psmith added a comment.

I agree it's not what I'd like. I'm not sure how to react to MaskRays comment. 
The top of the ClangCommandLineReference.rst says:

  ---
  NOTE: This file is automatically generated by running clang-tblgen
  -gen-opt-docs. Do not edit this file by hand!!
  ---

This uses Options.td to generate the file. I don't know how true this is 
anymore given that both files are checked in. They may have been separated. 
I've seen at least one instance in the log where the whole file has been 
regenerated.

I'm happy to revert the Options.td change if there is another way?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85239

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Whoa--that's the *help* text?  Well, if that's the only documentation for 
options that there is, I guess it has to go there; but it seems pretty 
excessive for the (ideally concise) output of `clang --help`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85239

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

[Peter Smith via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
psmith marked an inline comment as done.
Closed by commit rG839d974ee0e4: [DOCS] Add more detail to stack protector 
documentation (authored by psmith).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85239

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1801,10 +1801,15 @@
"as well as any calls to alloca or the taking of an address from a 
local variable">;
 def fstack_protector : Flag<["-"], "fstack-protector">, Group,
   HelpText<"Enable stack protectors for some functions vulnerable to stack 
smashing. "
-   "This uses a loose heuristic which considers functions vulnerable "
-   "if they contain a char (or 8bit integer) array or constant sized 
calls to "
-   "alloca, which are of greater size than ssp-buffer-size (default: 8 
bytes). "
-   "All variable sized calls to alloca are considered vulnerable">;
+   "This uses a loose heuristic which considers functions vulnerable 
if they "
+   "contain a char (or 8bit integer) array or constant sized calls to 
alloca "
+   ", which are of greater size than ssp-buffer-size (default: 8 
bytes). All "
+   "variable sized calls to alloca are considered vulnerable. A 
function with"
+   "a stack protector has a guard value added to the stack frame that 
is "
+   "checked on function exit. The guard value must be positioned in 
the "
+   "stack frame such that a buffer overflow from a vulnerable variable 
will "
+   "overwrite the guard value before overwriting the function's return 
"
+   "address. The reference stack guard value is stored in a global 
variable.">;
 def ftrivial_auto_var_init : Joined<["-"], "ftrivial-auto-var-init=">, 
Group,
   Flags<[CC1Option, CoreOption]>, HelpText<"Initialize trivial automatic stack 
variables: uninitialized (default)"
   " | pattern">, Values<"uninitialized,pattern">;
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -2136,7 +2136,7 @@
 
 .. option:: -fstack-protector, -fno-stack-protector
 
-Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca, which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable
+Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca , which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable. A function witha stack protector has a 
guard value added to the stack frame that is checked on function exit. The 
guard value must be positioned in the stack frame such that a buffer overflow 
from a vulnerable variable will overwrite the guard value before overwriting 
the function's return address. The reference stack guard value is stored in a 
global variable.
 
 .. option:: -fstack-protector-all
 


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1801,10 +1801,15 @@
"as well as any calls to alloca or the taking of an address from a local variable">;
 def fstack_protector : Flag<["-"], "fstack-protector">, Group,
   HelpText<"Enable stack protectors for some functions vulnerable to stack smashing. "
-   "This uses a loose heuristic which considers functions vulnerable "
-   "if they contain a char (or 8bit integer) array or constant sized calls to "
-   "alloca, which are of greater size than ssp-buffer-size (default: 8 bytes). "
-   "All variable sized calls to alloca are considered vulnerable">;
+   "This uses a loose heuristic which considers functions vulnerable if they "
+   "contain a char (or 8bit integer) array or constant sized calls to alloca "
+   ", which are of greater size than ssp-buffer-size (default: 8 bytes). All "
+   "variable sized calls to alloca are considered vulnerable. A function with"
+   "a stack protector has a guard value added to the stack frame that is "
+   "checked 

[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

[Peter Smith via Phabricator via cfe-commits]

psmith marked an inline comment as done.
psmith added inline comments.



Comment at: clang/docs/ClangCommandLineReference.rst:2139
 
-Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca, which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable
+Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca, which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable. A function with a stack protector has a 
guard value added to the stack frame that is checked on function exit. The 
guard value must be positioned in the stack frame such that a buffer overflow 
from a vulnerable variable will overwrite to the guard value before overwriting 
the function's return address. The reference stack guard value is stored in a 
global variable.
 

probinson wrote:
> "overwrite to the guard variable" -> "overwrite the guard variable"
Thanks, now updated


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

https://reviews.llvm.org/D85239

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

[Peter Smith via Phabricator via cfe-commits]

psmith updated this revision to Diff 283201.
psmith added a comment.
Herald added a subscriber: dang.

uploaded diff with both Options.td and ClangCommandLineReference.rst


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

https://reviews.llvm.org/D85239

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1801,10 +1801,15 @@
"as well as any calls to alloca or the taking of an address from a 
local variable">;
 def fstack_protector : Flag<["-"], "fstack-protector">, Group,
   HelpText<"Enable stack protectors for some functions vulnerable to stack 
smashing. "
-   "This uses a loose heuristic which considers functions vulnerable "
-   "if they contain a char (or 8bit integer) array or constant sized 
calls to "
-   "alloca, which are of greater size than ssp-buffer-size (default: 8 
bytes). "
-   "All variable sized calls to alloca are considered vulnerable">;
+   "This uses a loose heuristic which considers functions vulnerable 
if they "
+   "contain a char (or 8bit integer) array or constant sized calls to 
alloca "
+   ", which are of greater size than ssp-buffer-size (default: 8 
bytes). All "
+   "variable sized calls to alloca are considered vulnerable. A 
function with"
+   "a stack protector has a guard value added to the stack frame that 
is "
+   "checked on function exit. The guard value must be positioned in 
the "
+   "stack frame such that a buffer overflow from a vulnerable variable 
will "
+   "overwrite the guard value before overwriting the function's return 
"
+   "address. The reference stack guard value is stored in a global 
variable.">;
 def ftrivial_auto_var_init : Joined<["-"], "ftrivial-auto-var-init=">, 
Group,
   Flags<[CC1Option, CoreOption]>, HelpText<"Initialize trivial automatic stack 
variables: uninitialized (default)"
   " | pattern">, Values<"uninitialized,pattern">;
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -2136,7 +2136,7 @@
 
 .. option:: -fstack-protector, -fno-stack-protector
 
-Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca, which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable
+Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca , which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable. A function witha stack protector has a 
guard value added to the stack frame that is checked on function exit. The 
guard value must be positioned in the stack frame such that a buffer overflow 
from a vulnerable variable will overwrite the guard value before overwriting 
the function's return address. The reference stack guard value is stored in a 
global variable.
 
 .. option:: -fstack-protector-all
 


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1801,10 +1801,15 @@
"as well as any calls to alloca or the taking of an address from a local variable">;
 def fstack_protector : Flag<["-"], "fstack-protector">, Group,
   HelpText<"Enable stack protectors for some functions vulnerable to stack smashing. "
-   "This uses a loose heuristic which considers functions vulnerable "
-   "if they contain a char (or 8bit integer) array or constant sized calls to "
-   "alloca, which are of greater size than ssp-buffer-size (default: 8 bytes). "
-   "All variable sized calls to alloca are considered vulnerable">;
+   "This uses a loose heuristic which considers functions vulnerable if they "
+   "contain a char (or 8bit integer) array or constant sized calls to alloca "
+   ", which are of greater size than ssp-buffer-size (default: 8 bytes). All "
+   "variable sized calls to alloca are considered vulnerable. A function with"
+   "a stack protector has a guard value added to the stack frame that is "
+   "checked on function exit. The guard value must be positioned in the "
+   "stack frame such that a buffer overflow from a vu

[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

[Peter Smith via Phabricator via cfe-commits]

psmith added a comment.

In D85239#2194604 , @MaskRay wrote:

> This file is auto generated. The real documentation should go to 
> `clang/include/clang/Driver/Options.td`

Thanks for pointing that out! I regenerated the ClangCommandLineReference.rst 
from Options.td but it looks like I'm not the only one that missed the comment 
on the top line. There are 27 differences in the file including some that are 
not in Options.td. I'm loathe to lose work by regenerating the whole file so 
I've included just the diff created by the change to Options.td. At least if 
the file is synched up again then we'll keep the edits.

Assuming this is still OK I'll commit tomorrow.


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

https://reviews.llvm.org/D85239

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

This file is auto generated. The real documentation should go to 
`clang/include/clang/Driver/Options.td`


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

https://reviews.llvm.org/D85239

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Looks okay (one grammar nit), probably worth waiting for someone else to chime 
in.




Comment at: clang/docs/ClangCommandLineReference.rst:2139
 
-Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca, which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable
+Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca, which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable. A function with a stack protector has a 
guard value added to the stack frame that is checked on function exit. The 
guard value must be positioned in the stack frame such that a buffer overflow 
from a vulnerable variable will overwrite to the guard value before overwriting 
the function's return address. The reference stack guard value is stored in a 
global variable.
 

"overwrite to the guard variable" -> "overwrite the guard variable"


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

https://reviews.llvm.org/D85239

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

[Peter Smith via Phabricator via cfe-commits]

psmith created this revision.
psmith added reviewers: jfb, Shayne, emaste, kristof.beyls, mattdr, ojhunt, 
probinson, reames, serge-sans-paille, dim.
Herald added a subscriber: dexonsmith.
psmith requested review of this revision.

The Clang -fstack-protector documentation mentions what functions are 
considered vulnerable but does not mention the details of the
implementation such as the use of a global variable for the guard value. This 
brings the documentation more in line with the GCC
documentation at: 
https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html and gives 
someone using the option more idea about what is protected.

This partly addresses https://bugs.llvm.org/show_bug.cgi?id=42764


https://reviews.llvm.org/D85239

Files:
  clang/docs/ClangCommandLineReference.rst


Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -2136,7 +2136,7 @@
 
 .. option:: -fstack-protector, -fno-stack-protector
 
-Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca, which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable
+Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca, which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable. A function with a stack protector has a 
guard value added to the stack frame that is checked on function exit. The 
guard value must be positioned in the stack frame such that a buffer overflow 
from a vulnerable variable will overwrite to the guard value before overwriting 
the function's return address. The reference stack guard value is stored in a 
global variable.
 
 .. option:: -fstack-protector-all
 


Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -2136,7 +2136,7 @@
 
 .. option:: -fstack-protector, -fno-stack-protector
 
-Enable stack protectors for some functions vulnerable to stack smashing. This uses a loose heuristic which considers functions vulnerable if they contain a char (or 8bit integer) array or constant sized calls to alloca, which are of greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls to alloca are considered vulnerable
+Enable stack protectors for some functions vulnerable to stack smashing. This uses a loose heuristic which considers functions vulnerable if they contain a char (or 8bit integer) array or constant sized calls to alloca, which are of greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls to alloca are considered vulnerable. A function with a stack protector has a guard value added to the stack frame that is checked on function exit. The guard value must be positioned in the stack frame such that a buffer overflow from a vulnerable variable will overwrite to the guard value before overwriting the function's return address. The reference stack guard value is stored in a global variable.
 
 .. option:: -fstack-protector-all
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits