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

[Kyrill Tkachov]

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\";
    }
  )