[PATCH] D89344: Introduce convenience macro ASAN_NO_INSTR_PTR.

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added a comment.

In D89344#2330706 <https://reviews.llvm.org/D89344#2330706>, @delcypher wrote:

> In D89344#2329258 <https://reviews.llvm.org/D89344#2329258>, @vitalybuka wrote:
>
>> Ubsan ignores volatile vars https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html
>> maybe better to make sure that asan does the same here as well?
>
> Why would we want that behaviour for ASan? I checked and it looks like `volatile` pointers do get instrumented by ASan and I see no reason to change that.

To have consistent way to disable instrumentation on variable.



================
Comment at: compiler-rt/include/sanitizer/asan_interface.h:328
+// by ASan. Loads and stores on pointers of this type will not be
+// instrumented.
+//
----------------
delcypher wrote:
> vitalybuka wrote:
> > I don't think it should be here
> > we don't put here defines for __attribute__((no_sanitize("address")))
> > 
> > Also this header is interface for runtime, and define controls compiler side.
> > Also this header is interface for runtime, and define controls compiler side.
> 
> This header **already provides defines** for use at compile time so I don't agree with this objection.
> 
> Alternatively we could introduce a new header file (e.g. `santizer/asan_util.h`) for convenience macros that are used at compile time.
> 
Either this or asan_util.h are going to be used by external code which we can't see. In future it's going to be hard to remove or change anything that here so we like to keep as simple as possible and avoid adding unnecessary convenience stuff here.

Defines you mentioned exist from 2011 and probably we didn't remove them for the reason above. But I don't see recent convenience stuff here.

I don't know if it's official but I assume we expect such tools inside of clients code. @kcc ?


================
Comment at: compiler-rt/include/sanitizer/asan_interface.h:343
+#    if __has_feature(address_sanitizer)
+#      define ASAN_NO_INSTR_PTR(TYPE) __attribute__((address_space(1))) TYPE
+#    else
----------------
Why does it need (TYPE) as parameter?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89344/new/

https://reviews.llvm.org/D89344