clang-12 -fsanitize=array-bounds messes with __builtin_constant_p(): invalid asm

[arndb]

Some randconfig kernel builds result in an incorrect asm instruction based on a .insn directive with a non-constant value

<instantiation>:1:7: error: expected constant expression in '.inst' directive
.inst (0xd5200000|((((2) << 19) | ((1) << 16) | (((((reg) >> 7) & 0x7)) << 12) | ((((reg) & 0xf)) << 8) | (((((reg) >> 4) & 0x7)) << 5)))|(.L__reg_num_x0))
      ^
/git/arm-soc/drivers/hwtracing/coresight/coresight-etm4x.h:145:9: note: while in macro instantiation
        return read_sysreg_s(ETM4x_REG_NUM_TO_SYSREG(reg));
               ^
/git/arm-soc/arch/arm64/include/asm/sysreg.h:1108:15: note: expanded from macro 'read_sysreg_s'
        asm volatile(__mrs_s("%0", r) : "=r" (__val));                  \
                     ^
/git/arm-soc/arch/arm64/include/asm/sysreg.h:1074:2: note: expanded from macro '__mrs_s'
"       mrs_s " v ", " __stringify(r) "\n"                      \
 ^
<inline asm>:8:2: note: instantiated into assembly here
        mrs_s x0, (((2) << 19) | ((1) << 16) | (((((reg) >> 7) & 0x7)) << 12) | ((((reg) & 0xf)) << 8) | (((((reg) >> 4) & 0x7)) << 5))
        ^

This comes from a deeply nested macro in the coresight kernel driver, but I reduced it to

struct {
  int a[4];
} b;
char c;
void d(void) {
  int e = 0;
  for (; e < c; e++)
    b.a[e] = ({
      if (__builtin_constant_p(e))
        asm(".inst "
            "0 | 0 | 0 | 0 | 0 + i * 4 >> "
            "2 >> 4 & 7 << 5");
      0;
    });
}

The problem happens with clang-12 and clang-13, but I could not reproduce it with clang-11.

https://godbolt.org/z/bKMsno

[arndb]

Sent a workaround and some more explanation to the driver maintainers: https://lore.kernel.org/lkml/20210225094324.3542511-1-arnd@kernel.org/

[nickdesaulniers]

Is that the letter i in the asm string?

expected constant expression in '.inst' directive

https://godbolt.org/z/3PrzGe Was the reduced case over-reduced? Irrespective of the __builtin_constant_p, I'm curious if there's underlying inline asm we fail to parse.

[arndb]

The underlying problem is passing a register number that is constant according to __builtin_constant_p() but not a literal. The original bug complains about this instruction:

    mrs_s x0, (((2) << 19) | ((1) << 16) | (((((reg) >> 7) & 0x7)) << 12) | ((((reg) & 0xf)) << 8) | (((((reg) >> 4) & 0x7)) << 5))

where the long arithmetic operation is something that the compiler can evaluate to a constant expression at compile-time, but passing it into an inline assembly as a string breaks because 'reg' is not known to the assembler.

The __builtin_constant_p() check works for all the other cases because the argument is usually either a number literal expanded from another macro, or a variable that is not known at compile time. This means the problem is a misuse of __builtin_constant_p() in the kernel that breaks in a few corner cases.

[nathanchance]

Is there a config available to reproduce this issue?

[arndb]

I can still reproduce it with the attached defconfig. I have sent a patch, but that was not accepted so far. I don't think there is anything clang can do to avoid this problem though, as the code in the driver is clearly fragile. One can either rewrite it more thoroughly to avoid the use of these macros (which will make the code slower) or fix it up every time the compiler runs into a corner case with the __builtin_constant_p().

https://lore.kernel.org/linux-arm-kernel/20210225094324.3542511-1-arnd@kernel.org/
0xF18744D0_defconfig.gz

[arndb]

Sent a new patch: https://lore.kernel.org/lkml/20210429145752.3218324-1-arnd@kernel.org/

[nickdesaulniers]

Some randconfig kernel builds...

Is there a config available to reproduce this issue?

In #1625 I was able to determine that this is reproducible with:

$ ARCH=arm64 make LLVM=1 -j72 defconfig
$ ./scripts/config -e CONFIG_CORESIGHT -e CONFIG_CORESIGHT_SOURCE_ETM4X -e UBSAN -e UBSAN_TRAP
$ ARCH=arm64 make LLVM=1 -j72 olddefconfig drivers/hwtracing/coresight/coresight-etm4x-core.o

@arndb was there ever a v3?

[nickdesaulniers]

Looks like someone else sent a v3, but that also had feedback that AFAICT was never followed up on.
https://lore.kernel.org/lkml/1632652550-26048-1-git-send-email-quic_taozha@quicinc.com/

[nickdesaulniers]

Sent a v4: https://lore.kernel.org/llvm/20220614220229.1640085-1-ndesaulniers@google.com/

[nickdesaulniers]

accepted: https://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git/commit/?h=next&id=4d45bc82df667ad9e9cb8361830e54fc1264e993

[nathanchance]

https://git.kernel.org/linus/4d45bc82df667ad9e9cb8361830e54fc1264e993