On Fri, May 23, 2014 at 04:11:48PM +0400, Konstantin Serebryany wrote:
> >> 2) it doesn't still deal with unaligned power of two accesses properly,
> >> but neither does llvm (at least not 3.4). Am not talking about
> >> undefined behavior cases where the compiler isn't told the access
> >> is misaligned, but e.g. when accessing struct S { int x; }
> >> __attribute__((packed)) and similar (or aligned(1)). Supposedly
> >> we could force __asan_report_*_n for that case too, because
> >> normal wider check assumes it is aligned
> >
> > Yep, we don't do it.
> Now we do: http://llvm.org/viewvc/llvm-project?rev=209508&view=rev
Here is the GCC side of the thing, ok for trunk if it bootstraps/regtests?
(on top of the earlier posted two patches).
Note, I think some work is needed on the library side,
ERROR: AddressSanitizer: unknown-crash on address 0xffc439cf at pc 0x804898e bp
0xffc438d8 sp 0xffc438cc
READ of size 4 at 0xffc439cf thread T0
#0 0x804898d in foo
/usr/src/gcc/gcc/testsuite/c-c++-common/asan/misalign-1.c:10
#1 0x8048746 in main
/usr/src/gcc/gcc/testsuite/c-c++-common/asan/misalign-1.c:34
#2 0x49e39b72 in __libc_start_main (/lib/libc.so.6+0x49e39b72)
#3 0x8048848 (/usr/src/gcc/obj2/gcc/testsuite/gcc/misalign-1.exe+0x8048848)
Address 0xffc439cf is located in stack of thread T0 at offset 175 in frame
#0 0x804868f in main
/usr/src/gcc/gcc/testsuite/c-c++-common/asan/misalign-1.c:27
This frame has 3 object(s):
[32, 36) 'v'
[96, 100) 'w'
[160, 176) 't' <== Memory access at offset 175 partially overflows this
variable
HINT: this may be a false positive if your program uses some custom stack
unwind mechanism or swapcontext
(longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: unknown-crash
/usr/src/gcc/gcc/testsuite/c-c++-common/asan/misalign-1.c:10 foo
Shadow bytes around the buggy address:
0x3ff886e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x3ff886f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x3ff88700: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x3ff88710: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x3ff88720: 00 00 00 00 f1 f1 f1 f1 04 f4 f4 f4 f2 f2 f2 f2
=>0x3ff88730: 04 f4 f4 f4 f2 f2 f2 f2 00[00]f4 f4 f3 f3 f3 f3
0x3ff88740: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x3ff88750: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x3ff88760: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x3ff88770: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x3ff88780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Heap right redzone: fb
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack partial redzone: f4
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
ASan internal: fe
is just too ugly (I mean, it shouldn't print unknown-crash).
It is true that the first byte of the __asan_report_load_n range
corresponds to shadow byte 0, but for _n you should either check
it for all bytes in that range, or at least the first and last byte
(which would correspond to what the caller of __asan_report_*_n
actually does right now).
2014-05-23 Jakub Jelinek <ja...@redhat.com>
* asan.c (report_error_func): Add SLOW_P argument, use
BUILT_IN_ASAN_*_N if set.
(build_check_stmt): Likewise.
(instrument_derefs): If T has insufficient alignment,
force same handling as for odd sizes.
* c-c++-common/asan/misalign-1.c: New test.
* c-c++-common/asan/misalign-2.c: New test.
--- gcc/asan.c.jj 2014-05-23 17:17:46.000000000 +0200
+++ gcc/asan.c 2014-05-23 18:14:08.630807014 +0200
@@ -1319,7 +1319,7 @@ asan_protect_global (tree decl)
IS_STORE is either 1 (for a store) or 0 (for a load). */
static tree
-report_error_func (bool is_store, HOST_WIDE_INT size_in_bytes)
+report_error_func (bool is_store, HOST_WIDE_INT size_in_bytes, bool slow_p)
{
static enum built_in_function report[2][6]
= { { BUILT_IN_ASAN_REPORT_LOAD1, BUILT_IN_ASAN_REPORT_LOAD2,
@@ -1329,7 +1329,8 @@ report_error_func (bool is_store, HOST_W
BUILT_IN_ASAN_REPORT_STORE4, BUILT_IN_ASAN_REPORT_STORE8,
BUILT_IN_ASAN_REPORT_STORE16, BUILT_IN_ASAN_REPORT_STORE_N } };
if ((size_in_bytes & (size_in_bytes - 1)) != 0
- || size_in_bytes > 16)
+ || size_in_bytes > 16
+ || slow_p)
return builtin_decl_implicit (report[is_store][5]);
return builtin_decl_implicit (report[is_store][exact_log2 (size_in_bytes)]);
}
@@ -1508,7 +1509,8 @@ build_shadow_mem_access (gimple_stmt_ite
static void
build_check_stmt (location_t location, tree base, gimple_stmt_iterator *iter,
- bool before_p, bool is_store, HOST_WIDE_INT size_in_bytes)
+ bool before_p, bool is_store, HOST_WIDE_INT size_in_bytes,
+ bool slow_p = false)
{
gimple_stmt_iterator gsi;
basic_block then_bb, else_bb;
@@ -1522,9 +1524,15 @@ build_check_stmt (location_t location, t
HOST_WIDE_INT real_size_in_bytes = size_in_bytes;
tree sz_arg = NULL_TREE;
- if ((size_in_bytes & (size_in_bytes - 1)) != 0
- || size_in_bytes > 16)
- real_size_in_bytes = 1;
+ if (size_in_bytes == 1)
+ slow_p = false;
+ else if ((size_in_bytes & (size_in_bytes - 1)) != 0
+ || size_in_bytes > 16
+ || slow_p)
+ {
+ real_size_in_bytes = 1;
+ slow_p = true;
+ }
/* Get an iterator on the point where we can add the condition
statement for the instrumentation. */
@@ -1582,8 +1590,8 @@ build_check_stmt (location_t location, t
t = gimple_assign_lhs (gimple_seq_last (seq));
gimple_seq_set_location (seq, location);
gsi_insert_seq_after (&gsi, seq, GSI_CONTINUE_LINKING);
- /* For weird access sizes, check first and last byte. */
- if (real_size_in_bytes != size_in_bytes)
+ /* For weird access sizes or misaligned, check first and last byte. */
+ if (slow_p)
{
g = gimple_build_assign_with_ops (PLUS_EXPR,
make_ssa_name (uintptr_type, NULL),
@@ -1626,7 +1634,7 @@ build_check_stmt (location_t location, t
/* Generate call to the run-time library (e.g. __asan_report_load8). */
gsi = gsi_start_bb (then_bb);
- g = gimple_build_call (report_error_func (is_store, size_in_bytes),
+ g = gimple_build_call (report_error_func (is_store, size_in_bytes, slow_p),
sz_arg ? 2 : 1, base_addr, sz_arg);
gimple_set_location (g, location);
gsi_insert_after (&gsi, g, GSI_NEW_STMT);
@@ -1722,8 +1730,31 @@ instrument_derefs (gimple_stmt_iterator
base = build_fold_addr_expr (t);
if (!has_mem_ref_been_instrumented (base, size_in_bytes))
{
+ bool slow_p = false;
+ if (size_in_bytes > 1)
+ {
+ if ((size_in_bytes & (size_in_bytes - 1)) != 0
+ || size_in_bytes > 16)
+ slow_p = true;
+ else
+ {
+ unsigned int align = get_object_alignment (t);
+ if (align < size_in_bytes * BITS_PER_UNIT)
+ {
+ /* On non-strict alignment targets, if
+ 16-byte access is just 8-byte aligned,
+ this will result in misaligned shadow
+ memory 2 byte load, but otherwise can
+ be handled using one read. */
+ if (size_in_bytes != 16
+ || STRICT_ALIGNMENT
+ || align < 8 * BITS_PER_UNIT)
+ slow_p = true;
+ }
+ }
+ }
build_check_stmt (location, base, iter, /*before_p=*/true,
- is_store, size_in_bytes);
+ is_store, size_in_bytes, slow_p);
update_mem_ref_hash_table (base, size_in_bytes);
update_mem_ref_hash_table (t, size_in_bytes);
}
--- gcc/testsuite/c-c++-common/asan/misalign-1.c.jj 2014-05-23
18:03:40.400842565 +0200
+++ gcc/testsuite/c-c++-common/asan/misalign-1.c 2014-05-23
18:29:42.899264340 +0200
@@ -0,0 +1,42 @@
+/* { dg-do run { target { ilp32 || lp64 } } } */
+/* { dg-options "-O2" } */
+/* { dg-shouldfail "asan" } */
+
+struct S { int i; } __attribute__ ((packed));
+
+__attribute__((noinline, noclone)) int
+foo (struct S *s)
+{
+ return s->i;
+}
+
+__attribute__((noinline, noclone)) int
+bar (int *s)
+{
+ return *s;
+}
+
+__attribute__((noinline, noclone)) struct S
+baz (struct S *s)
+{
+ return *s;
+}
+
+int
+main ()
+{
+ struct T { char a[3]; struct S b[3]; char c; } t;
+ int v = 5;
+ struct S *p = t.b;
+ asm volatile ("" : "+rm" (p));
+ p += 3;
+ if (bar (&v) != 5) __builtin_abort ();
+ volatile int w = foo (p);
+ return 0;
+}
+
+/* { dg-output "ERROR: AddressSanitizer:\[^\n\r]*on address\[^\n\r]*" } */
+/* { dg-output "0x\[0-9a-f\]+ at pc 0x\[0-9a-f\]+ bp 0x\[0-9a-f\]+ sp
0x\[0-9a-f\]+\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*READ of size 4 at 0x\[0-9a-f\]+ thread
T0\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output " #0 0x\[0-9a-f\]+ (in
_*foo(\[^\n\r]*misalign-1.c:10|\[^\n\r]*:0)|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output " #1 0x\[0-9a-f\]+ (in _*main
(\[^\n\r]*misalign-1.c:34|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */
--- gcc/testsuite/c-c++-common/asan/misalign-2.c.jj 2014-05-23
18:07:59.820594400 +0200
+++ gcc/testsuite/c-c++-common/asan/misalign-2.c 2014-05-23
18:29:51.808220759 +0200
@@ -0,0 +1,42 @@
+/* { dg-do run { target { ilp32 || lp64 } } } */
+/* { dg-options "-O2" } */
+/* { dg-shouldfail "asan" } */
+
+struct S { int i; } __attribute__ ((packed));
+
+__attribute__((noinline, noclone)) int
+foo (struct S *s)
+{
+ return s->i;
+}
+
+__attribute__((noinline, noclone)) int
+bar (int *s)
+{
+ return *s;
+}
+
+__attribute__((noinline, noclone)) struct S
+baz (struct S *s)
+{
+ return *s;
+}
+
+int
+main ()
+{
+ struct T { char a[3]; struct S b[3]; char c; } t;
+ int v = 5;
+ struct S *p = t.b;
+ asm volatile ("" : "+rm" (p));
+ p += 3;
+ if (bar (&v) != 5) __builtin_abort ();
+ volatile struct S w = baz (p);
+ return 0;
+}
+
+/* { dg-output "ERROR: AddressSanitizer:\[^\n\r]*on address\[^\n\r]*" } */
+/* { dg-output "0x\[0-9a-f\]+ at pc 0x\[0-9a-f\]+ bp 0x\[0-9a-f\]+ sp
0x\[0-9a-f\]+\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*READ of size 4 at 0x\[0-9a-f\]+ thread
T0\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output " #0 0x\[0-9a-f\]+ (in
_*baz(\[^\n\r]*misalign-2.c:22|\[^\n\r]*:0)|\[(\])\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output " #1 0x\[0-9a-f\]+ (in _*main
(\[^\n\r]*misalign-2.c:34|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */
Jakub
