diff mbox series

tools/nolibc: pass argc, argv and envp to constructors

Message ID 20240728-nolibc-constructor-args-v1-1-36d0bf5cd4c0@weissschuh.net (mailing list archive)
State Accepted
Commit 754283ce8326fdce75d589c99cc58456199c123d
Headers show
Series tools/nolibc: pass argc, argv and envp to constructors | expand

Commit Message

Thomas Weißschuh July 28, 2024, 8:34 p.m. UTC
Mirror glibc behavior for compatibility.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/include/nolibc/crt.h                   | 23 ++++++++++++-----------
 tools/testing/selftests/nolibc/nolibc-test.c |  5 +++--
 2 files changed, 15 insertions(+), 13 deletions(-)


---
base-commit: 0db287736bc586fcd5a2925518ef09eec6924803
change-id: 20240728-nolibc-constructor-args-68bf56ff7438

Best regards,

Comments

Willy Tarreau Aug. 3, 2024, 10:03 a.m. UTC | #1
On Sun, Jul 28, 2024 at 10:34:11PM +0200, Thomas Weißschuh wrote:
> Mirror glibc behavior for compatibility.

Generally speaking I think you should make a bit longer sentences in
your commit messages, Thomas. One first reason is to think that during
reviews the reviewer has to scroll up to find the subject for the context
this sentence applies to. And doing so quickly encourages to give a little
bit more background to justify a change. I have a simple principle that
works reasonably fine for this, which is that a commit subject should
normally be unique in a project (modulo rare cases, reverts or accidents)
and that commit message bodies should really always be unique. Here we
see that it doesn't work ;-)

An example could be something like this:

  Glibc has been passing argc/argv/envp to constructors since version XXX.
  This is particularly convenient, and missing it can significantly
  complicate some ports to nolibc. Let's do the same since it's an easy
  change that comes at no cost.

Anyway I agree with the change, I wasn't aware of this support from glibc,
so thank you for enlighting me on this one ;-)

Willy
Thomas Weißschuh Aug. 3, 2024, 6:23 p.m. UTC | #2
Aug 3, 2024 12:03:27 Willy Tarreau <w@1wt.eu>:

> On Sun, Jul 28, 2024 at 10:34:11PM +0200, Thomas Weißschuh wrote:
>> Mirror glibc behavior for compatibility.
>
> Generally speaking I think you should make a bit longer sentences in
> your commit messages, Thomas. One first reason is to think that during
> reviews the reviewer has to scroll up to find the subject for the context
> this sentence applies to. And doing so quickly encourages to give a little
> bit more background to justify a change. I have a simple principle that
> works reasonably fine for this, which is that a commit subject should
> normally be unique in a project (modulo rare cases, reverts or accidents)
> and that commit message bodies should really always be unique. Here we
> see that it doesn't work ;-)

Complete Ack :-)
I tend to become lazy when I feel to get away with it.
Thanks for calling me out on it.

> An example could be something like this:
>
>   Glibc has been passing argc/argv/envp to constructors since version XXX.
>   This is particularly convenient, and missing it can significantly
>   complicate some ports to nolibc. Let's do the same since it's an easy
>   change that comes at no cost.
>
> Anyway I agree with the change, I wasn't aware of this support from glibc,
> so thank you for enlighting me on this one ;-)

It's meticulously undocumented,
and a very fringe usecase.

I'll use your proposal,  fill it with more background and apply it.


Thomas
diff mbox series

Patch

diff --git a/tools/include/nolibc/crt.h b/tools/include/nolibc/crt.h
index 43b551468c2a..ac291574f6c0 100644
--- a/tools/include/nolibc/crt.h
+++ b/tools/include/nolibc/crt.h
@@ -13,11 +13,11 @@  const unsigned long *_auxv __attribute__((weak));
 static void __stack_chk_init(void);
 static void exit(int);
 
-extern void (*const __preinit_array_start[])(void) __attribute__((weak));
-extern void (*const __preinit_array_end[])(void) __attribute__((weak));
+extern void (*const __preinit_array_start[])(int, char **, char**) __attribute__((weak));
+extern void (*const __preinit_array_end[])(int, char **, char**) __attribute__((weak));
 
-extern void (*const __init_array_start[])(void) __attribute__((weak));
-extern void (*const __init_array_end[])(void) __attribute__((weak));
+extern void (*const __init_array_start[])(int, char **, char**) __attribute__((weak));
+extern void (*const __init_array_end[])(int, char **, char**) __attribute__((weak));
 
 extern void (*const __fini_array_start[])(void) __attribute__((weak));
 extern void (*const __fini_array_end[])(void) __attribute__((weak));
@@ -29,7 +29,8 @@  void _start_c(long *sp)
 	char **argv;
 	char **envp;
 	int exitcode;
-	void (* const *func)(void);
+	void (* const *ctor_func)(int, char **, char **);
+	void (* const *dtor_func)(void);
 	const unsigned long *auxv;
 	/* silence potential warning: conflicting types for 'main' */
 	int _nolibc_main(int, char **, char **) __asm__ ("main");
@@ -66,16 +67,16 @@  void _start_c(long *sp)
 		;
 	_auxv = auxv;
 
-	for (func = __preinit_array_start; func < __preinit_array_end; func++)
-		(*func)();
-	for (func = __init_array_start; func < __init_array_end; func++)
-		(*func)();
+	for (ctor_func = __preinit_array_start; ctor_func < __preinit_array_end; ctor_func++)
+		(*ctor_func)(argc, argv, envp);
+	for (ctor_func = __init_array_start; ctor_func < __init_array_end; ctor_func++)
+		(*ctor_func)(argc, argv, envp);
 
 	/* go to application */
 	exitcode = _nolibc_main(argc, argv, envp);
 
-	for (func = __fini_array_end; func > __fini_array_start;)
-		(*--func)();
+	for (dtor_func = __fini_array_end; dtor_func > __fini_array_start;)
+		(*--dtor_func)();
 
 	exit(exitcode);
 }
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 093d0512f4c5..0800b10fc3f7 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -686,9 +686,10 @@  static void constructor1(void)
 }
 
 __attribute__((constructor))
-static void constructor2(void)
+static void constructor2(int argc, char **argv, char **envp)
 {
-	constructor_test_value *= 2;
+	if (argc && argv && envp)
+		constructor_test_value *= 2;
 }
 
 int run_startup(int min, int max)