[ARM32] CompareExchange working wrong in debug build with clang3.9

[Buyduck]

With next C# code:

using System;
using System.Threading;

class Test
{
    static void Main()
    {
        long x = 322;
        Console.WriteLine(x);
        Interlocked.CompareExchange(ref x, 0, 0);
        Console.WriteLine(x);
    }
}

While cross building debug version CoreCLR for ARM32 with clang3.9 got next result:

jenkins@hoult-odroid:~/dotnet-arm$ ./overlayDBG39/corerun test.exe
322
0

And, while building with clang4.0 - everything is ok:

jenkins@hoult-odroid:~/dotnet-arm$ ./overlayDBG40/corerun test.exe 
322
322

With Release version everything is ok. Seems, like in pal.h __sync_val_compare_and_swap() for some reason working wrong:

// See the 32-bit variant in interlock2.s
EXTERN_C
PALIMPORT
inline
LONGLONG
PALAPI
InterlockedCompareExchange64(
    IN OUT LONGLONG volatile *Destination,
    IN LONGLONG Exchange,
    IN LONGLONG Comperand)
{
    return __sync_val_compare_and_swap(
        Destination, /* The pointer to a variable whose value is to be compared with. */
        Comperand, /* The value to be compared */
        Exchange /* The value to be stored */);
}

@alpencolt

[alpencolt]

@dotnet/arm32-contrib please take a look.

[Buyduck]

If I cross comple next c++ program with clang3.9

int main()
{
    long long l = 322;
    __sync_val_compare_and_swap(&l, 0, 0); 
    return l;
}

With next command:

/usr/bin/clang++-3.9 -O0 -L/root/xenial-armhf/lib/arm-linux-gnueabihf -target arm-linux-gnueabihf -B/root/xenial-armhf/usr/lib/gcc/arm-linux-gnueabihf --sysroot=/root/xenial-armhf main.cpp

__sync_val_compare_and_swap() working wrong:

i.vagin@odroid:~$ ./a.out 
i.vagin@odroid:~$ echo $?
0

And if I build it with clang4.0 - I get the correct result:

i.vagin@odroid:~$ ./a.out 
i.vagin@odroid:~$ echo $?
66

[alpencolt]

Dump for 3.9:

