On Tue, Mar 04, 2025 at 09:07:57AM -0800, Kees Cook wrote: > On Tue, Mar 04, 2025 at 03:51:19PM +0100, Thomas Weißschuh wrote: > > 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. > > I wasn't seeing with Clang 20 from git: > ClangBuiltLinux clang version 20.0.0git > (g...@github.com:llvm/llvm-project.git > 72901fe19eb1e55d0ee1c380ab7a9f57d2f187c5) > > But I do see the error with ToT Clang: > ClangBuiltLinux clang version 21.0.0git > (g...@github.com: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...
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
