[PATCH] selftests/nolibc: only run constructor tests on nolibc

[PATCH] selftests/nolibc: only run constructor tests on nolibc

[Thomas Weißschuh]

The nolibc testsuite can be run against other libcs to test for
interoperability. Some aspects of the constructor execution are not
standardized and musl does not provide all tested feature, for one it
does not provide arguments to the constructors, anymore?

Skip the constructor tests on non-nolibc configurations.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/testing/selftests/nolibc/nolibc-test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 798fbdcd3ff8c36b514feb3fa1c7b8d7701cccd7..94db506eca906ff0ce8f518298dee34abf386484 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -738,9 +738,9 @@ int run_startup(int min, int max)
 		CASE_TEST(environ_HOME);     EXPECT_PTRNZ(1, getenv("HOME")); break;
 		CASE_TEST(auxv_addr);        EXPECT_PTRGT(test_auxv != (void *)-1, test_auxv, brk); break;
 		CASE_TEST(auxv_AT_UID);      EXPECT_EQ(1, getauxval(AT_UID), getuid()); break;
-		CASE_TEST(constructor);      EXPECT_EQ(1, constructor_test_value, 2); break;
+		CASE_TEST(constructor);      EXPECT_EQ(is_nolibc, constructor_test_value, 2); break;
 		CASE_TEST(linkage_errno);    EXPECT_PTREQ(1, linkage_test_errno_addr(), &errno); break;
-		CASE_TEST(linkage_constr);   EXPECT_EQ(1, linkage_test_constructor_test_value, 6); break;
+		CASE_TEST(linkage_constr);   EXPECT_EQ(is_nolibc, linkage_test_constructor_test_value, 6); break;
 		case __LINE__:
 			return ret; /* must be last */
 		/* note: do not set any defaults so as to permit holes above */

---
base-commit: 16681bea9a80080765c98b545ad74c17de2d513c
change-id: 20250212-nolibc-test-constructor-42491ba71a19

Best regards,
-- 
Thomas Weißschuh <linux@weissschuh.net>

Re: [PATCH] selftests/nolibc: only run constructor tests on nolibc

[Willy Tarreau]

On Wed, Feb 12, 2025 at 07:01:01PM +0100, Thomas Weißschuh wrote:
> The nolibc testsuite can be run against other libcs to test for
> interoperability. Some aspects of the constructor execution are not
> standardized and musl does not provide all tested feature, for one it
> does not provide arguments to the constructors, anymore?
>
> Skip the constructor tests on non-nolibc configurations.

I'm not much surprised, I've always avoided arguments in my use of
constructors due to a lack of portability. However the patch disables
all constructors tests, while I'm seeing that the linkage_test version
does not make use of arguments, though there is an implied expectation
that they're executed in declaration order, which is not granted.

I'm wondering if we shouldn't make the tests more robust:
  1) explicitly set linkage_test_constructor_test_value to zero in the
     declaration, because here it's not set so we have no guarantee
     (we're not in the kernel)
  2) only add values to check for cumulated values (e.g. |1 in const1,
     |2 in const2) and verify that the result is properly 3
  3) make the argument test add a distinct value (|4) so that when
     testing it's instantly obvious which test was not called.

And indeed, we can disable the tests we know fail on other libcs and
even split that by feature (e.g. test that at least one constructor
was called using !=0, that all non-arg ones were called via &3 == 3,
and that the args were passed via &4==4). That would allow to further
refine the tests if desired so that we can keep the differences in mind.

In any case all of this can also be done later, and I'm obviously fine
with this immediate adjustement.

> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>

Acked-by: Willy Tarreau <w@1wt.eu>

Willy

Re: [PATCH] selftests/nolibc: only run constructor tests on nolibc

[Thomas Weißschuh]

On 2025-02-16 10:39:40+0100, Willy Tarreau wrote:
> On Wed, Feb 12, 2025 at 07:01:01PM +0100, Thomas Weißschuh wrote:
> > The nolibc testsuite can be run against other libcs to test for
> > interoperability. Some aspects of the constructor execution are not
> > standardized and musl does not provide all tested feature, for one it
> > does not provide arguments to the constructors, anymore?
> >
> > Skip the constructor tests on non-nolibc configurations.
> 
> I'm not much surprised, I've always avoided arguments in my use of
> constructors due to a lack of portability. However the patch disables
> all constructors tests, while I'm seeing that the linkage_test version
> does not make use of arguments, though there is an implied expectation
> that they're executed in declaration order, which is not granted.

The tests are written specifically to test for execution order.
While we can not rely on the order for other libcs, the idea was to
expect a given order for the nolibc implementation.

