ld.lld: error: undefined symbol: __bad_mask in gpi_update_reg

[nathanchance]

When CONFIG_UBSAN_UNSIGNED_OVERFLOW + CONFIG_QCOM_GPI_DMA are enabled:

$ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- LLVM=1 O=out/arm64 distclean defconfig

$ scripts/config \
--file out/arm64/.config \
-e QCOM_GPI_DMA \
-e UBSAN \
-e UBSAN_UNSIGNED_OVERFLOW

$ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- LLVM=1 O=out/arm64 olddefconfig vmlinux
...
ld.lld: error: undefined symbol: __bad_mask
>>> referenced by bitfield.h:119 (/home/nathan/src/linux/include/linux/bitfield.h:119)
>>>               dma/qcom/gpi.o:(gpi_update_reg) in archive drivers/built-in.a
>>> referenced by bitfield.h:119 (/home/nathan/src/linux/include/linux/bitfield.h:119)
>>>               dma/qcom/gpi.o:(gpi_update_reg) in archive drivers/built-in.a
...

Marking gpi_update_reg as __always_inline fixes it, although I am not sure if that is a proper fix?

diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
index d2334f535de2..8d39d3e24686 100644
--- a/drivers/dma/qcom/gpi.c
+++ b/drivers/dma/qcom/gpi.c
@@ -584,7 +584,7 @@ static inline void gpi_write_reg_field(struct gpii *gpii, void __iomem *addr,
        gpi_write_reg(gpii, addr, val);
 }

-static inline void
+static __always_inline void
 gpi_update_reg(struct gpii *gpii, u32 offset, u32 mask, u32 val)
 {
        void __iomem *addr = gpii->regs + offset;

cvise spits out

field_mask_field;
static u32_encode_bits(field) {
  unsigned __trans_tmp_2 = field_mask_field, __trans_tmp_4 = field;
  __trans_tmp_2 - 1;
  if (__trans_tmp_4 - 1 + 1)
    __bad_mask();
  __trans_tmp_2 - 1 + 1;
}
gpi_update_reg() { u32_encode_bits(); }

in case this is a UBSAN bug.

cc @kees

[nathanchance]

Another solution to this would be removing the __bad_mask if we cannot prove field is a compiletime constant:

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 4e035aca6f7e..47e56653b276 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -115,7 +115,8 @@ extern void __compiletime_error("bad bitfield mask")
 __bad_mask(void);
 static __always_inline u64 field_multiplier(u64 field)
 {
-       if ((field | (field - 1)) & ((field | (field - 1)) + 1))
+       if (__builtin_constant_p(field) &&
+           (field | (field - 1)) & ((field | (field - 1)) + 1))
                __bad_mask();
        return field & -field;
 }

I guess that kind of defeats the purpose of that check though. Regardless, I sent the first diff. Maybe the one above is to be considered if it comes up in the future.

https://lore.kernel.org/r/20210112191214.1264793-1-natechancellor@gmail.com

[nathanchance]

Accepted: https://git.kernel.org/vkoul/dmaengine/c/0a6d3038d914b51d6860f23ea2b508590e8340de

[nathanchance]

Merged into mainline: https://git.kernel.org/torvalds/c/0a6d3038d914