Skip to contentSkip to conversation

kbuild: clang: Support building UM with SUBARCH=i386

PATCH Superseded Kees Cook 3 participants 7 messages Mar 3, 2025 – Mar 6, 2025

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Kees Cook

author
#0 lore raw

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
...
Fixes c7500c1b53bf ("um: Allow builds with Clang")

---
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...

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
index 2435efae67f5..fa6f9a9be4ac 100644
@@ -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
#1 lore raw
Quoted 11 lines · Kees Cook

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
...
Quoted 7 lines
Fixes c7500c1b53bf ("um: Allow builds with Clang")

---
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...

This seems to work for me:

scripts/Makefile.clang +1 -0
index 2435efae67f5..8e349bf30fa8 100644
@@ -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
index 2435efae67f5..fa6f9a9be4ac 100644
@@ -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
#2 lore raw
Quoted 27 lines · Thomas Weißschuh
Quoted 11 lines · Kees Cook

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
...
Quoted 7 lines
Fixes c7500c1b53bf ("um: Allow builds with Clang")

---
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...

This seems to work for me:

scripts/Makefile.clang +0 -0
index 2435efae67f5..8e349bf30fa8 100644

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 missing CONFIG_FORTIFY_SOURCE on clang x86_32 and 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
index 2435efae67f5..fa6f9a9be4ac 100644
@@ -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
#3 lore raw
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 missing CONFIG_FORTIFY_SOURCE on clang x86_32 and 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]/

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>

author
#4 lore raw
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 missing CONFIG_FORTIFY_SOURCE on clang x86_32 and 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]/

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=n then 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): 4

Clang 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): 4

GCC 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): 4

Clang 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): 0

GCC 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

It 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
#5 lore raw
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 running
Quoted 2 lines

If I force CONFIG_CC_HAS_COUNTED_BY=n then 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): 4

Clang 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): 4

GCC 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): 4

Clang 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): 0

GCC 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 __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...

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

author
#6 lore raw
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 running
Quoted 2 lines

If I force CONFIG_CC_HAS_COUNTED_BY=n then 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): 4

Clang 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): 4

GCC 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): 4

Clang 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): 0

GCC 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 __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...

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