> I'm wondering if we shouldn't make the tests more robust:
>   1) explicitly set linkage_test_constructor_test_value to zero in the
>      declaration, because here it's not set so we have no guarantee
>      (we're not in the kernel)

Ack.

>   2) only add values to check for cumulated values (e.g. |1 in const1,
>      |2 in const2) and verify that the result is properly 3

This would stop validating the order.

>   3) make the argument test add a distinct value (|4) so that when
>      testing it's instantly obvious which test was not called.
> 
> And indeed, we can disable the tests we know fail on other libcs and
> even split that by feature (e.g. test that at least one constructor
> was called using !=0, that all non-arg ones were called via &3 == 3,
> and that the args were passed via &4==4). That would allow to further
> refine the tests if desired so that we can keep the differences in mind.

I'm not yet convinced about the additional value.
But I'll give it some thought.

> In any case all of this can also be done later, and I'm obviously fine
> with this immediate adjustement.
> 
> > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> 
> Acked-by: Willy Tarreau <w@1wt.eu>

Thanks!

> Willy

Re: [PATCH] selftests/nolibc: only run constructor tests on nolibc

[Willy Tarreau]

Hi Thomas,

On Mon, Feb 17, 2025 at 10:24:11PM +0100, Thomas Weißschuh wrote:
> On 2025-02-16 10:39:40+0100, Willy Tarreau wrote:
> > On Wed, Feb 12, 2025 at 07:01:01PM +0100, Thomas Weißschuh wrote:
> > > The nolibc testsuite can be run against other libcs to test for
> > > interoperability. Some aspects of the constructor execution are not
> > > standardized and musl does not provide all tested feature, for one it
> > > does not provide arguments to the constructors, anymore?
> > >
> > > Skip the constructor tests on non-nolibc configurations.
> > 
> > I'm not much surprised, I've always avoided arguments in my use of
> > constructors due to a lack of portability. However the patch disables
> > all constructors tests, while I'm seeing that the linkage_test version
> > does not make use of arguments, though there is an implied expectation
> > that they're executed in declaration order, which is not granted.
> 
> The tests are written specifically to test for execution order.
> While we can not rely on the order for other libcs, the idea was to
> expect a given order for the nolibc implementation.

OK.

> > I'm wondering if we shouldn't make the tests more robust:
> >   1) explicitly set linkage_test_constructor_test_value to zero in the
> >      declaration, because here it's not set so we have no guarantee
> >      (we're not in the kernel)
> 
> Ack.
> 
> >   2) only add values to check for cumulated values (e.g. |1 in const1,
> >      |2 in const2) and verify that the result is properly 3
> 
> This would stop validating the order.

That was my purpose but OK I got it. Then there's another option which
preserves the order and even gives history:

  __attribute__((constructor))
  static void constructor1(void)
  {
      constructor_test_value = constructor_test_value * 0x10 + 1;
  }

  __attribute__((constructor))
  static void constructor2(void)
  {
      constructor_test_value = constructor_test_value * 0x10 + 2;
  }
 
Then if executed in the right order, you'll find 0x12. If both
are executed in any order, it will always be >= 0x10. If only one
is executed, it will be < 0x10, and if none is executed, it's 0.

Willy

Re: [PATCH] selftests/nolibc: only run constructor tests on nolibc

[Thomas Weißschuh]

Hi Willy,

On 2025-02-22 10:38:51+0100, Willy Tarreau wrote:
> On Mon, Feb 17, 2025 at 10:24:11PM +0100, Thomas Weißschuh wrote:
> > On 2025-02-16 10:39:40+0100, Willy Tarreau wrote:
> > > On Wed, Feb 12, 2025 at 07:01:01PM +0100, Thomas Weißschuh wrote:
> > > > The nolibc testsuite can be run against other libcs to test for
> > > > interoperability. Some aspects of the constructor execution are not
> > > > standardized and musl does not provide all tested feature, for one it
> > > > does not provide arguments to the constructors, anymore?
> > > >
> > > > Skip the constructor tests on non-nolibc configurations.
> > > 
> > > I'm not much surprised, I've always avoided arguments in my use of
> > > constructors due to a lack of portability. However the patch disables
> > > all constructors tests, while I'm seeing that the linkage_test version
> > > does not make use of arguments, though there is an implied expectation
> > > that they're executed in declaration order, which is not granted.
> > 
> > The tests are written specifically to test for execution order.
> > While we can not rely on the order for other libcs, the idea was to
> > expect a given order for the nolibc implementation.
> 
> OK.
> 
> > > I'm wondering if we shouldn't make the tests more robust:
> > >   1) explicitly set linkage_test_constructor_test_value to zero in the
> > >      declaration, because here it's not set so we have no guarantee
> > >      (we're not in the kernel)
> > 
> > Ack.
> > 
> > >   2) only add values to check for cumulated values (e.g. |1 in const1,
> > >      |2 in const2) and verify that the result is properly 3
> > 
> > This would stop validating the order.
> 
> That was my purpose but OK I got it. Then there's another option which
> preserves the order and even gives history:
> 
>   __attribute__((constructor))
>   static void constructor1(void)
>   {
>       constructor_test_value = constructor_test_value * 0x10 + 1;
>   }
> 
>   __attribute__((constructor))
>   static void constructor2(void)
>   {
>       constructor_test_value = constructor_test_value * 0x10 + 2;
>   }
>  
> Then if executed in the right order, you'll find 0x12. If both
> are executed in any order, it will always be >= 0x10. If only one
> is executed, it will be < 0x10, and if none is executed, it's 0.

