Messages in this thread | | | Subject | Re: [PATCH v5 00/18] Rework READ_ONCE() to improve codegen | From | Martin Liška <> | Date | Tue, 9 Jun 2020 13:55:37 +0200 |
| |
On 6/8/20 9:56 PM, Marco Elver wrote: > 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).
Hello.
Works for me and I've just sent patch for that: https://gcc.gnu.org/pipermail/gcc-patches/2020-June/547618.html
> > Note this applies to all sanitizers (TSAN/ASAN/MSAN) and their > no_sanitize attribute that GCC has.
Sure.
> > 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.
The patch is approved now.
And then we also need a param > tsan-instrument-func-entry-exit, which LLVM has for TSAN. One step at > a time though.)
Yes, please send a patch for it.
Martin
> > 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 >>> >>
| |