[PATCH] D89344: Introduce convenience macro ASAN_NO_INSTR_PTR.

[Dan Liew via Phabricator via llvm-commits]

delcypher added a comment.

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

> In D89344#2328787 <https://reviews.llvm.org/D89344#2328787>, @yln wrote:
>
>>> Previously __attribute__((address_space(1))) had to be applied to pointer types to avoid instrumentation.
>>
>> I wasn't aware of this.  Is this a principled way to tell ASan to skip instrumentation of a local var, or is it used because "it works" (but it depends on implementation details)?
>> Note that I don't know the actual meaning/semantics of `address_space`.
>
> My understanding is that attribute is used "because it works", not because it's principled in anyway.
>
> IIUC correctly the `address_space` attribute maps to LLVM IR's notion of address space. Here's a snippet from the LLVM language reference
>
>   A global variable may be declared to reside in a target-specific numbered address space. For targets that support them, address spaces may affect how optimizations are performed and/or what target instructions are used to access the variable. The default address space is zero. The address space qualifier must precede any other attributes.
>
> This suggests that there might be downsides to using this attribute (e.g. it might inhibit optimizations)
>
>> Would generalizing `__attribute__((no_sanitize("address")))` to also apply to local vars (of pointer type) be a cleaner way?
>> https://clang.llvm.org/docs/AddressSanitizer.html#disabling-instrumentation-with-attribute-no-sanitize-address
>
> I don't think that will work because I think `no_sanitize` is a function level attribute.
>
> If there is a more principled attribute then I'm happy to change this patch to use it. If such an attribute doesn't exist then I do not think that this patch should be blocked on having a more principled attribute . The whole point of this patch is to introduce a macro that hides the **current** implementation details. Having the macro introduced in this patch means that we can change the underlying implementation details and users of the macro will not need to care.

@yln I tried using the attribute on a local variable. It's not supported.

  Volumes/data/dev/llvm/llvm.org/master/llvm/compiler-rt/test/asan/TestCases/no_instrument_pointer.cpp:35:3: error: 'no_sanitize' attribute only applies to functions, Objective-C methods, and global variables
    ASAN_NO_INSTR_PTR(int *)
    ^
  /Volumes/data/dev/llvm/llvm.org/master/builds/RelWithDebInfo/lib/clang/12.0.0/include/sanitizer/asan_interface.h:343:54: note: expanded from macro 'ASAN_NO_INSTR_PTR'
  #      define ASAN_NO_INSTR_PTR(TYPE) __attribute__((no_sanitize("address"))) TYPE
                                                       ^
  /Volumes/data/dev/llvm/llvm.org/master/llvm/compiler-rt/test/asan/TestCases/no_instrument_pointer.cpp:36:10: warning: 'no_sanitize' attribute ignored when parsing type [-Wignored-attributes]
    ptr = (ASAN_NO_INSTR_PTR(int *))source();
           ^~~~~~~~~~~~~~~~~~~~~~~~
  /Volumes/data/dev/llvm/llvm.org/master/builds/RelWithDebInfo/lib/clang/12.0.0/include/sanitizer/asan_interface.h:343:54: note: expanded from macro 'ASAN_NO_INSTR_PTR'
  #      define ASAN_NO_INSTR_PTR(TYPE) __attribute__((no_sanitize("address"))) TYPE
                                                       ^~~~~~~~~~~~~~~~~~~~~~
  1 warning and 1 error generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89344