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