[BUG] bad debug-info line numbering with arm64 Clang in NDK r20

[rprichard]

I'm filing this bug to track the upstream LLVM bug, https://bugs.llvm.org/show_bug.cgi?id=40887.

Clang sometimes hoists instructions loading a symbol's address to the start of a function, then marks them with the line number of their first reference. The effect is that:

• Stepping through the hoisted instructions appears to step through various lines of the function (even those that never execute).
• I can set a breakpoint on a line that doesn't execute, and the breakpoint will hit.

AFAICT, the issue only affects arm64, not the other NDK architectures (arm32, x86-32, or x86-64).

The issue affects:

• clang-r349610b
• clang-r353983c
• NDK r20 (uses r346389c)

Unaffected:

• clang-r339409b
• NDK r19c (uses r339409)

With r19c on the same test case, I still see symbol references that are hoisted to the start of the function, but they're marked with the line number of the function's opening curly brace.

If I add -fno-experimental-isel to the CFLAGS, then the instructions aren't hoisted anymore, and the problem stops reproducing.

Example function:

(create a new NDK C++ project in Android Studio and replace native-lib.cpp with this file)

#include <jni.h>

int var1;
int var2;
int var3;
int var4;
int var5;

extern "C" void foo() {     // 9
    if (var1 & 1) {         // 10
        var2 = 2;           // 11
    }                       // 12
    if (var1 & 2) {         // 13
        var3 = 2;           // 14
    }                       // 15
    if (var1 & 4) {         // 16
        var4 = 2;           // 17
    }                       // 18
    if (var1 & 8) {         // 19
        var5 = 2;           // 20
    }                       // 21
}                           // 22

extern "C" JNIEXPORT jstring JNICALL
Java_com_example_hellodebugging_MainActivity_stringFromJNI(
        JNIEnv *env,
        jobject /* this */)
{
    foo();
    return env->NewStringUTF("hello");
}

Set a breakpoint on the "foo()" call, then step-into repeatedly. lldb advances through lines in this order: 10, 11, 14, 17, 20, 10, 13, 16, 19, 22.

Restart. Set a breakpoint on lines 11, 14, 17, and 20, then run. The debugger hits all four lines.

[rprichard]

Chromium is passing -fno-experimental-isel for their iOS debug builds:

https://chromium-review.googlesource.com/c/chromium/src/+/1492044

[rprichard]

With NDK r20's Clang (5220042 based on r346389c), I think GlobalISel is off by default, except for ARM64 with -O0. It can be turned on for other configurations using -fexperimental-isel, but then Clang will print a warning like:

• clang80: warning: -fexperimental-isel support for the 'armv7' architecture is incomplete [-Wexperimental-isel]

• clang80: warning: -fexperimental-isel support is incomplete for this architecture at the current optimization level [-Wexperimental-isel]

With -fno-experimental-isel, ARM64 -O0 appears to use the FastISel instruction selector. Higher optimization levels instead use the SelectionDAG selector.

https://reviews.llvm.org/D41362, https://reviews.llvm.org/D42276

https://android.googlesource.com/toolchain/clang/+/b55f2d4ebfd35bf643d27dbca1bb228957008617/lib/Driver/ToolChains/Clang.cpp#5070

https://android.googlesource.com/toolchain/llvm/+/3c393fe7a7e13b0fba4ac75a01aa683d7a5b11cd/lib/CodeGen/TargetPassConfig.cpp#720

https://android.googlesource.com/toolchain/llvm/+/3c393fe7a7e13b0fba4ac75a01aa683d7a5b11cd/lib/Target/AArch64/AArch64TargetMachine.cpp#141

https://android.googlesource.com/toolchain/llvm/+/3c393fe7a7e13b0fba4ac75a01aa683d7a5b11cd/lib/Target/AArch64/AArch64TargetMachine.cpp#277

[stephenhines]

https://reviews.llvm.org/D63286 is the upstream patch for this. I can talk to Nick about ensuring this is in our next toolchain update.

[rprichard]

I uploaded https://android-review.googlesource.com/c/platform/ndk/+/982883 to turn off -fno-experimental-isel until upstream is fixed. I think this can go into r20b. (Maybe I should reupload that patch for ndk-release-r20 and leave master alone?)

[DanAlbert]

Patch should go into master as well just in case (also helps any canary users).

[stephenhines]

@nickdesaulniers - this is the bug/patch we should ensure is in the next update.

[rprichard]

The -fno-experimental-isel is merged into master and ndk-release-r20. It should be in r20b. I'll leave this issue open until our toolchain is fixed.

[DanAlbert]

Master has r365631, and the fix was r363331. This commit cleans up all the now unnecessary -fno-experimental-isel: https://android-review.googlesource.com/c/platform/ndk/+/1113972