This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 0/17] KASan 4.9 backport
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Yury Gribov <y dot gribov at samsung dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Dmitry Vyukov <dvyukov at google dot com>, Konstantin Khlebnikov <k dot khlebnikov at samsung dot com>, Andrey Ryabinin <a dot ryabinin at samsung dot com>, Viacheslav Garbuzov <v dot garbuzov at samsung dot com>
- Date: Fri, 17 Oct 2014 15:49:46 +0200
- Subject: Re: [PATCH 0/17] KASan 4.9 backport
- Authentication-results: sourceware.org; auth=none
- References: <543F831B dot 4090502 at samsung dot com> <20141017121217 dot GQ10376 at tucnak dot redhat dot com> <54411D6D dot 7000009 at samsung dot com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Fri, Oct 17, 2014 at 05:45:17PM +0400, Yury Gribov wrote:
> On 10/17/2014 04:12 PM, Jakub Jelinek wrote:
> >I had a brief look at what ended up on the branch in the end, and
> >from what I understand, the 4.9 libasan.so has
> >__asan_report_store_n and __asan_report_load_n entry points, but does
> >not have any __asan_loadN/__asan_reportN entrypoints (neither 1/2/4/8/16,
> >nor variable).
> >So, what the branch does seems to not match what the library provides.
>
> I agree, __asan_report_loadN is indeed there and misalign tests seem to pass
> fine. Probably I should have examined 4.9 libasan closer.
>
> >With -fsanitize=address -O2 --param asan-instrumentation-with-call-threshold=0
> >foo is again unexpectedly not instrumented, and bar is instrumented
> >with __asan_load8, which looks wrong to me, because the library does not
> >provide any such entry point.
>
> By default asan-instrumentation-with-call-threshold is INT_MAX which means
> that compiler will never generate __asan_load*/__asan_store* calls unless
> forced by the user (e.g. for Kasan).
>
> >But, in execute_sanopt force !use_calls
> >for (flag_sanitize & SANITIZE_USER_ADDRESS).
>
> Do you think above limitation is not enough?
Yeah, even if the default is that it doesn't make the non-existing calls,
anyone who uses the parameter will get code that doesn't link.
>
> >Thus, IMHO the:
> > if ((flag_sanitize & SANITIZE_USER_ADDRESS) != 0
> > && ((size_in_bytes & (size_in_bytes - 1)) != 0
> > || (unsigned HOST_WIDE_INT) size_in_bytes - 1 >= 16))
> > return;
> >should be nuked from 4.9, we can do unaligned/non-{1,2,4,8,16}
> >accesses fine.
>
> Right. I'd also import misalign tests.
>
> >Or were there any bugfixes needed for __asan_report_{store,load}_n
> >on the library side?
>
> I don't think so.
So, what about this? Just checked that with
make -k check-g{cc,++} RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} asan.exp tsan.exp ubsan.exp'
so far. Plus if you add misalign tests...
2014-10-17 Jakub Jelinek <jakub@redhat.com>
* asan.c (instrument_derefs): Allow instrumentation of odd-sized
accesses even for -fsanitize=address.
(execute_sanopt): Only allow use_calls for -fsanitize=kernel-address.
* c-c++-common/asan/instrument-with-calls-1.c: Add
-fno-sanitize=address -fsanitize=kernel-address to dg-options.
* c-c++-common/asan/instrument-with-calls-2.c: Likewise.
--- gcc/asan.c.jj 2014-10-17 12:51:27.000000000 +0200
+++ gcc/asan.c 2014-10-17 15:21:29.921495259 +0200
@@ -1707,10 +1707,6 @@ instrument_derefs (gimple_stmt_iterator
size_in_bytes = int_size_in_bytes (type);
if (size_in_bytes <= 0)
return;
- if ((flag_sanitize & SANITIZE_USER_ADDRESS) != 0
- && ((size_in_bytes & (size_in_bytes - 1)) != 0
- || (unsigned HOST_WIDE_INT) size_in_bytes - 1 >= 16))
- return;
HOST_WIDE_INT bitsize, bitpos;
tree offset;
@@ -2780,8 +2776,10 @@ execute_sanopt (void)
}
}
- bool use_calls = ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD < INT_MAX
- && asan_num_accesses >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;
+ bool use_calls
+ = ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD < INT_MAX
+ && (flag_sanitize & SANITIZE_KERNEL_ADDRESS)
+ && asan_num_accesses >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;
FOR_EACH_BB_FN (bb, cfun)
{
--- gcc/testsuite/c-c++-common/asan/instrument-with-calls-1.c.jj 2014-10-17 12:51:27.000000000 +0200
+++ gcc/testsuite/c-c++-common/asan/instrument-with-calls-1.c 2014-10-17 15:34:06.679627168 +0200
@@ -1,5 +1,5 @@
/* { dg-do assemble } */
-/* { dg-options "--param asan-instrumentation-with-call-threshold=0 -save-temps" } */
+/* { dg-options "-fno-sanitize=address -fsanitize=kernel-address --param asan-instrumentation-with-call-threshold=0 -save-temps" } */
void f(char *a, int *b) {
*b = *a;
--- gcc/testsuite/c-c++-common/asan/instrument-with-calls-2.c.jj 2014-10-17 12:51:27.000000000 +0200
+++ gcc/testsuite/c-c++-common/asan/instrument-with-calls-2.c 2014-10-17 15:34:15.569472032 +0200
@@ -1,5 +1,5 @@
/* { dg-do assemble } */
-/* { dg-options "--param asan-instrumentation-with-call-threshold=1 -save-temps" } */
+/* { dg-options "-fno-sanitize=address -fsanitize=kernel-address --param asan-instrumentation-with-call-threshold=1 -save-temps" } */
int x;
Jakub