Messages in this thread | | | From | Marco Elver <> | Date | Mon, 8 Jun 2020 21:56:23 +0200 | Subject | Re: [PATCH v5 00/18] Rework READ_ONCE() to improve codegen |
| |
On Mon, 8 Jun 2020 at 19:32, Martin Liška <mliska@suse.cz> wrote: > > On 6/3/20 9:23 PM, Marco Elver wrote: > > > > > > On Wed, 03 Jun 2020, Borislav Petkov wrote: > > > >> On Thu, May 14, 2020 at 12:05:38PM +0100, Will Deacon wrote: > >>> Talking off-list, Clang >= 7 is pretty reasonable wrt inlining decisions > >>> and the behaviour for __always_inline is: > >>> > >>> * An __always_inline function inlined into a __no_sanitize function is > >>> not instrumented > >>> * An __always_inline function inlined into an instrumented function is > >>> instrumented > >>> * You can't mark a function as both __always_inline __no_sanitize, because > >>> __no_sanitize functions are never inlined > >>> > >>> GCC, on the other hand, may still inline __no_sanitize functions and then > >>> subsequently instrument them. > >> > >> Yeah, about that: I've been looking for a way to trigger this so that > >> I can show preprocessed source to gcc people. So do you guys have a > >> .config or somesuch I can try? > > > > For example take this: > > > > int x; > > > > static inline __attribute__((no_sanitize_thread)) void do_not_sanitize(void) { > > x++; > > } > > > > void sanitize_this(void) { > > do_not_sanitize(); > > } > > > > Then > > > > gcc-10 -O3 -fsanitize=thread -o example.o -c example.c > > objdump -D example.o > > Hello. > > Thank you for the example. It seems to me that Clang does not inline a no_sanitize_* function > into one which is instrumented. Is it a documented behavior ([1] doesn't mention that)? > If so, we can do the same in GCC.
It is not explicitly mentioned in [1]. But the contract of "no_sanitize" is "that a particular instrumentation or set of instrumentations should not be applied". That contract is broken if a function is instrumented, however that may happen. It sadly does happen with GCC when a function is inlined. Presumably because the sanitizer passes for TSAN/ASAN/MSAN run after the optimizer -- this definitely can't change. Also because it currently gives us the property that __always_inline functions are instrumented according to the function they are inlined into (a property we want).
The easy fix to no_sanitize seems to be to do what Clang does, and never inline no_sanitize functions (with or without "inline" attribute). always_inline functions should remain unchanged (specifying no_sanitize on an always_inline function is an error).
Note this applies to all sanitizers (TSAN/ASAN/MSAN) and their no_sanitize attribute that GCC has.
The list of requirements were also summarized in more detail here: https://lore.kernel.org/lkml/CANpmjNMTsY_8241bS7=XAfqvZHFLrVEkv_uM4aDUWE_kh3Rvbw@mail.gmail.com/
Hope that makes sense. (I also need to send a v2 for param tsan-distinguish-volatile, but haven't gotten around to it yet -- hopefully soon. And then we also need a param tsan-instrument-func-entry-exit, which LLVM has for TSAN. One step at a time though.)
Thanks, -- Marco
> Thanks, > Martin > > [1] https://clang.llvm.org/docs/AttributeReference.html#no-sanitize > > > > > will show that do_not_sanitize() was inlined into sanitize_this() and is > > instrumented. (With Clang this doesn't happen.) > > > > Hope this is enough. > > > > Thanks, > > -- Marco > > >
| |