Hi Nicolin,
On Mon, Jun 09, 2025 at 11:40:34PM -0700, Nicolin Chen wrote:
Hi Thomas,
CC += Jason
On Mon, May 05, 2025 at 05:15:30PM +0200, Thomas Weißschuh wrote:
Usage of longjmp() was added to ensure that teardown is always run in commit 63e6b2a42342 ("selftests/harness: Run TEARDOWN for ASSERT failures") However instead of calling longjmp() to the teardown handler it is easier to just call the teardown handler directly from __bail(). Any potential duplicate teardown invocations are harmless as the actual handler will only ever be executed once since commit fff37bd32c76 ("selftests/harness: Fix fixture teardown").
Additionally this removes a incompatibility with nolibc, which does not support setjmp()/longjmp().
Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de
The iommufd selftest (CONFIG_IOMMUFD_TEST) starts to give warnings when building with v6.16-rc1, though the test code wasn't changed at these two functions:
Thanks for the report.
make: Entering directory '/nicolinc/linux-stable/tools/testing/selftests/iommu' CC iommufd iommufd.c: In function ‘wrapper_iommufd_mock_domain_all_aligns’: iommufd.c:1806:17: warning: ‘mfd’ may be used uninitialized in this function [-Wmaybe-uninitialized] 1806 | close(mfd); | ^~~~~~~~~~ iommufd.c:1766:13: note: ‘mfd’ was declared here 1766 | int mfd; | ^~~ iommufd.c: In function ‘wrapper_iommufd_mock_domain_all_aligns_copy’: iommufd.c:1869:17: warning: ‘mfd’ may be used uninitialized in this function [-Wmaybe-uninitialized] 1869 | close(mfd); | ^~~~~~~~~~ iommufd.c:1818:13: note: ‘mfd’ was declared here 1818 | int mfd; | ^~~ CC iommufd_fail_nth make: Leaving directory '/nicolinc/linux-stable/tools/testing/selftests/iommu'
Git bisect points to this patch, and reverting it fixes these.
Both mfds are under the same "if (variant->driver)" check, so the warnings don't seem legit to me.
Do you have any idea why this happens?
It does look very weird. I could understand if the compiler assumes that variant->file changes during the runtime of the function. But even if I work around this, by introducing a local variable "bool file = variant->file" the issue persists. However as soon as the value of of "bool file" is fixed to either "true" or "false" it goes away.
The following diff *alone* also prevents the warning, but that doesn't make any sense either:
--- a/tools/testing/selftests/iommu/iommufd_utils.h +++ b/tools/testing/selftests/iommu/iommufd_utils.h @@ -65,7 +65,7 @@ static inline void *memfd_mmap(size_t length, int prot, int flags, int *mfd_p) return MAP_FAILED; if (ftruncate(mfd, length)) return MAP_FAILED; - *mfd_p = mfd; + *mfd_p = 0; return mmap(0, length, prot, flags, mfd, 0); }
Maybe the logic became too complex for GCC? Case in point, when trying with an older GCC 13.2, the following warning appeared:
In file included from iommufd_utils.h:14, from iommufd.c:12: In function 'iommufd_viommu_vdevice_alloc', inlined from 'wrapper_iommufd_viommu_vdevice_alloc' at iommufd.c:2731:1: ../kselftest_harness.h:760:12: warning: 'ret' may be used uninitialized [-Wmaybe-uninitialized] 760 | if (!(__exp _t __seen)) { \ | ^ ../kselftest_harness.h:513:9: note: in expansion of macro '__EXPECT' 513 | __EXPECT(expected, #expected, seen, #seen, ==, 1) | ^~~~~~~~ iommufd_utils.h:1005:9: note: in expansion of macro 'ASSERT_EQ' 1005 | ASSERT_EQ(0, _test_cmd_trigger_vevents(self->fd, dev_id, nvevents)) | ^~~~~~~~~ iommufd.c:2766:17: note: in expansion of macro 'test_cmd_trigger_vevents' 2766 | test_cmd_trigger_vevents(dev_id, 3); | ^~~~~~~~~~~~~~~~~~~~~~~~ iommufd_utils.h: In function 'wrapper_iommufd_viommu_vdevice_alloc': iommufd_utils.h:993:13: note: 'ret' was declared here 993 | int ret; | ^~~
Which is also just a false-positive and fixed with newer GCCs.
Thomas