Sounds good! Do you want to write a patch?
It should also add the missing zero-initializion of
constructor_test_value.


Thomas

Re: [PATCH] selftests/nolibc: only run constructor tests on nolibc

[Willy Tarreau]

Hi Thomas!

On Tue, Feb 25, 2025 at 10:37:24AM +0100, Thomas Weißschuh wrote:
> > > The tests are written specifically to test for execution order.
> > > While we can not rely on the order for other libcs, the idea was to
> > > expect a given order for the nolibc implementation.
> > 
> > OK.
> > 
> > > > I'm wondering if we shouldn't make the tests more robust:
> > > >   1) explicitly set linkage_test_constructor_test_value to zero in the
> > > >      declaration, because here it's not set so we have no guarantee
> > > >      (we're not in the kernel)
> > > 
> > > Ack.
> > > 
> > > >   2) only add values to check for cumulated values (e.g. |1 in const1,
> > > >      |2 in const2) and verify that the result is properly 3
> > > 
> > > This would stop validating the order.
> > 
> > That was my purpose but OK I got it. Then there's another option which
> > preserves the order and even gives history:
> > 
> >   __attribute__((constructor))
> >   static void constructor1(void)
> >   {
> >       constructor_test_value = constructor_test_value * 0x10 + 1;
> >   }
> > 
> >   __attribute__((constructor))
> >   static void constructor2(void)
> >   {
> >       constructor_test_value = constructor_test_value * 0x10 + 2;
> >   }
> >  
> > Then if executed in the right order, you'll find 0x12. If both
> > are executed in any order, it will always be >= 0x10. If only one
> > is executed, it will be < 0x10, and if none is executed, it's 0.
> 
> Sounds good! Do you want to write a patch?
> It should also add the missing zero-initializion of
> constructor_test_value.

OK so I've tested the patch below which does what we want, except that
it reveals that the order is still not granted. Actually I haven't found
what dictates it. On one machine (gcc-9.5, ld-2.26) I'm getting:

  $ ./nolibc-test|grep cst
  17 linkage_cst = 0                                              [FAIL]
  18 linkage_cst_ord = 0                                          [FAIL]

On this same machine, using another toolchain relying on ld-2.27 gives me
this:

  $ ./nolibc-test|grep cst
  17 linkage_cst = 1                                                [OK]
  18 linkage_cst_ord = 33                                          [FAIL]

And I'm getting this as well on another machine with various toolchains
such as gcc-9.5+ld-2.34. The nolibc toolchains fail similarly on gcc-5.5
(ld-2.27) and gcc-6.5 (ld-2.32), but work for gcc-7.5 with ld-2.32, while
other combinations do work:

  $ ./nolibc-test|grep -i cst
  17 linkage_cst = 1                                                [OK]
  18 linkage_cst_ord = 18                                           [OK]

All of this is a bit confusing.

I continue not to understand what could guarantee an implicit execution
order since for me it solely depends on how things are linked, so the
purpose of the test remains uncertain to me and I think we'd rather not
try to enforce any ordering that might work only by pure luck.

What do you think ?

Cheers,
Willy

---

diff --git a/tools/testing/selftests/nolibc/nolibc-test-linkage.c b/tools/testing/selftests/nolibc/nolibc-test-linkage.c
index 5ff4c8a1db2a4..ebb3eb4208efd 100644
--- a/tools/testing/selftests/nolibc/nolibc-test-linkage.c
+++ b/tools/testing/selftests/nolibc/nolibc-test-linkage.c
@@ -11,16 +11,16 @@ void *linkage_test_errno_addr(void)
 	return &errno;
 }
 
