Re: [asan] Emit GIMPLE directly, small cleanups

[Wei Mi <wmi at google dot com>]

Hi,

Here is the initial test results of gcc asan patch, and it shows us
some missing features in gcc but existing in llvm.
[1]. gcc regression test for gcc-asan passes.
[2]. llvm regression tests for gcc-asan: 18 failures in 123 for tests
written in google test and 24 failures in 28 for tests written in lit
tests.

gcc missing features:
1. gcc implementation doesn't support stack/global overflow check
1. gcc implementation doesn't support some attributes, such as
__attribute__((no_address_safety_analysis)), which llvm does
2. gcc doesn't detect out-of-bound bitfield access of heap, which llvm does
3. gcc doesn't detect out-of-bound memset, memcopy, strcat and strcpy
for heap allocated memory or string, which llvm does
4. gcc doesn't contain similar options: -mllvm -asan-blacklist, -mllvm
-asan-initialization-order
5. gcc -fasan doesn't take effect at -O0, but llvm does. Most lit
tests contain checks from -O0 to -O3, which makes gcc fail.
6. $HOME/llvm/trunk/projects/compiler-rt/lib/asan/scripts/asan_symbolize.py
could generate valid source locations from virtual addresses for llvm
binary, but fail to do that for gcc binary.  example, llvm result #1
0x402694 in main heap-overflow.cc:23 .vs. gcc result: #1 0x402694 in
main ??:0. Some FileCheck in llvm lit tests expect the valid source
locations.

Thanks,
Wei.


On Thu, Oct 11, 2012 at 10:31 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Oct 11, 2012 at 01:14:31PM -0400, Diego Novillo wrote:
>> On 2012-10-11 12:38 , Jakub Jelinek wrote:
>>
>> >-  gimple_seq seq, stmts;
>> >-  tree shadow_type = size_in_bytes == 16 ?
>> >-      short_integer_type_node : char_type_node;
>> >-  tree shadow_ptr_type = build_pointer_type (shadow_type);
>> >-  tree uintptr_type = lang_hooks.types.type_for_mode (ptr_mode,
>> >-                                                      /*unsignedp=*/true);
>> >+  tree shadow_ptr_type = shadow_ptr_types[size_in_bytes == 16];
>>
>> Add '? 1 : 0' in the array index expression.
>
> Ok.
>
>> >    /* Build
>> >-     (base_addr >> ASAN_SHADOW_SHIFT) | targetm.asan_shadow_offset ().  */
>> >+     (base_addr >> ASAN_SHADOW_SHIFT) + targetm.asan_shadow_offset ().  */
>>
>> Hm, I wonder if this is a documentation problem or we're generating
>> bad runtime code.  Wei, you tested the runtime and it was working
>> with the GCC generated code, right?
>
> The asan web pages document |, the old tree-asan.c emitted +, I've changed
> it to BIT_IOR_EXPR, but that resulted in worse assembly, and I believe at
> least for the current x86_64 and i686 address ranges and shadow offset
> values it actually doesn't matter.
> On x86_64 stack is like 0x7ffff6e00000, shifted down by 3 is still smaller
> than 1L << 44 that is ored or added to it.  And the negative half of the
> address space is used by the kernel, nothing is mapped into it (besides
> vsyscall page) and neither | nor + of 1L << 44 to it would work well.
> On i386, | and + works the same for all addresses, as 0xffffffffU >> 3
> is still smaller than 1 << 29.
> The reason why + generates better code on x86_64/i686 is that one can use
> e.g. movzbl (%r1, %r2), %r3 instead of orq %r2, %r1; movzb (%r1), %r3.
>
>> >+  if (shadow_ptr_types[0] == NULL_TREE)
>> >+    {
>> >+      alias_set_type set = new_alias_set ();
>> >+      shadow_ptr_types[0]
>> >+    = build_distinct_type_copy (unsigned_char_type_node);
>> >+      TYPE_ALIAS_SET (shadow_ptr_types[0]) = set;
>> >+      shadow_ptr_types[0] = build_pointer_type (shadow_ptr_types[0]);
>> >+      shadow_ptr_types[1]
>> >+    = build_distinct_type_copy (short_unsigned_type_node);
>> >+      TYPE_ALIAS_SET (shadow_ptr_types[1]) = set;
>> >+      shadow_ptr_types[1] = build_pointer_type (shadow_ptr_types[1]);
>> >+    }
>>
>> Move this to an initialization function, please.
>
> Okay.
>
>         Jakub