00010444 <main>:
   10444:   e92d4830    push  {r4, r5, fp, lr}
   10448:   e24dd020    sub   sp, sp, dotnet/coreclr#32
   1044c:   e3a00000    mov   r0, #0
   10450:   e58d001c    str   r0, [sp, dotnet/runtime#3860]
   10454:   e58d0014    str   r0, [sp, dotnet/coreclr#20]
   10458:   e3a01042    mov   r1, dotnet/coreclr#66  ; 0x42
   1045c:   e3811c01    orr   r1, r1, dotnet/coreclr#256   ; 0x100
   10460:   e58d1010    str   r1, [sp, dotnet/coreclr#16]
   10464:   ee070fba    mcr   15, 0, r0, cr7, cr10, {5}
   10468:   e1a02000    mov   r2, r0
   1046c:   e1a03000    mov   r3, r0
   10470:   e28d1010    add   r1, sp, dotnet/coreclr#16
   10474:   e1b14f9f    ldrexd   r4, [r1]
   10478:   e1540002    cmp   r4, r2
   1047c:   e0d5c003    sbcs  ip, r5, r3
   10480:   1a000002    bne   10490 <main+0x4c>
   10484:   e1a1cf92    strexd   ip, r2, [r1]
   10488:   e35c0000    cmp   ip, #0
   1048c:   1afffff8    bne   10474 <main+0x30>
   10490:   ee070fba    mcr   15, 0, r0, cr7, cr10, {5}
   10494:   e59d0010    ldr   r0, [sp, dotnet/coreclr#16]
   10498:   e58dc00c    str   ip, [sp, dotnet/coreclr#12]
   1049c:   e1cd40f0    strd  r4, [sp]
   104a0:   e28dd020    add   sp, sp, dotnet/coreclr#32
   104a4:   e8bd8830    pop   {r4, r5, fp, pc}

for 4.0:

00010444 <main>:
   10444:   e92d4830    push  {r4, r5, fp, lr}
   10448:   e28db008    add   fp, sp, dotnet/coreclr#8
   1044c:   e24dd020    sub   sp, sp, dotnet/coreclr#32
   10450:   e3a00000    mov   r0, #0
   10454:   e50b000c    str   r0, [fp, #-12]
   10458:   e58d0014    str   r0, [sp, dotnet/coreclr#20]
   1045c:   e3a01042    mov   r1, dotnet/coreclr#66  ; 0x42
   10460:   e3811c01    orr   r1, r1, dotnet/coreclr#256   ; 0x100
   10464:   e58d1010    str   r1, [sp, dotnet/coreclr#16]
   10468:   ee070fba    mcr   15, 0, r0, cr7, cr10, {5}
   1046c:   e1a02000    mov   r2, r0
   10470:   e1a03000    mov   r3, r0
   10474:   e28d1010    add   r1, sp, dotnet/coreclr#16
   10478:   e1b14f9f    ldrexd   r4, [r1]
   1047c:   e1540002    cmp   r4, r2
   10480:   01550003    cmpeq r5, r3
   10484:   1a000002    bne   10494 <main+0x50>
   10488:   e1a1cf92    strexd   ip, r2, [r1]
   1048c:   e35c0000    cmp   ip, #0
   10490:   1afffff8    bne   10478 <main+0x34>
   10494:   ee070fba    mcr   15, 0, r0, cr7, cr10, {5}
   10498:   e59d0010    ldr   r0, [sp, dotnet/coreclr#16]
   1049c:   e58dc00c    str   ip, [sp, dotnet/coreclr#12]
   104a0:   e1cd40f0    strd  r4, [sp]
   104a4:   e24bd008    sub   sp, fp, dotnet/coreclr#8
   104a8:   e8bd8830    pop   {r4, r5, fp, pc}

Difference in one command:

; in 3.9
1047c:   e0d5c003    sbcs  ip, r5, r3
; in 4.0:
10480:   01550003    cmpeq r5, r3

[janvorli]

@alpencolt the code generated by 3.9 is clearly wrong. But it is really strange that our tests on arm would not be crashing left and right with such a bug. What is the exact version of clang - is it 3.9 or 3.9.1? clang --version will show.

[alpencolt]

@janvorli I use 3.9.1:

clang version 3.9.1-4ubuntu3~16.04.1 (tags/RELEASE_391/rc2)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

3.8.1 works well as 3.9.1 with -01. sbcs ip, r5, r3 is being inserted only in -O0 mode. I can report this bug to LLVM but not sure that it will be fixed soon.

@Buyduck could you check your version?

[Buyduck]

@alpencolt I use 3.9.1 too:

clang version 3.9.1-4ubuntu3~16.04.2 (tags/RELEASE_391/rc2)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

I've installed it with:

wget -O - http://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add -
apt-add-repository "deb http://apt.llvm.org/xenial/ llvm-toolchain-xenial-3.9 main"
apt-get -y install clang-3.9

[janvorli]

@alpencolt Thank you for confirming the versions and also mentioning again that it only happens with -O0. I have missed the information in the issue description that it is a debug build issue only.
@Buyduck, I would then recommend either building the debug version with clang 4.0 or using -O1 (which would hurt debuggability though). If you cannot use clang 4.0 for some reason and need to use -O0, then one more option would be to replace the body of the InterlockedCompareExchange64 by inline assembler code. I've just remembered we had to do it during arm64 android bringup, the debug build was screwing the interlocked operations too, but in a different way. See https://github.com/dotnet/coreclr/issues/8927#issuecomment-272586938 (that one is for arm64, so for arm32, it would be different, but you can get some idea from that).

[Buyduck]

@janvorli Thanks a lot!
I was going to write a bug report to llvm, but got respond:

... There's never going to be another
3.9.x release so filing a bug report probably won't do much good.

Also, I can currently cross build CoreCLR for arm only with 'skiptests' key, otherwise I got next error, while compiling hfa_native.cpp:

fatal error: error in backend: Error while trying to spill LR from class GPR: Cannot scavenge register without an emergency spill slot!
clang: error: clang frontend command failed with exit code 70 (use -v to see invocation)
clang version 4.0.0-1ubuntu1~16.04.2 (tags/RELEASE_400/rc1)
Target: armv7--linux-gnueabihf
Thread model: posix
InstalledDir: /usr/bin

cc: @alpencolt

[janvorli]

Nice :-(. Looks like yet another bug in clang. Maybe you can just disable this test for your purpose by adding it to tests/testsFailingOutsideWindows.txt and comment out the contents of the tests/src/JIT/jit64/hfa/main/dll/CMakeLists.txt (I hope that's enough).

[janvorli]

There is nothing we can do about it since it is a compiler bug and LLVM said they will not backfix it in 3.9.1. So we can only recommend people to build with clang 4.0 for arm32. Our official builds are fine since they are release builds and this issue happens only for debug builds. Even checked builds are ok.

[BruceForstall]

@janvorli Should we add documentation to https://github.com/dotnet/coreclr/blob/master/Documentation/building/linux-instructions.md indicating that Debug builds for ARM must use clang4.0 or later?

Should we change build.sh to emit a warning (or error) if building arm debug without this?

Should we try to get the official tools used to build ARM updated to 4.0 or later?

This problem is going to cause many Debug CI jobs for ARM Ubuntu to fail, as described in dotnet/coreclr#16995.

[janvorli]

@BruceForstall yes, such a comment in the doc would be great. It seems we should migrate our builds to clang 4.0 then. Other option, if we really wanted to make it work with the 3.9 for some reason too, is to implement the workaround I've mentioned. Detect that the compiler produces faulty synchronization intrinsics at build time and add inline assembler implementation of our interlocked functions in that case.