This is an archive of the discontinued LLVM Phabricator instance.

[DOCS] Add more detail to stack protector documentation
ClosedPublic

Authored by psmith on Aug 4 2020, 12:34 PM.

Details

Summary

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

Diff Detail

Event Timeline

psmith created this revision.Aug 4 2020, 12:34 PM
psmith requested review of this revision.Aug 4 2020, 12:34 PM

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

clang/docs/ClangCommandLineReference.rst
2139

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

jfb accepted this revision.Aug 4 2020, 1:33 PM
This revision is now accepted and ready to land.Aug 4 2020, 1:33 PM
MaskRay added a subscriber: MaskRay.EditedAug 4 2020, 2:19 PM

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

psmith added a comment.Aug 5 2020, 4:56 AM

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.

psmith updated this revision to Diff 283201.Aug 5 2020, 4:57 AM

uploaded diff with both Options.td and ClangCommandLineReference.rst

psmith marked an inline comment as done.Aug 5 2020, 4:57 AM
psmith added inline comments.
clang/docs/ClangCommandLineReference.rst
2139

Thanks, now updated

This revision was automatically updated to reflect the committed changes.
psmith marked an inline comment as done.
Herald added a project: Restricted Project. · View Herald TranscriptAug 6 2020, 5:59 AM

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.

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?

MaskRay added a comment.EditedAug 6 2020, 9:53 PM

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. If 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

@rsmith

MaskRay added a subscriber: rsmith.Aug 6 2020, 9:54 PM