Page MenuHomePhabricator

Add documentation for -fsanitize-address-use-after-return.
ClosedPublic

Authored by kda on Jun 11 2021, 2:51 PM.

Diff Detail

Event Timeline

kda requested review of this revision.Jun 11 2021, 2:51 PM
kda created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJun 11 2021, 2:51 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
vitalybuka added inline comments.Jun 11 2021, 3:09 PM
clang/docs/AddressSanitizer.rst
17

Please check that this is formatted reasonable in preview.

clang/docs/ClangCommandLineReference.rst
889 ↗(On Diff #351566)

"Code generated and always enabled." is implementation details.
Something like this?

Valid options are:
* ``always`` - Detect use-after-return.
* ``runtime`` - Detect use-after-return with runtime ON/OFF switch (environment variable `ASAN_OPTIONS=detect_stack_use_after_return=1`, default: 0)
* ``never`` - Do not detect use-after-return.
kda updated this revision to Diff 351571.Jun 11 2021, 3:24 PM
kda marked an inline comment as done.
  • Revised according to feedback.
MaskRay added inline comments.
clang/docs/AddressSanitizer.rst
16–17

rst uses two backsticks instead of one.

MaskRay added inline comments.Jun 13 2021, 1:08 PM
clang/docs/ClangCommandLineReference.rst
3 ↗(On Diff #351571)

This file is generated by clang-tblgen -gen-opt-docs.

You can edit clang/docs/UsersManual.rst and include the information that =never can reduce the object file size.

kda updated this revision to Diff 352153.Jun 15 2021, 8:23 AM
  • attempting to make documentation pretty.
vitalybuka added inline comments.
clang/docs/ClangCommandLineReference.rst
3 ↗(On Diff #351571)

to clarify you need
configure -DLLVM_ENABLE_SPHINX=ON -DSPHINX_OUTPUT_HTML=ON -DSPHINX_OUTPUT_MAN=ON

and then: ninja docs-clang-html

AddressSanitizer.html will be somewhere in output, so you can check results

@MaskRay @rsmith However I am not sure what to do with ClangCommandLineReference.rst.
The one generated in build dir contains tens of missing flags. Still looks like the public doc https://clang.llvm.org/docs/ClangCommandLineReference.html is generated from the one committed here.
Should we just pick lines related to the feature and ignore the rest or don't touch at all and let?

kda updated this revision to Diff 352287.Jun 15 2021, 4:21 PM
kda marked 2 inline comments as done.
  • more beauty, added UsersManual changes.
kda updated this revision to Diff 352289.Jun 15 2021, 4:24 PM
kda marked 2 inline comments as done.
  • more consistency.
kda added a comment.Jun 15 2021, 4:25 PM

clang/docs/AddressSanitizer.rst
17

See attached screenshot.

MaskRay added inline comments.Jun 15 2021, 5:32 PM
clang/docs/ClangCommandLineReference.rst
3 ↗(On Diff #351571)

The current state isn't ideal but that is what I have now (https://lists.llvm.org/pipermail/cfe-dev/2020-September/066900.html)

vitalybuka added inline comments.Jun 15 2021, 6:23 PM
clang/docs/AddressSanitizer.rst
19

seems overloaded for the list of bug types:

Please create section, similar to

Stack Use After Return
---------------------
...
...
...
Memory leak detection
---------------------

and include additional information there

clang/docs/ClangCommandLineReference.rst
894 ↗(On Diff #352289)

I believe the best solution for now is just revert changes in this file and replace it with auto-generated version in a separate patch.

clang/docs/UsersManual.rst
3747

How about we update Options.td (in this patch) with:
HelpText<"Select the mode of detecting stack use-after-return in AddressSanitizer: never | runtime (default) | always">,

then we put corresponding text here and move block from UsersManual.rst:1851 into AddressSanitizer.rst (separate section)

kda updated this revision to Diff 352849.Jun 17 2021, 2:33 PM
kda marked an inline comment as done.
  • Responding to comments.
kda marked 2 inline comments as done.Jun 17 2021, 2:33 PM
vitalybuka accepted this revision.Jun 18 2021, 12:30 PM
vitalybuka added inline comments.
clang/docs/AddressSanitizer.rst
17–20

I guess just flag name is enough here, the rest in the section below.

This revision is now accepted and ready to land.Jun 18 2021, 12:30 PM
vitalybuka added inline comments.Jun 18 2021, 12:32 PM
clang/docs/AddressSanitizer.rst
143

Maybe -fsanitize=address is reduntant here, we are already in the -fsanitize=address document. Mentioning the flag in the line 11 seems more appropriate.

kda updated this revision to Diff 357385.Thu, Jul 8, 4:42 PM
kda marked an inline comment as done.
  • removed redundant mention of the flag.
This revision was landed with ongoing or failed builds.Thu, Jul 8, 4:44 PM
This revision was automatically updated to reflect the committed changes.