Message ID | 0575e910-77da-cd5a-555b-95ea8432fd32@suse.cz |
---|---|
State | New |
Headers | show |
Series | sanitizer: do not inline no-sanitize into sanitizer fn | expand |
On Tue, Jun 09, 2020 at 01:53:38PM +0200, Martin Liška wrote: > + const sanitize_code codes[] = > + { > + SANITIZE_ADDRESS, > + SANITIZE_THREAD, > + SANITIZE_LEAK, Why leak? That is really a link time option only, doesn't affect code generation in any way. On the other side, does the SANITIZE_ADDRESS you have in there handle properly also -fsanitize=kernel-address cases? Jakub
On 6/9/20 2:01 PM, Jakub Jelinek wrote: > On Tue, Jun 09, 2020 at 01:53:38PM +0200, Martin Liška wrote: >> + const sanitize_code codes[] = >> + { >> + SANITIZE_ADDRESS, >> + SANITIZE_THREAD, >> + SANITIZE_LEAK, > > Why leak? I was too eager ;) > That is really a link time option only, doesn't affect code > generation in any way. > > On the other side, does the SANITIZE_ADDRESS you have in there > handle properly also -fsanitize=kernel-address cases? Yes, I've just added a new test-case for it. Martin > > Jakub >
On Tue, Jun 09, 2020 at 02:09:06PM +0200, Martin Liška wrote: > - return ((sanitize_flags_p (SANITIZE_ADDRESS, caller) > - == sanitize_flags_p (SANITIZE_ADDRESS, callee)) > - && (sanitize_flags_p (SANITIZE_POINTER_COMPARE, caller) > - == sanitize_flags_p (SANITIZE_POINTER_COMPARE, callee)) > - && (sanitize_flags_p (SANITIZE_POINTER_SUBTRACT, caller) > - == sanitize_flags_p (SANITIZE_POINTER_SUBTRACT, callee))); > + const sanitize_code codes[] = > + { > + SANITIZE_ADDRESS, > + SANITIZE_THREAD, > + SANITIZE_UNDEFINED, > + SANITIZE_UNDEFINED_NONDEFAULT > + }; > + > + for (unsigned i = 0; i < sizeof (codes) / sizeof (codes[0]); i++) > + if (sanitize_flags_p (codes[i], caller) > + != sanitize_flags_p (codes[i], callee)) > + return false; Sorry for not writing everything at once, but are the SANITIZER_POINTER_{COMPARE,SUBTRACT} differences unimportant? Jakub
On 6/9/20 2:15 PM, Jakub Jelinek wrote: > Sorry for not writing everything at once, but are the > SANITIZER_POINTER_{COMPARE,SUBTRACT} differences unimportant? They are. I got confused that they are not part of SANITIZE_UNDEFINED or SANITIZE_UNDEFINED_NONDEFAULT. Anyway, good point! Martin
On Tue, Jun 09, 2020 at 02:32:36PM +0200, Martin Liška wrote: > >From 5fe0671ad79d14d1c9d0fead1a471875a4416fac Mon Sep 17 00:00:00 2001 > From: Martin Liska <mliska@suse.cz> > Date: Tue, 9 Jun 2020 13:03:55 +0200 > Subject: [PATCH] sanitizer: do not inline no-sanitize into sanitizer fn > > gcc/ChangeLog: > > * cif-code.def (ATTRIBUTE_MISMATCH): Rename to... > (SANITIZE_ATTRIBUTE_MISMATCH): ...this. > * ipa-inline.c (sanitize_attrs_match_for_inline_p): > Handle all sanitizer options. > (can_inline_edge_p): Use renamed CIF_* enum value. > > gcc/testsuite/ChangeLog: > > * c-c++-common/asan/inline.c: New test. > * c-c++-common/asan/inline-kernel.c: New test. > * c-c++-common/tsan/inline.c: New test. > * c-c++-common/ubsan/inline.c: New test. Ok, thanks. Jakub
Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > On Tue, Jun 09, 2020 at 02:32:36PM +0200, Martin Liška wrote: >> >From 5fe0671ad79d14d1c9d0fead1a471875a4416fac Mon Sep 17 00:00:00 2001 >> From: Martin Liska <mliska@suse.cz> >> Date: Tue, 9 Jun 2020 13:03:55 +0200 >> Subject: [PATCH] sanitizer: do not inline no-sanitize into sanitizer fn >> >> gcc/ChangeLog: >> >> * cif-code.def (ATTRIBUTE_MISMATCH): Rename to... >> (SANITIZE_ATTRIBUTE_MISMATCH): ...this. >> * ipa-inline.c (sanitize_attrs_match_for_inline_p): >> Handle all sanitizer options. >> (can_inline_edge_p): Use renamed CIF_* enum value. >> >> gcc/testsuite/ChangeLog: >> >> * c-c++-common/asan/inline.c: New test. >> * c-c++-common/asan/inline-kernel.c: New test. >> * c-c++-common/tsan/inline.c: New test. >> * c-c++-common/ubsan/inline.c: New test. > > Ok, thanks. The new inline-kernel.c testcase FAILs on Solaris/SPARC and x86 (and everywhere else, I assume) with +UNRESOLVED: c-c++-common/asan/inline-kernel.c -O0 scan-tree-dump-times optimized "Function do_not_sanitize" 1 +FAIL: c-c++-common/asan/inline-kernel.c -O0 (test for excess errors) +UNRESOLVED: c-c++-common/asan/inline-kernel.c -O1 scan-tree-dump-times optimized "Function do_not_sanitize" 1 +FAIL: c-c++-common/asan/inline-kernel.c -O1 (test for excess errors) +UNRESOLVED: c-c++-common/asan/inline-kernel.c -O2 scan-tree-dump-times optimized "Function do_not_sanitize" 1 +FAIL: c-c++-common/asan/inline-kernel.c -O2 (test for excess errors) +UNRESOLVED: c-c++-common/asan/inline-kernel.c -O2 -flto scan-tree-dump-times optimized "Function do_not_sanitize" 1 +FAIL: c-c++-common/asan/inline-kernel.c -O2 -flto (test for excess errors) +UNRESOLVED: c-c++-common/asan/inline-kernel.c -O2 -flto -flto-partition=none scan-tree-dump-times optimized "Function do_not_sanitize" 1 +FAIL: c-c++-common/asan/inline-kernel.c -O2 -flto -flto-partition=none (test for excess errors) +UNRESOLVED: c-c++-common/asan/inline-kernel.c -O3 -g scan-tree-dump-times optimized "Function do_not_sanitize" 1 +FAIL: c-c++-common/asan/inline-kernel.c -O3 -g (test for excess errors) +UNRESOLVED: c-c++-common/asan/inline-kernel.c -Os scan-tree-dump-times optimized "Function do_not_sanitize" 1 +FAIL: c-c++-common/asan/inline-kernel.c -Os (test for excess errors) Excess errors: cc1: error: '-fsanitize=address' is incompatible with '-fsanitize=kernel-address' Rainer
On 6/9/20 9:42 PM, Rainer Orth wrote: > Excess errors: > cc1: error: '-fsanitize=address' is incompatible with '-fsanitize=kernel-address' Sorry for that, I'm going to install the following patch. Martin
Hi! On Tue, Jun 09, 2020 at 09:58:11PM +0200, Martin Liška wrote: > On 6/9/20 9:42 PM, Rainer Orth wrote: > > Excess errors: > > cc1: error: '-fsanitize=address' is incompatible with '-fsanitize=kernel-address' > > Sorry for that, I'm going to install the following patch. These tests are UNRESOLVED because -fdump-tree-optimized can't be scanned with slim LTO. Other *san/ tests deal with this by adding -ffat-lto-objects. Tested on x86_64-linux, committed to trunk as obvious. 2020-06-16 Jakub Jelinek <jakub@redhat.com> * c-c++-common/asan/inline.c: Add -ffat-lto-objects to dg-options. * c-c++-common/asan/inline-kernel.c: Likewise. * c-c++-common/ubsan/inline.c: Likewise. --- gcc/testsuite/c-c++-common/asan/inline.c.jj 2020-06-10 14:58:17.539494212 +0200 +++ gcc/testsuite/c-c++-common/asan/inline.c 2020-06-16 18:17:36.011197816 +0200 @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-fsanitize=address -c -O3 -fdump-tree-optimized" } */ +/* { dg-options "-fsanitize=address -c -O3 -fdump-tree-optimized -ffat-lto-objects" } */ int x; --- gcc/testsuite/c-c++-common/asan/inline-kernel.c.jj 2020-06-10 14:58:17.539494212 +0200 +++ gcc/testsuite/c-c++-common/asan/inline-kernel.c 2020-06-16 18:17:51.096981545 +0200 @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-fno-sanitize=address -fsanitize=kernel-address -c -O3 -fdump-tree-optimized" } */ +/* { dg-options "-fno-sanitize=address -fsanitize=kernel-address -c -O3 -fdump-tree-optimized -ffat-lto-objects" } */ int x; --- gcc/testsuite/c-c++-common/ubsan/inline.c.jj 2020-06-10 14:58:17.539494212 +0200 +++ gcc/testsuite/c-c++-common/ubsan/inline.c 2020-06-16 18:18:08.037738683 +0200 @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-fsanitize=vla-bound -c -O3 -fdump-tree-optimized" } */ +/* { dg-options "-fsanitize=vla-bound -c -O3 -fdump-tree-optimized -ffat-lto-objects" } */ int x; Jakub
diff --git a/gcc/cif-code.def b/gcc/cif-code.def index 31c18c6c691..c65b2477203 100644 --- a/gcc/cif-code.def +++ b/gcc/cif-code.def @@ -128,9 +128,10 @@ DEFCIFCODE(OPTIMIZATION_MISMATCH, CIF_FINAL_ERROR, DEFCIFCODE(USES_COMDAT_LOCAL, CIF_FINAL_ERROR, N_("callee refers to comdat-local symbols")) -/* We can't inline because of mismatched caller/callee attributes. */ -DEFCIFCODE(ATTRIBUTE_MISMATCH, CIF_FINAL_ERROR, - N_("function attribute mismatch")) +/* We can't inline because of mismatched caller/callee + sanitizer attributes. */ +DEFCIFCODE(SANITIZE_ATTRIBUTE_MISMATCH, CIF_FINAL_ERROR, + N_("sanitizer function attribute mismatch")) /* We can't inline because the user requests only static functions but the function has external linkage for live patching purpose. */ diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c index f71443feff7..edf4095bcbc 100644 --- a/gcc/ipa-inline.c +++ b/gcc/ipa-inline.c @@ -264,18 +264,25 @@ sanitize_attrs_match_for_inline_p (const_tree caller, const_tree callee) if (!caller || !callee) return true; - /* Allow inlining always_inline functions into no_sanitize_address - functions. */ - if (!sanitize_flags_p (SANITIZE_ADDRESS, caller) - && lookup_attribute ("always_inline", DECL_ATTRIBUTES (callee))) + /* Follow clang and allow inlining for always_inline functions. */ + if (lookup_attribute ("always_inline", DECL_ATTRIBUTES (callee))) return true; - return ((sanitize_flags_p (SANITIZE_ADDRESS, caller) - == sanitize_flags_p (SANITIZE_ADDRESS, callee)) - && (sanitize_flags_p (SANITIZE_POINTER_COMPARE, caller) - == sanitize_flags_p (SANITIZE_POINTER_COMPARE, callee)) - && (sanitize_flags_p (SANITIZE_POINTER_SUBTRACT, caller) - == sanitize_flags_p (SANITIZE_POINTER_SUBTRACT, callee))); + const sanitize_code codes[] = + { + SANITIZE_ADDRESS, + SANITIZE_THREAD, + SANITIZE_LEAK, + SANITIZE_UNDEFINED, + SANITIZE_UNDEFINED_NONDEFAULT + }; + + for (unsigned i = 0; i < sizeof (codes) / sizeof (codes[0]); i++) + if (sanitize_flags_p (codes[i], caller) + != sanitize_flags_p (codes[i], callee)) + return false; + + return true; } /* Used for flags where it is safe to inline when caller's value is @@ -382,7 +389,7 @@ can_inline_edge_p (struct cgraph_edge *e, bool report, /* Don't inline a function with mismatched sanitization attributes. */ else if (!sanitize_attrs_match_for_inline_p (caller->decl, callee->decl)) { - e->inline_failed = CIF_ATTRIBUTE_MISMATCH; + e->inline_failed = CIF_SANITIZE_ATTRIBUTE_MISMATCH; inlinable = false; } if (!inlinable && report) diff --git a/gcc/testsuite/c-c++-common/asan/inline.c b/gcc/testsuite/c-c++-common/asan/inline.c new file mode 100644 index 00000000000..7c36702cf9e --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/inline.c @@ -0,0 +1,20 @@ +/* { dg-do compile } */ +/* { dg-options "-fsanitize=address -c -O3 -fdump-tree-optimized" } */ + +int x; + +static inline +__attribute__((no_sanitize("address"))) +void do_not_sanitize(void) +{ + x++; +} + +void +sanitize_this(void) +{ + x++; + do_not_sanitize(); +} + +/* { dg-final { scan-tree-dump-times "Function do_not_sanitize" 1 "optimized" } } */ diff --git a/gcc/testsuite/c-c++-common/tsan/inline.c b/gcc/testsuite/c-c++-common/tsan/inline.c new file mode 100644 index 00000000000..7fb3e576f54 --- /dev/null +++ b/gcc/testsuite/c-c++-common/tsan/inline.c @@ -0,0 +1,20 @@ +/* { dg-do compile } */ +/* { dg-options "-fsanitize=thread -c -O3 -fdump-tree-optimized" } */ + +int x; + +static inline +__attribute__((no_sanitize("thread"))) +void do_not_sanitize(void) +{ + x++; +} + +void +sanitize_this(void) +{ + x++; + do_not_sanitize(); +} + +/* { dg-final { scan-tree-dump-times "Function do_not_sanitize" 1 "optimized" } } */ diff --git a/gcc/testsuite/c-c++-common/ubsan/inline.c b/gcc/testsuite/c-c++-common/ubsan/inline.c new file mode 100644 index 00000000000..de9660427f6 --- /dev/null +++ b/gcc/testsuite/c-c++-common/ubsan/inline.c @@ -0,0 +1,20 @@ +/* { dg-do compile } */ +/* { dg-options "-fsanitize=vla-bound -c -O3 -fdump-tree-optimized" } */ + +int x; + +static inline +__attribute__((no_sanitize("undefined"))) +void do_not_sanitize(void) +{ + x++; +} + +void +sanitize_this(void) +{ + x++; + do_not_sanitize(); +} + +/* { dg-final { scan-tree-dump-times "Function do_not_sanitize" 1 "optimized" } } */