[PATCH] Documentation for sanitizer special case list

Alexey Samsonov samsonov at google.com
Mon Aug 5 07:48:18 PDT 2013



================
Comment at: docs/SanitizerSpecialCaseList.rst:20-21
@@ +19,4 @@
+
+To achieve this, user may provide a file listing the objects he wants to
+ignore, and provide it to clang at compile-time.
+
----------------
Dmitri Gribenko wrote:
> 'objects' might be ambiguous with 'object files'.  Could we use a different word?  'entities'?  'source-level entities'?
Done

================
Comment at: docs/SanitizerSpecialCaseList.rst:11-20
@@ +10,12 @@
+
+User of sanitizer tools, such as :doc:`AddressSanitizer`, :doc:`ThreadSanitizer`
+or :doc:`MemorySanitizer` may want to disable or alter some checks for
+certain objects to:
+
+* speedup hot function, which is known to be correct;
+* ignore a function that does some low-level magic (e.g. walks through the
+  thread stack, bypassing the frame boundaries);
+* ignore a known problem.
+
+To achieve this, user may provide a file listing the objects he wants to
+ignore, and provide it to clang at compile-time.
----------------
Sean Silva wrote:
> These two usages of the term "objects" here is confusing, as it makes it sound like this feature applies at the granularity of object files. Maybe "parts of the program" or "program entities" would be better?
Done

================
Comment at: docs/SanitizerSpecialCaseList.rst:8-9
@@ +7,4 @@
+
+Goal
+====
+
----------------
Sean Silva wrote:
> In this section, make sure to include a summary of what information the document provides (after reading just the introduction, the reader should be able to know if this document contains the information they need).
Done (in an introduction section)

================
Comment at: docs/SanitizerSpecialCaseList.rst:23-24
@@ +22,4 @@
+
+Generally, the use of this file is **discouraged**, in most cases it's better
+to fix the reported issue in the source code.
+
----------------
Sean Silva wrote:
> This doesn't add anything to the content (this isn't meant to be a persuasive document).
Removed

================
Comment at: docs/SanitizerSpecialCaseList.rst:21
@@ +20,3 @@
+To achieve this, user may provide a file listing the objects he wants to
+ignore, and provide it to clang at compile-time.
+
----------------
Sean Silva wrote:
> Please reiterate the compiler flags relevant for this feature. You can also probably move the sentence in the "Usage" section up to here for this purpose, and omit the "Usage" section since you are already pointing users to UsersManual.rst
Moved the sentence from Usage section up here.

================
Comment at: docs/SanitizerSpecialCaseList.rst:26-27
@@ +25,4 @@
+
+Format
+======
+
----------------
Sean Silva wrote:
> Please add a "How-to" or "Example" section before describing the format in detail, which gives a simple (but complete) example of how to ignore a function "foo" for a particular sanitizer (or whatever example seems most appropriate). I.e. show a trivial .c file and a corresponding blacklist file for ignoring a function in it, and a compiler command line for it, exhibiting the correct flags for enabling the feature.
Done

================
Comment at: docs/SanitizerSpecialCaseList.rst:35
@@ +34,3 @@
+    # Turn off checks for the source file:
+    src:/path/to/source/file.c
+    # Turn off checks for a particular functions (use mangled names):
----------------
Sean Silva wrote:
> Alexey Samsonov wrote:
> > Evgeniy Stepanov wrote:
> > > You forgot the most important part about search-vs-match behaviour of source file paths.
> > Expanded a comment a bit. I think it should be pretty clear now.
> Does it have to be an absolute path? If not, then what cwd is used when looking up relative paths? Is there any other file-matching logic besides that (e.g. if I say `src:foo.c`, will it match a file `libA/foo.c` and libB/foo.c`?)
Fixed in newer revision. The path is either absolute, or relevant to cwd (not sure how to clarify cwd, though).

================
Comment at: docs/SanitizerSpecialCaseList.rst:43
@@ +42,3 @@
+
+Some sanitizer tools may introduce custom entry types - see the tool-specific docs.
+
----------------
Sean Silva wrote:
> I was not able to find anything in the the linked tool-specific docs describing entries for this file. Please be more specific and provide links (or even better, add/improve the relevant documents here in-tree).
Updated in the newer revision (see "Blacklist" part of (Address|Memory|Thread)Sanitizer docs. Or you suggest to move tool-specific stuff here?

================
Comment at: docs/SanitizerSpecialCaseList.rst:29
@@ +28,3 @@
+
+Each special case list entry occupies a single line.
+
----------------
Sean Silva wrote:
> Please add a generic description of what each "entry" consists of. The description in SpecialCaseList.h seems good for this purpose:
> 
> ```
> // Each line contains a prefix, followed by a colon and a wild card expression,
> // followed optionally by an equals sign and an instrumentation-specific
> // category.  Empty lines and lines starting with "#" are ignored.
> ```
> 
> Also, SpecialCaseList.h seems to also discuss an `=<category>` at the end of the line. Why do you not discuss that?
Clarified this. I'd like to avoid describing categories here (at least for a first revision), as
1) they are not widely used yet (they were introduced for dfsan, which is not in trunk yet)
2) they are tool-specific.

================
Comment at: docs/SanitizerSpecialCaseList.rst:40-41
@@ +39,4 @@
+    # Wildcards are supported:
+    src:bad/sources/*
+    fun:*BadFunction*
+
----------------
Sean Silva wrote:
> Is `*` the only supported wildcard? (e.g., is `[a-zA-Z_]` or `?` supported?) If not, then please provide an example of each type of wildcard supported (maybe say "wildcarding as in the following examples is supported"). If it is the only supported wildcard, then please describe it as "shell-like usage of `*` is supported" to avoid being overly broad.
Clarified this a bit.


http://llvm-reviews.chandlerc.com/D1268



More information about the cfe-commits mailing list