Re: [GCC, ARM] armv8 linux toolchain asan testcase fail due to stl missing conditional code

[Kyrill Tkachov <kyrylo dot tkachov at arm dot com>]

Hi Shiva,

On 03/06/15 05:12, Shiva Chen wrote:
> Hi,
>
> I noticed that armv8(32 bit target) linux toolchain
>
> run asan testcase would get the following message:
>
>
> FAIL: c-c++-common/asan/heap-overflow-1.c -O0 output pattern test, is
> Executing on host:
> /home/gccbuilder-x86/test/mgcc5.0/testsuite/../tools/x86_64/install/bin/qemu-arm
> -E LD_LIBRARY_PATH=/home/gccbuilder-x86/test/mgcc5.0/Release/install/armv8-marvell-linux-gnueabihf-hard-5.1.1_x86_64/bin/../arm-linux-gnueabihf/libc/lib/arm-linux-gnueabihf:/home/gccbuilder-x86/test/mgcc5.0/Release/install/armv8-marvell-linux-gnueabihf-hard-5.1.1_x86_64/bin/../arm-linux-gnueabihf/libc/usr/lib/arm-linux-gnueabihf
> -L/home/gccbuilder-x86/test/mgcc5.0/Release/install/armv8-marvell-linux-gnueabihf-hard-5.1.1_x86_64/bin/../arm-linux-gnueabihf/libc
> ./heap-overflow-1.exe
> =================================================================
> ==2182==ERROR: AddressSanitizer: heap-buffer-overflow on address
> 0xf4a007fa at pc 0x000108a0 bp 0xf6ffc264 sp 0xf6ffc25c
> READ of size 1 at 0xf4a007fa thread T0
> ASAN:SIGSEGV
>
>
> sanitizer library use the source in gcc-src/libbacktrace to allocate memory.
>
> The error cause by null pointer reference in libbacktrace/mmap.c
>
>
> void
> backtrace_free (struct backtrace_state *state, void *addr, size_t size,
>                  backtrace_error_callback error_callback ATTRIBUTE_UNUSED,
>                  void *data ATTRIBUTE_UNUSED)
> ...
>    if (locked)
>      {
>        backtrace_free_locked (state, addr, size);
>
>        if (state->threaded) <= line 201
>          __sync_lock_release (&state->lock_alloc); <= line 202
>      }
> }
>
> .loc 1 201 0
> cmp r3, #0 <= r3 contain the value of state->threaded
> .loc 1 202 0
> addne r3, r5, #32
> movne r2, #0
> stl r2, [r3] <= should be conditional execution

I think you're right.

> when r3 is 0, line 202 should not execute.
>
> It seems that stl should generate as stlne.
>
> Otherwise, slt will get null reference when r3 is 0.
>
>
> To fix the issue, add %? when output stl assembly pattern in sync.md.
>
> Is this patch ok for trunk?
>
> Thanks,
> Shiva
>
> Fix_slt_lda_missing_conditional_code.diff
>
>
> diff --git a/gcc/config/arm/sync.md b/gcc/config/arm/sync.md
> index 44cda61..79b039e 100644
> --- a/gcc/config/arm/sync.md
> +++ b/gcc/config/arm/sync.md
> @@ -75,9 +75,9 @@
>     {
>       enum memmodel model = memmodel_from_int (INTVAL (operands[2]));
>       if (is_mm_relaxed (model) || is_mm_consume (model) || is_mm_release (model))
> -      return \"ldr<sync_sfx>\\t%0, %1\";
> +      return \"ldr<sync_sfx>%?\\t%0, %1\";
>       else
> -      return \"lda<sync_sfx>\\t%0, %1\";
> +      return \"lda<sync_sfx>%?\\t%0, %1\";
>     }
>   )

This pattern is not predicable though, i.e. it doesn't have the "predicable" attribute set to "yes".
Therefore the compiler should be trying to branch around here rather than try to do a cond_exec.
Why does the generated code above look like it's converted to conditional execution?
Could you produce a self-contained reduced testcase for this?

Thanks,
Kyrill

>   
> @@ -91,9 +91,9 @@
>     {
>       enum memmodel model = memmodel_from_int (INTVAL (operands[2]));
>       if (is_mm_relaxed (model) || is_mm_consume (model) || is_mm_acquire (model))
> -      return \"str<sync_sfx>\t%1, %0\";
> +      return \"str<sync_sfx>%?\t%1, %0\";
>       else
> -      return \"stl<sync_sfx>\t%1, %0\";
> +      return \"stl<sync_sfx>%?\t%1, %0\";
>     }
>   )
>