This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible.
For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html --- selftests/harness: order TEST_F and XFAIL_ADD constructors This commit fixes a constructor ordering race where XFAIL_ADD() could run before TEST_F() and read a NULL _##fixture_##test##_object pointer. The fix uses constructor priorities to ensure TEST_F runs first. > TEST_F() allocates and registers its struct __test_metadata via mmap() > inside its constructor, and only then assigns the > _##fixture_##test##_object pointer. > > XFAIL_ADD() runs in a constructor too and reads > _##fixture_##test##_object to initialize xfail->test. If XFAIL_ADD runs > first, xfail->test can be NULL and the expected failure will be reported > as FAIL. > > Use constructor priorities to ensure TEST_F registration runs before > XFAIL_ADD, without adding extra state or runtime lookups. > > Fixes: 2709473c9386 ("selftests: kselftest_harness: support using xfail") Should the Fixes: tag point to commit 24cf65a62266 instead? The existing tag points to 2709473c9386 ("selftests: kselftest_harness: support using xfail"), but that commit used static initialization (.test = &object) which had no race condition. The race was actually introduced by commit 24cf65a62266 ("selftests/harness: Share _metadata between forked processes"), which changed TEST_F to use constructor-based allocation and also changed XFAIL_ADD to read the pointer value in its constructor. This created the race where XFAIL_ADD could run before TEST_F and read NULL. > Signed-off-by: Sun Jian <[email protected]> > diff --git a/tools/testing/selftests/kselftest_harness.h > b/tools/testing/selftests/kselftest_harness.h > index 16a119a4656c..4afaef01c22e 100644 > --- a/tools/testing/selftests/kselftest_harness.h > +++ b/tools/testing/selftests/kselftest_harness.h > @@ -76,6 +76,9 @@ static inline void __kselftest_memset_safe(void *s, int c, > size_t n) > memset(s, c, n); > } > > +#define KSELFTEST_PRIO_TEST_F 20000 > +#define KSELFTEST_PRIO_XFAIL 20001 > + > #define TEST_TIMEOUT_DEFAULT 30 > > /* Utilities exposed to the test definitions */ > @@ -465,7 +468,7 @@ static inline void __kselftest_memset_safe(void *s, int > c, size_t n) > fixture_name##_teardown(_metadata, self, variant); \ > } \ > static struct __test_metadata *_##fixture_name##_##test_name##_object; \ > - static void __attribute__((constructor)) \ > + static void __attribute__((constructor(KSELFTEST_PRIO_TEST_F))) \ > _register_##fixture_name##_##test_name(void) \ > { \ > struct __test_metadata *object = mmap(NULL, sizeof(*object), \ > @@ -880,7 +883,7 @@ struct __test_xfail { > .fixture = &_##fixture_name##_fixture_object, \ > .variant = &_##fixture_name##_##variant_name##_object, \ > }; \ > - static void __attribute__((constructor)) \ > + static void __attribute__((constructor(KSELFTEST_PRIO_XFAIL))) \ > > _register_##fixture_name##_##variant_name##_##test_name##_xfail(void) \ > { \ > _##fixture_name##_##variant_name##_##test_name##_xfail.test = \
