[llvm] [LLVM][rtsan] Add nosanitize_realtime instrumentation (PR #106125)
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 29 15:17:54 PDT 2024
================
@@ -51,6 +53,7 @@ RealtimeSanitizerPass::RealtimeSanitizerPass(
PreservedAnalyses RealtimeSanitizerPass::run(Function &F,
AnalysisManager<Function> &AM) {
if (F.hasFnAttribute(Attribute::SanitizeRealtime)) {
+ assert(!F.hasFnAttribute(Attribute::NoSanitizeRealtime));
----------------
vitalybuka wrote:
> Thanks for pointing that out, I wasn't aware it existed!
>
> **Pros of ScopedDisabler:**
>
> * Enables finer-grained disabling within a function.
> * Avoids modifying LLVM attributes or Clang, simplifying the implementation.
> * Precedent exists with lsan.
>
> **Cons of ScopedDisabler:**
>
> * Doesn't follow existing patterns for disabling interceptors, unlike asan and others.
> * Users need to recreate build system infrastructure for conditional compilation with `-fsanitize=realtime`.
>
> The biggest concern is that second "con" -- users would need to manually include out-of-project headers and manage build conditions, leading to some potential issues:
conditions should not be a problem if you check __feature in rtsan head.
So it could be included unconditionally, and just compile into no-op without RTSan
we should do the same for all sanitizers, we have some inconsistency there.
>
> 1. Duplication of effort across users.
Code modification is needed. It should be either attribute or C++ object either way, so I don't see much difference in duplication.
> 2. Difficulty in locating the required header files. (and conditionally including them if the sanitizer is enabled, leading to more complex build systems)
Should be unconditional. Header installed with clang.
However with g++ it may need a condition.
But I don't think it's a big issue. We used to preprocessor in C++.
> 3. Risk of users re-implementing ScopedDisabler in their codebases.
Why it's a problem? They can do so even you go with existing attribute. I am 100% sure that ScopedDisabler like will be needed, as function scope may affect to much.
Also inliner and other passes can be affected by attribute. Regular C++ code will be more predictable for end user.
>
> The `nosanitize` attribute is comparatively simpler, which "just works" without extra steps. I would prefer whatever solution we go forward with, it uses a construct that would be as non-invasive as the nosanitize approach to avoid burdening users with additional setup. Are there other solutions that provide this level of ease to the end user?
lsan has larger user base then rtsan, at least for now, and it was never an issue. Attribute approach can work for lsan as well.
>
> For finer-grained interception, we've advised extracting code into new functions and applying the nosanitize attribute. This approach is a little more invasive but clearly indicates that the code is treated differently.
I am not aware of context for this, can you point me there? How attribute is better here then ScopedDisabler?
So unless we have strong reason, I would prefer we solve the same problem in all component consistently.
https://github.com/llvm/llvm-project/pull/106125
More information about the llvm-commits
mailing list