-int linkage_test_constructor_test_value;
+int linkage_test_constructor_test_value = 0;
 
 __attribute__((constructor))
 static void constructor1(void)
 {
-	linkage_test_constructor_test_value = 2;
+	linkage_test_constructor_test_value = linkage_test_constructor_test_value * 0x10 + 1;
 }
 
 __attribute__((constructor))
 static void constructor2(void)
 {
-	linkage_test_constructor_test_value *= 3;
+	linkage_test_constructor_test_value = linkage_test_constructor_test_value * 0x10 + 2;
 }
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 6fba7025c5e3c..2b5c30033e5eb 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -682,14 +682,15 @@ int expect_strtox(int llen, void *func, const char *input, int base, intmax_t ex
 __attribute__((constructor))
 static void constructor1(void)
 {
-	constructor_test_value = 1;
+	constructor_test_value = constructor_test_value * 0x10 + 1;
 }
 
 __attribute__((constructor))
 static void constructor2(int argc, char **argv, char **envp)
 {
+	constructor_test_value = constructor_test_value * 0x10 + 2;
 	if (argc && argv && envp)
-		constructor_test_value *= 2;
+		constructor_test_value = constructor_test_value * 0x10 + 4;
 }
 
 int run_startup(int min, int max)
@@ -728,9 +729,11 @@ int run_startup(int min, int max)
 		CASE_TEST(environ_HOME);     EXPECT_PTRNZ(1, getenv("HOME")); break;
 		CASE_TEST(auxv_addr);        EXPECT_PTRGT(test_auxv != (void *)-1, test_auxv, brk); break;
 		CASE_TEST(auxv_AT_UID);      EXPECT_EQ(1, getauxval(AT_UID), getuid()); break;
-		CASE_TEST(constructor);      EXPECT_EQ(1, constructor_test_value, 2); break;
+		CASE_TEST(constructor);      EXPECT_NE(1, (constructor_test_value & 0x111) && (constructor_test_value & 0x222), 0); break;
+		CASE_TEST(constructor_ord);  EXPECT_EQ(is_nolibc, constructor_test_value, 0x124); break;
 		CASE_TEST(linkage_errno);    EXPECT_PTREQ(1, linkage_test_errno_addr(), &errno); break;
-		CASE_TEST(linkage_constr);   EXPECT_EQ(1, linkage_test_constructor_test_value, 6); break;
+		CASE_TEST(linkage_cst);      EXPECT_NE(1, (linkage_test_constructor_test_value & 0x11) && (linkage_test_constructor_test_value & 0x22), 0); break;
+		CASE_TEST(linkage_cst_ord);  EXPECT_EQ(is_nolibc, linkage_test_constructor_test_value, 0x12); break;
 		case __LINE__:
 			return ret; /* must be last */
 		/* note: do not set any defaults so as to permit holes above */

Re: [PATCH] selftests/nolibc: only run constructor tests on nolibc

[Thomas Weißschuh]

Hi Willy!

On 2025-03-01 12:07:35+0100, Willy Tarreau wrote:
> [..]

> OK so I've tested the patch below which does what we want, except that
> it reveals that the order is still not granted. Actually I haven't found
> what dictates it. On one machine (gcc-9.5, ld-2.26) I'm getting:
> 
>   $ ./nolibc-test|grep cst
>   17 linkage_cst = 0                                              [FAIL]
>   18 linkage_cst_ord = 0                                          [FAIL]

Apparently no constructors are executed at all.
Can you show the default linkerscript used?

gcc -static -o /dev/null /dev/null -Wl,--verbose

> On this same machine, using another toolchain relying on ld-2.27 gives me
> this:
> 
>   $ ./nolibc-test|grep cst
>   17 linkage_cst = 1                                                [OK]
>   18 linkage_cst_ord = 33                                          [FAIL]
> 
> And I'm getting this as well on another machine with various toolchains
> such as gcc-9.5+ld-2.34. The nolibc toolchains fail similarly on gcc-5.5
> (ld-2.27) and gcc-6.5 (ld-2.32), but work for gcc-7.5 with ld-2.32, while
> other combinations do work:
> 
>   $ ./nolibc-test|grep -i cst
>   17 linkage_cst = 1                                                [OK]
>   18 linkage_cst_ord = 18                                           [OK]
> 
> All of this is a bit confusing.
> 
> I continue not to understand what could guarantee an implicit execution
> order since for me it solely depends on how things are linked, so the
> purpose of the test remains uncertain to me and I think we'd rather not
> try to enforce any ordering that might work only by pure luck.

I don't think anything guarantees the order. 
It is just what happened to work in my tests so far.

> What do you think ?

Let's get rid of the validation.


Thomas