koverstreet / bcachefs

Other
643 stars 71 forks source link

Build failure of 32-bit kernel .config with clang-17: fs/bcachefs/replicas.c:36:52: error: builtin functions must be directly called (builds ok with gcc-13.2), kernel 6.7.0 #625

Open ernsteiswuerfel opened 6 months ago

ernsteiswuerfel commented 6 months ago

Get this on my Ryzen when I use clang 17.0.6 to build a kernel for my Thinkpad T60 (same config builds ok with gcc 13.2.1):

#  LLVM=1 LLVM_IAS=1 make
  CALL    scripts/checksyscalls.sh
  CC [M]  fs/bcachefs/replicas.o
fs/bcachefs/replicas.c:36:52: error: builtin functions must be directly called
   36 |         eytzinger0_sort(r->entries, r->nr, r->entry_size, memcmp, NULL);
      |                                                           ^
./arch/x86/include/asm/string_32.h:159:16: note: expanded from macro 'memcmp'
  159 | #define memcmp __builtin_memcmp
      |                ^
fs/bcachefs/replicas.c:836:9: error: builtin functions must be directly called
  836 |                       memcmp, NULL);
      |                       ^
./arch/x86/include/asm/string_32.h:159:16: note: expanded from macro 'memcmp'
  159 | #define memcmp __builtin_memcmp
      |                ^
2 errors generated.
make[4]: *** [scripts/Makefile.build:243: fs/bcachefs/replicas.o] Fehler 1
make[3]: *** [scripts/Makefile.build:480: fs/bcachefs] Fehler 2
make[2]: *** [scripts/Makefile.build:480: fs] Fehler 2
make[1]: *** [/usr/src/linux-stable/Makefile:1911: .] Fehler 2
make: *** [Makefile:234: __sub-make] Fehler 2

Kernel .config attached. config_670_p3.txt

immolo commented 6 months ago

I've tried to recreate this on my Gentoo LLVM17 system using:

make localyesconfig
make nconfig (add all bcachefs driver support)
LLVM=1 LLVM_TAS=1 make

However was unable to hit this bug, could please share some more system information and we can see if we can find the root cause of this one.

ernsteiswuerfel commented 6 months ago

@immolo The build system does not seem to matter much. Tried building my 32-bit .config from 1st post on another system (AMD FX-8370 based) where I get the same error with clang-17. However my v6.7.0 64-bit build for the FX-9370 itself builds just fine with clang-17.

So I tried building my 32-bit .config from above with 64BIT=Y set and in this case it also builds just fine.

It seems to hit this issue you need a 32-bit kernel .config. And the same .config builds ok when a 64-bit kernel build is done.

So far it seems this is an 32-bit only issue. Part of the error message ./arch/x86/include/asm/string_32.h:159:16: note: expanded from macro 'memcmp' also points in that direction.

Some data of my build system here attached anyhow. emerge_info-FX8370.txt

koverstreet commented 6 months ago

This is really a compiler bug, but there's a simple workaround - I'll have a fix up soon.

ernsteiswuerfel commented 6 months ago

If it is a compiler bug should I perhaps open a clang-17 issue on their github page and link here?

Edit; This already has been reported earlier as clang issue #1932

nathanchance commented 6 months ago

Sorry for not continuing to investigate that issue. For what it's worth, the workaround is needed in another place within that file:

diff --git a/fs/bcachefs/replicas.c b/fs/bcachefs/replicas.c
index 58536898b142..532f75054d0c 100644
--- a/fs/bcachefs/replicas.c
+++ b/fs/bcachefs/replicas.c
@@ -6,6 +6,12 @@
 #include "replicas.h"
 #include "super-io.h"

+/* Some (buggy!) compilers don't allow memcmp to be passed as a pointer */
+static int bch2_memcmp(const void *l, const void *r, size_t size)
+{
+       return memcmp(l, r, size);
+}
+
 static int bch2_cpu_replicas_to_sb_replicas(struct bch_fs *,
                                            struct bch_replicas_cpu *);

@@ -33,7 +39,7 @@ void bch2_replicas_entry_sort(struct bch_replicas_entry_v1 *e)

 static void bch2_cpu_replicas_sort(struct bch_replicas_cpu *r)
 {
-       eytzinger0_sort(r->entries, r->nr, r->entry_size, memcmp, NULL);
+       eytzinger0_sort(r->entries, r->nr, r->entry_size, bch2_memcmp, NULL);
 }

 static void bch2_replicas_entry_v0_to_text(struct printbuf *out,
@@ -812,12 +818,6 @@ static int bch2_cpu_replicas_to_sb_replicas(struct bch_fs *c,
        return 0;
 }

-/* Some (buggy!) compilers don't allow memcmp to be passed as a pointer */
-static int bch2_memcmp(const void *l, const void *r, size_t size)
-{
-       return memcmp(l, r, size);
-}
-
 static int bch2_cpu_replicas_validate(struct bch_replicas_cpu *cpu_r,
                                      struct bch_sb *sb,
                                      struct printbuf *err)

The LLVM change that appears to introduce this problem has been around for some time (https://github.com/llvm/llvm-project/commit/34866c7719f893c957d93f5918f760b6edebd6be) and I suspect this likely has to do with architectural differences between clang and GCC. This is really exacerbated by CONFIG_FORTIFY_SOURCE not working for ARCH=i386 at all (due to https://github.com/ClangBuiltLinux/linux/issues/1583, which I think we can now fix in the kernel sources), hence the #define memcmp __builtin_memcmp.

ernsteiswuerfel commented 6 months ago

I can confirm that both of your patches combined applied on v6.7.0 fix my 32-bit clang-17 build issues. 🥳 Thanks!