[rtsan][NFC] Documentation of suppression flag

[cjappl]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Chris Apple (cjappl)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/112727.diff

1 Files Affected:

• (modified) clang/docs/RealtimeSanitizer.rst (+62-4)

diff --git a/clang/docs/RealtimeSanitizer.rst b/clang/docs/RealtimeSanitizer.rst
index 103842e055db70..bec15fd6cad2ac 100644
--- a/clang/docs/RealtimeSanitizer.rst
+++ b/clang/docs/RealtimeSanitizer.rst
@@ -183,6 +183,10 @@ A **partial** list of flags RealtimeSanitizer respects:
      - ``true``
      - boolean
      - If set, use the symbolizer to turn virtual addresses to file/line locations. If false, can greatly speed up the error reporting.
+   * - ``suppressions``
+     - ""
+     - path
+     - If set to a valid suppressions file, will suppress issue reporting. See details in "Disabling", below.
 
 
 Some issues with flags can be debugged using the ``verbosity=$NUM`` flag:
@@ -194,12 +198,43 @@ Some issues with flags can be debugged using the ``verbosity=$NUM`` flag:
    misspelled_flag
    ...
 
-Disabling
----------
+Disabling and suppressing
+-------------------------
 
-In some circumstances, you may want to suppress error reporting in a specific scope.
+There are multiple ways to suppress error reporting when using RealtimeSanitizer.
 
-In C++, this is achieved via  ``__rtsan::ScopedDisabler``. Within the scope where the ``ScopedDisabler`` object is instantiated, all sanitizer error reports are suppressed. This suppression applies to the current scope as well as all invoked functions, including any functions called transitively.
+In general, ``ScopedDisabler`` should be preferred, as it is the most performant.
+
+.. list-table:: Suppression methods
+   :widths: 30 15 15 10 70
+   :header-rows: 1
+
+   * - Suppression method
+     - Specified at?
+     - Scope
+     - Run-time cost
+     - Description
+   * - ``ScopedDisabler``
+     - Compile-time
+     - Stack
+     - Very low
+     - Suppresses all sanitizer error reports in the current scope and all invoked functions.
+   * - ``function-name-matches`` suppression
+     - Run-time
+     - Single function
+     - Medium
+     - Suppresses intercepted and ``[[clang::blocking]]`` function calls by name.
+   * - ``call-stack-contains`` suppression
+     - Run-time
+     - Stack
+     - High
+     - Suppresses any stack trace contaning the specified pattern.
+    
+
+``ScopedDisabler``
+##################
+
+At compile time, RealtimeSanitizer may be disabled for a scope using ``__rtsan::ScopedDisabler``. Within the scope where the ``ScopedDisabler`` object is instantiated, all sanitizer error reports are suppressed. This suppression applies to the current scope as well as all invoked functions, including any functions called transitively.
 
 .. code-block:: c++
 
@@ -233,6 +268,29 @@ In C, you can use the ``__rtsan_disable()`` and ``rtsan_enable()`` functions to
 
 Each call to ``__rtsan_disable()`` must be paired with a subsequent call to ``__rtsan_enable()`` to restore normal sanitizer functionality. If a corresponding ``rtsan_enable()`` call is not made, the behavior is undefined.
 
+Suppression file
+################
+
+At run-time, suppressions may be specified using a suppressions file passed in ``RTSAN_OPTIONS``. Run-time suppression may be useful if the source cannot be changed.
+
+.. code-block:: console
+
+   > cat suppressions.supp
+   call-stack-contains:MallocViolation
+   call-stack-contains:std::*vector
+   function-name-matches:free
+   function-name-matches:CustomMarkedBlocking*
+   > RTSAN_OPTIONS="suppressions=suppressions.supp" ./a.out
+   ...
+
+Suppressions specified in this file are one of two flavors.
+
+``function-name-matches`` suppresses reporting of any intercepted library call, or function marked ``[[clang::blocking]]`` by name. If, for instance, you know that ``malloc`` is real-time safe on your system, you can disable the check for it via ``function-name-matches:malloc``.
+
+``call-stack-contains`` suppresses reporting of errors in any stack that contains a string matching the pattern specified. For example, suppressing error reporting of any non-real-time-safe behavior in ``std::vector`` may be specified ``call-stack-contains:std::*vector``. You must include symbols in your build for this method to be effective, unsymbolicated stack traces cannot be matched. ``call-stack-contains`` has the highest run-time cost of any method of suppression.
+
+Patterns may be exact matches or are "regex-light" patterns, containing special characters such as ``^$*``.
+
 Compile-time sanitizer detection
 --------------------------------
 

[davidtrevelyan]

I wonder whether the word "Suppresses" is a bit loaded in this context and could lead the reader down the wrong path a little. I worry the user might put this in the same mental box as the suppressions file. Perhaps that's not a problem.

How about something more like "- All violations are ignored for the lifetime of the disabler"?

[cjappl]

Great thought. I'm not sure where I come down on it, but I'm also amenable to this change.

I guess the question is (similar to the above) "Is there a difference between suppressing and ignoring an issue? As a user do I care?"

I could see one case for calling everything suppressing (which is what I inadvertently did in this PR), or make the differentiation between suppressing (what a suppression file does) and ignoring (what the Disabler does).

To the end user, they do the same thing, to us, we behave differently and go through different mechanisms to achieve that same result.

[cjappl]

How are you feeling about this a week on?

I'm fine with changing it, but wanted to check to see if your opinion had shifted. I think one thing that puts me in camp "ignored" is that anything ignored by the ScopedDisabler will not show up in our exit statistics as a "suppressed error count".

Therefore I think it's OK to split the terminology. If you still feel like this would be a good idea I would change it.

[davidtrevelyan]

Nice thought. I was marginally in favour of "ignored", but your point here makes me lean further towards "ignored". I feel like the user's use cases mean:

• using ScopedDisabler is the user saying "turn off realtimesanitizer"
• using suppressions means "keep using rtsan - hide these errors, but keep note of them"

Definitely support splitting the terminology here, but not ultra-strongly if anyone feels otherwise

[cjappl]

Switched it. Please have a read through of the latest commit and make sure everything is good - switched some wording around minorly to make the grammar work.

[cjappl]

Did one last revision on this like we discussed on our call. I'll submit this later if tests pass

[davidtrevelyan]

"all sanitizer error reports are suppressed ignored", maybe?

[cjappl]

(test failure seems unrelated, but I will ensure it's green before I submit after I get approval, going to rebase now)

After bisecting it seems related to #113255

[davidtrevelyan]

Minor nits: not sure the word "potential" adds value here (it raises questions), and I think "for that thread" and "invoked functions" are a given as we're already talking about a variable scope. I'd propose a simplification as follows:

All RTSan errors originating within the ScopedDisabler instance variable scope are ignored.

I don't feel strongly though, if you feel this is less clear - no worries.

[cjappl]

Agreed this sentence was getting a bit overwrought. Check out the latest, reworded yours a bit.