kbuild: clang: Support building UM with SUBARCH=i386
Subsystems (2)
PATCH 1/1 kbuild: clang: Support building UM with SUBARCH=i386
The UM builds distinguish i386 from x86_64 via SUBARCH, but we don't support building i386 directly with Clang. To make SUBARCH work for i386 UM, we need to explicitly test for it.
This lets me run i386 KUnit tests with Clang:
$ ./tools/testing/kunit/kunit.py run \ --make_options LLVM=1 \ --make_options SUBARCH=i386 ...
---
I could not find a cleaner way to do this without hardcoding a test for i386 UM. Does anyone see a more sane way to accomplish this? The comment above the CLANG_TARGET_FLAGS seems like it can't be done with UM's Makefile...
--- scripts/Makefile.clang | 4 ++++ 1 file changed, 4 insertions(+)
scripts/Makefile.clang +4 -0
@@ -12,8 +12,12 @@ CLANG_TARGET_FLAGS_riscv := riscv64-linux-gnu CLANG_TARGET_FLAGS_s390 := s390x-linux-gnu CLANG_TARGET_FLAGS_sparc := sparc64-linux-gnu CLANG_TARGET_FLAGS_x86 := x86_64-linux-gnu +ifeq ($(SRCARCH):$(SUBARCH),um:i386) +CLANG_TARGET_FLAGS := i386-linux-gnu +else CLANG_TARGET_FLAGS_um := $(CLANG_TARGET_FLAGS_$(SUBARCH)) CLANG_TARGET_FLAGS := $(CLANG_TARGET_FLAGS_$(SRCARCH)) +endif ifeq ($(CLANG_TARGET_FLAGS),) $(error add '--target=' option to scripts/Makefile.clang)
Signature · 1 line
2.34.1
Quoted 11 lines · Kees Cook
The UM builds distinguish i386 from
x86_64via SUBARCH, but we don't support building i386 directly with Clang. To make SUBARCH work for i386 UM, we need to explicitly test for it.This lets me run i386 KUnit tests with Clang:
$ ./tools/testing/kunit/kunit.py run \ --make_options LLVM=1 \ --make_options SUBARCH=i386 ...
Quoted 7 lines
Fixes c7500c1b53bf ("um: Allow builds with Clang")Signed-off-by Kees Cook <[email protected]>---
I could not find a cleaner way to do this without hardcoding a test for i386 UM. Does anyone see a more sane way to accomplish this? The comment above theCLANG_TARGET_FLAGSseems like it can't be done with UM'sMakefile...
This seems to work for me:
scripts/Makefile.clang +1 -0
@@ -12,6 +12,7 @@ CLANG_TARGET_FLAGS_riscv := riscv64-linux-gnu CLANG_TARGET_FLAGS_s390 := s390x-linux-gnu CLANG_TARGET_FLAGS_sparc := sparc64-linux-gnu CLANG_TARGET_FLAGS_x86 := x86_64-linux-gnu +CLANG_TARGET_FLAGS_i386 := i386-linux-gnu CLANG_TARGET_FLAGS_um := $(CLANG_TARGET_FLAGS_$(SUBARCH)) CLANG_TARGET_FLAGS := $(CLANG_TARGET_FLAGS_$(SRCARCH))
This is also what exists in tools/testing/selftests/lib.mk. Minus the missing CONFIG_FORTIFY_SOURCE on clang x86_32 and a failure of overflow.DEFINE_FLEX_test (clang 19.1.7).
Quoted 32 lines
Cc Nathan Chancellor <[email protected]>Cc Nick Desaulniers <[email protected]>Cc Bill Wendling <[email protected]>Cc Justin Stitt <[email protected]>Cc Masahiro Yamada <[email protected]>Cc Nicolas Schier <[email protected]>--- scripts/Makefile.clang | 4 ++++ 1 file changed, 4 insertions(+)scripts/Makefile.clang +4
-0@@ -12,8 +12,12 @@ CLANG_TARGET_FLAGS_riscv := riscv64-linux-gnu CLANG_TARGET_FLAGS_s390 := s390x-linux-gnu CLANG_TARGET_FLAGS_sparc := sparc64-linux-gnu CLANG_TARGET_FLAGS_x86 := x86_64-linux-gnu +ifeq ($(SRCARCH):$(SUBARCH),um:i386) +CLANG_TARGET_FLAGS := i386-linux-gnu +else CLANG_TARGET_FLAGS_um := $(CLANG_TARGET_FLAGS_$(SUBARCH)) CLANG_TARGET_FLAGS := $(CLANG_TARGET_FLAGS_$(SRCARCH)) +endif ifeq ($(CLANG_TARGET_FLAGS),) $(error add '--target=' option to scripts/Makefile.clang)Signature · 1 line
2.34.1
Nathan Chancellor
maintainerQuoted 27 lines · Thomas Weißschuh
Quoted 11 lines · Kees Cook
The UM builds distinguish i386 from
x86_64via SUBARCH, but we don't support building i386 directly with Clang. To make SUBARCH work for i386 UM, we need to explicitly test for it.This lets me run i386 KUnit tests with Clang:
$ ./tools/testing/kunit/kunit.py run \ --make_options LLVM=1 \ --make_options SUBARCH=i386 ...Quoted 7 lines
Fixes c7500c1b53bf ("um: Allow builds with Clang")Signed-off-by Kees Cook <[email protected]>---
I could not find a cleaner way to do this without hardcoding a test for i386 UM. Does anyone see a more sane way to accomplish this? The comment above theCLANG_TARGET_FLAGSseems like it can't be done with UM'sMakefile...This seems to work for me:
scripts/Makefile.clang +0
-0
scripts/Makefile.clang CLANG_TARGET_FLAGS_riscv := riscv64-linux-gnu · line 17
Quoted 6 lines
scripts/Makefile.clang +1
-0@@ -12,6 +12,7 @@ CLANG_TARGET_FLAGS_riscv := riscv64-linux-gnu CLANG_TARGET_FLAGS_s390 := s390x-linux-gnu CLANG_TARGET_FLAGS_sparc := sparc64-linux-gnu CLANG_TARGET_FLAGS_x86 := x86_64-linux-gnu +CLANG_TARGET_FLAGS_i386 := i386-linux-gnu CLANG_TARGET_FLAGS_um := $(CLANG_TARGET_FLAGS_$(SUBARCH)) CLANG_TARGET_FLAGS := $(CLANG_TARGET_FLAGS_$(SRCARCH))
Yeah, I think I prefer this. As the comment at the top of this file notes, we normally want '-m32' and '-m64' to control the word size, which happens for regular x86 but not UML. Since UML is already weird here, I think going this route for simplicity rather than consistency is not that big of a deal. I would probably add a comment it is only there for ARCH=um SUBARCH=i386 though just so we do not accidentally remove it.
Quoted 3 lines
This is also what exists in
tools/testing/selftests/lib.mk. Minus the missingCONFIG_FORTIFY_SOURCEon clangx86_32and a failure of overflow.DEFINE_FLEX_test (clang 19.1.7).
Does Kees's other patch resolve the second issue? It'll obviously fix the first :P
https://lore.kernel.org/[email protected]/
Quoted 33 lines
Quoted 32 lines
Cc Nathan Chancellor <[email protected]>Cc Nick Desaulniers <[email protected]>Cc Bill Wendling <[email protected]>Cc Justin Stitt <[email protected]>Cc Masahiro Yamada <[email protected]>Cc Nicolas Schier <[email protected]>--- scripts/Makefile.clang | 4 ++++ 1 file changed, 4 insertions(+)scripts/Makefile.clang +4
-0@@ -12,8 +12,12 @@ CLANG_TARGET_FLAGS_riscv := riscv64-linux-gnu CLANG_TARGET_FLAGS_s390 := s390x-linux-gnu CLANG_TARGET_FLAGS_sparc := sparc64-linux-gnu CLANG_TARGET_FLAGS_x86 := x86_64-linux-gnu +ifeq ($(SRCARCH):$(SUBARCH),um:i386) +CLANG_TARGET_FLAGS := i386-linux-gnu +else CLANG_TARGET_FLAGS_um := $(CLANG_TARGET_FLAGS_$(SUBARCH)) CLANG_TARGET_FLAGS := $(CLANG_TARGET_FLAGS_$(SRCARCH)) +endif ifeq ($(CLANG_TARGET_FLAGS),) $(error add '--target=' option to scripts/Makefile.clang)Signature · 1 line
2.34.1
Quoted 2 lines · Nathan Chancellor
Quoted 1 line · Thomas Weißschuh
On 2025-03-03 13:52:41-0800, Kees Cook wrote:
<snip>
Quoted 8 lines
Quoted 3 lines
This is also what exists in
tools/testing/selftests/lib.mk. Minus the missingCONFIG_FORTIFY_SOURCEon clangx86_32and a failure of overflow.DEFINE_FLEX_test (clang 19.1.7).Does Kees's other patch resolve the second issue? It'll obviously fix the first :P
No, it doesn't.
Running tests with:
$ .kunit/linux kunit.filter_glob=overflow.DEFINE_FLEX_test kunit.enable=1 mem=1G console=tty kunit_shutdown=halt [15:48:30] =================== overflow (1 subtest) =================== [15:48:30] # DEFINE_FLEX_test: EXPECTATION FAILED at lib/overflow_kunit.c:1200 [15:48:30] Expected __builtin_dynamic_object_size(two_but_zero, 0) == expected_raw_size, but [15:48:30] __builtin_dynamic_object_size(two_but_zero, 0) == 12 (0xc) [15:48:30] expected_raw_size == 8 (0x8) [15:48:30] [FAILED] DEFINE_FLEX_test [15:48:30] # module: overflow_kunit [15:48:30] ==================== [FAILED] overflow ===================== [15:48:30] ============================================================ [15:48:30] Testing complete. Ran 1 tests: failed: 1 [15:48:31] Elapsed time: 43.985s total, 0.001s configuring, 43.818s building, 0.133s running
If I force CONFIG_CC_HAS_COUNTED_BY=n then the test succeeds. Clang 19.1.7 from the Arch Linux repos.
<snip>
Quoted 31 lines · Thomas Weißschuh
Quoted 2 lines · Nathan Chancellor
Quoted 1 line · Thomas Weißschuh
On 2025-03-03 13:52:41-0800, Kees Cook wrote:
<snip>
Quoted 8 lines
Quoted 3 lines
This is also what exists in
tools/testing/selftests/lib.mk. Minus the missingCONFIG_FORTIFY_SOURCEon clangx86_32and a failure of overflow.DEFINE_FLEX_test (clang 19.1.7).Does Kees's other patch resolve the second issue? It'll obviously fix the first :P
No, it doesn't.
Running tests with:
$ .kunit/linux kunit.filter_glob=overflow.DEFINE_FLEX_test kunit.enable=1 mem=1G console=tty kunit_shutdown=halt [15:48:30] =================== overflow (1 subtest) =================== [15:48:30] # DEFINE_FLEX_test: EXPECTATION FAILED at lib/overflow_kunit.c:1200 [15:48:30] Expected __builtin_dynamic_object_size(two_but_zero, 0) == expected_raw_size, but [15:48:30] __builtin_dynamic_object_size(two_but_zero, 0) == 12 (0xc) [15:48:30] expected_raw_size == 8 (0x8) [15:48:30] [FAILED] DEFINE_FLEX_test [15:48:30] # module: overflow_kunit [15:48:30] ==================== [FAILED] overflow ===================== [15:48:30] ============================================================ [15:48:30] Testing complete. Ran 1 tests: failed: 1 [15:48:31] Elapsed time: 43.985s total, 0.001s configuring, 43.818s building, 0.133s running
Quoted 2 lines
If I force
CONFIG_CC_HAS_COUNTED_BY=nthen the test succeeds. Clang 19.1.7 from the Arch Linux repos.
I wasn't seeing with Clang 20 from git:
ClangBuiltLinux clang version 20.0.0git ([email protected]:llvm/llvm-project.git 72901fe19eb1e55d0ee1c380ab7a9f57d2f187c5)
But I do see the error with ToT Clang:
ClangBuiltLinux clang version 21.0.0git ([email protected]:llvm/llvm-project.git eee3db5421040cfc3eae6e92ed714650a6f741fa)
Clang 17.1: (does not support counted_by)
# DEFINE_FLEX_test: missing counted_by
# DEFINE_FLEX_test: sizeof(two_but_zero): 8
# DEFINE_FLEX_test: __struct_size(two_but_zero): 12
# DEFINE_FLEX_test: __member_size(two_but_zero): 12
# DEFINE_FLEX_test: __member_size(two_but_zero->array): 4Clang 19.1.1: (actually is _does_ support counted_by, but Linux disables it)
# DEFINE_FLEX_test: missing counted_by
# DEFINE_FLEX_test: sizeof(two_but_zero): 8
# DEFINE_FLEX_test: __struct_size(two_but_zero): 12
# DEFINE_FLEX_test: __member_size(two_but_zero): 12
# DEFINE_FLEX_test: __member_size(two_but_zero->array): 4GCC 13.3:
# DEFINE_FLEX_test: missing counted_by
# DEFINE_FLEX_test: sizeof(two_but_zero): 8
# DEFINE_FLEX_test: __struct_size(two_but_zero): 12
# DEFINE_FLEX_test: __member_size(two_but_zero): 12
# DEFINE_FLEX_test: __member_size(two_but_zero->array): 4Clang 21 (ToT):
# DEFINE_FLEX_test: has counted_by
# DEFINE_FLEX_test: sizeof(two_but_zero): 8
# DEFINE_FLEX_test: __struct_size(two_but_zero): 12
# DEFINE_FLEX_test: __member_size(two_but_zero): 12
# DEFINE_FLEX_test: __member_size(two_but_zero->array): 0GCC 15 (ToT):
# DEFINE_FLEX_test: has counted_by
# DEFINE_FLEX_test: sizeof(two_but_zero): 8
# DEFINE_FLEX_test: __struct_size(two_but_zero): 12
# DEFINE_FLEX_test: __member_size(two_but_zero): 12
# DEFINE_FLEX_test: __member_size(two_but_zero->array): 0It seems like the on-stack sizes with __bdos all agree now, regardless of the used compiler features. It is only the array size calculation that now gets masked by counted_by. (i.e. the stack size is overridden by the zero "count" for the array elements.)
I'll send a fix for the test...
Signature · 1 line
Kees Cook
Nathan Chancellor
maintainerQuoted 66 lines · Kees Cook
Quoted 16 lines · Thomas Weißschuh
No, it doesn't.
Running tests with:
$ .kunit/linux kunit.filter_glob=overflow.DEFINE_FLEX_test kunit.enable=1 mem=1G console=tty kunit_shutdown=halt [15:48:30] =================== overflow (1 subtest) =================== [15:48:30] # DEFINE_FLEX_test: EXPECTATION FAILED at lib/overflow_kunit.c:1200 [15:48:30] Expected __builtin_dynamic_object_size(two_but_zero, 0) == expected_raw_size, but [15:48:30] __builtin_dynamic_object_size(two_but_zero, 0) == 12 (0xc) [15:48:30] expected_raw_size == 8 (0x8) [15:48:30] [FAILED] DEFINE_FLEX_test [15:48:30] # module: overflow_kunit [15:48:30] ==================== [FAILED] overflow ===================== [15:48:30] ============================================================ [15:48:30] Testing complete. Ran 1 tests: failed: 1 [15:48:31] Elapsed time: 43.985s total, 0.001s configuring, 43.818s building, 0.133s runningQuoted 2 lines
If I force
CONFIG_CC_HAS_COUNTED_BY=nthen the test succeeds. Clang 19.1.7 from the Arch Linux repos.I wasn't seeing with Clang 20 from git:
ClangBuiltLinux clang version 20.0.0git ([email protected]:llvm/llvm-project.git72901fe19eb1e55d0ee1c380ab7a9f57d2f187c5)But I do see the error with ToT Clang:
ClangBuiltLinux clang version 21.0.0git ([email protected]:llvm/llvm-project.giteee3db5421040cfc3eae6e92ed714650a6f741fa)Clang 17.1: (does not support
counted_by)# DEFINE_FLEX_test: missing counted_by # DEFINE_FLEX_test: sizeof(two_but_zero): 8 # DEFINE_FLEX_test: __struct_size(two_but_zero): 12 # DEFINE_FLEX_test: __member_size(two_but_zero): 12 # DEFINE_FLEX_test: __member_size(two_but_zero->array): 4Clang 19.1.1: (actually is _does_ support
counted_by, but Linux disables it)# DEFINE_FLEX_test: missing counted_by # DEFINE_FLEX_test: sizeof(two_but_zero): 8 # DEFINE_FLEX_test: __struct_size(two_but_zero): 12 # DEFINE_FLEX_test: __member_size(two_but_zero): 12 # DEFINE_FLEX_test: __member_size(two_but_zero->array): 4GCC 13.3:
# DEFINE_FLEX_test: missing counted_by # DEFINE_FLEX_test: sizeof(two_but_zero): 8 # DEFINE_FLEX_test: __struct_size(two_but_zero): 12 # DEFINE_FLEX_test: __member_size(two_but_zero): 12 # DEFINE_FLEX_test: __member_size(two_but_zero->array): 4Clang 21 (ToT):
# DEFINE_FLEX_test: has counted_by # DEFINE_FLEX_test: sizeof(two_but_zero): 8 # DEFINE_FLEX_test: __struct_size(two_but_zero): 12 # DEFINE_FLEX_test: __member_size(two_but_zero): 12 # DEFINE_FLEX_test: __member_size(two_but_zero->array): 0GCC 15 (ToT):
# DEFINE_FLEX_test: has counted_by # DEFINE_FLEX_test: sizeof(two_but_zero): 8 # DEFINE_FLEX_test: __struct_size(two_but_zero): 12 # DEFINE_FLEX_test: __member_size(two_but_zero): 12 # DEFINE_FLEX_test: __member_size(two_but_zero->array): 0
Quoted 6 lines
It seems like the on-stack sizes with
__bdosall agree now, regardless of the used compiler features. It is only the array size calculation that now gets masked bycounted_by. (i.e. the stack size is overridden by the zero "count" for the array elements.)I'll send a fix for the test...
Just for my own understanding, is this because of the adjustment that Bill did to the __bdos() calculation in [1]? I think that tracks because the version of LLVM 20 that you have is pretty old and does not have that change. I know for a fact I tested the original change to the overflow KUnit test to adjust the expected calculation result and it passed but it was before that change as well. If I use a current version of LLVM 20, I see the failure. If I allow LLVM 18 to use __counted_by(), the test passes with it. Not that it truly matters but it does explain how we got to this point.
[1]: https://github.com/llvm/llvm-project/commit/8c62bf54df76e37d0978f4901c6be6554e978b53
Cheers,
Nathan
Quoted 74 lines · Nathan Chancellor
Quoted 66 lines · Kees Cook
Quoted 16 lines · Thomas Weißschuh
No, it doesn't.
Running tests with:
$ .kunit/linux kunit.filter_glob=overflow.DEFINE_FLEX_test kunit.enable=1 mem=1G console=tty kunit_shutdown=halt [15:48:30] =================== overflow (1 subtest) =================== [15:48:30] # DEFINE_FLEX_test: EXPECTATION FAILED at lib/overflow_kunit.c:1200 [15:48:30] Expected __builtin_dynamic_object_size(two_but_zero, 0) == expected_raw_size, but [15:48:30] __builtin_dynamic_object_size(two_but_zero, 0) == 12 (0xc) [15:48:30] expected_raw_size == 8 (0x8) [15:48:30] [FAILED] DEFINE_FLEX_test [15:48:30] # module: overflow_kunit [15:48:30] ==================== [FAILED] overflow ===================== [15:48:30] ============================================================ [15:48:30] Testing complete. Ran 1 tests: failed: 1 [15:48:31] Elapsed time: 43.985s total, 0.001s configuring, 43.818s building, 0.133s runningQuoted 2 lines
If I force
CONFIG_CC_HAS_COUNTED_BY=nthen the test succeeds. Clang 19.1.7 from the Arch Linux repos.I wasn't seeing with Clang 20 from git:
ClangBuiltLinux clang version 20.0.0git ([email protected]:llvm/llvm-project.git72901fe19eb1e55d0ee1c380ab7a9f57d2f187c5)But I do see the error with ToT Clang:
ClangBuiltLinux clang version 21.0.0git ([email protected]:llvm/llvm-project.giteee3db5421040cfc3eae6e92ed714650a6f741fa)Clang 17.1: (does not support
counted_by)# DEFINE_FLEX_test: missing counted_by # DEFINE_FLEX_test: sizeof(two_but_zero): 8 # DEFINE_FLEX_test: __struct_size(two_but_zero): 12 # DEFINE_FLEX_test: __member_size(two_but_zero): 12 # DEFINE_FLEX_test: __member_size(two_but_zero->array): 4Clang 19.1.1: (actually is _does_ support
counted_by, but Linux disables it)# DEFINE_FLEX_test: missing counted_by # DEFINE_FLEX_test: sizeof(two_but_zero): 8 # DEFINE_FLEX_test: __struct_size(two_but_zero): 12 # DEFINE_FLEX_test: __member_size(two_but_zero): 12 # DEFINE_FLEX_test: __member_size(two_but_zero->array): 4GCC 13.3:
# DEFINE_FLEX_test: missing counted_by # DEFINE_FLEX_test: sizeof(two_but_zero): 8 # DEFINE_FLEX_test: __struct_size(two_but_zero): 12 # DEFINE_FLEX_test: __member_size(two_but_zero): 12 # DEFINE_FLEX_test: __member_size(two_but_zero->array): 4Clang 21 (ToT):
# DEFINE_FLEX_test: has counted_by # DEFINE_FLEX_test: sizeof(two_but_zero): 8 # DEFINE_FLEX_test: __struct_size(two_but_zero): 12 # DEFINE_FLEX_test: __member_size(two_but_zero): 12 # DEFINE_FLEX_test: __member_size(two_but_zero->array): 0GCC 15 (ToT):
# DEFINE_FLEX_test: has counted_by # DEFINE_FLEX_test: sizeof(two_but_zero): 8 # DEFINE_FLEX_test: __struct_size(two_but_zero): 12 # DEFINE_FLEX_test: __member_size(two_but_zero): 12 # DEFINE_FLEX_test: __member_size(two_but_zero->array): 0Quoted 6 lines
It seems like the on-stack sizes with
__bdosall agree now, regardless of the used compiler features. It is only the array size calculation that now gets masked bycounted_by. (i.e. the stack size is overridden by the zero "count" for the array elements.)I'll send a fix for the test...
Quoted 9 lines
Just for my own understanding, is this because of the adjustment that Bill did to the
__bdos()calculation in [1]? I think that tracks because the version of LLVM 20 that you have is pretty old and does not have that change. I know for a fact I tested the original change to the overflow KUnit test to adjust the expected calculation result and it passed but it was before that change as well. If I use a current version of LLVM 20, I see the failure. If I allow LLVM 18 to use__counted_by(), the test passes with it. Not that it truly matters but it does explain how we got to this point.
Yes, totally! This is exactly how I got there too. Great; thank you for summarizing! :)
Signature · 1 line
Kees Cook