[PATCH 2/2] add preinit_array support

[PATCH 2/2] add preinit_array support

[Szabolcs Nagy]

handle DT_PREINIT_ARRAY{SZ} in case of dynamic linking and
__preinit_array_{start,end} in case of static linking.

redefined DYN_CNT because 32 is not enough for DT_PREINIT_ARRAY.
---
 src/env/__libc_start_main.c |  5 +++++
 src/ldso/dynlink.c          | 12 ++++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/env/__libc_start_main.c b/src/env/__libc_start_main.c
index f9c7ca5..d4cc0ef 100644
--- a/src/env/__libc_start_main.c
+++ b/src/env/__libc_start_main.c
@@ -12,6 +12,9 @@ static void dummy(void) {}
 weak_alias(dummy, _init);
 
 __attribute__((__weak__, __visibility__("hidden")))
+extern void (*const __preinit_array_start[])(void), (*const __preinit_array_end[])(void);
+
+__attribute__((__weak__, __visibility__("hidden")))
 extern void (*const __init_array_start[])(void), (*const __init_array_end[])(void);
 
 static void dummy1(void *p) {}
@@ -57,6 +60,8 @@ static void libc_start_init(void)
 {
 	void (*const *f)(void);
 	_init();
+	for (f=__preinit_array_start; f<__preinit_array_end; f++)
+		(*f)();
 	for (f=__init_array_start; f<__init_array_end; f++)
 		(*f)();
 }
diff --git a/src/ldso/dynlink.c b/src/ldso/dynlink.c
index 93e7d67..623818a 100644
--- a/src/ldso/dynlink.c
+++ b/src/ldso/dynlink.c
@@ -135,11 +135,14 @@ static struct fdpic_dummy_loadmap app_dummy_loadmap;
 struct debug *_dl_debug_addr = &debug;
 
 __attribute__((__visibility__("hidden")))
-void (*const __init_array_start)(void)=0, (*const __fini_array_start)(void)=0;
+void (*const __init_array_start)(void)=0, (*const __fini_array_start)(void)=0,
+(*const __preinit_array_start)(void)=0;
 
 __attribute__((__visibility__("hidden")))
-extern void (*const __init_array_end)(void), (*const __fini_array_end)(void);
+extern void (*const __init_array_end)(void), (*const __fini_array_end)(void),
+(*const __preinit_array_end)(void);
 
+weak_alias(__preinit_array_start, __preinit_array_end);
 weak_alias(__init_array_start, __init_array_end);
 weak_alias(__fini_array_start, __fini_array_end);
 
@@ -1229,6 +1232,11 @@ static void do_init_fini(struct dso *p)
 		if ((dyn[0] & (1<<DT_INIT)) && dyn[DT_INIT])
 			fpaddr(p, dyn[DT_INIT])();
 #endif
+		if (dyn[0] & (1UL<<DT_PREINIT_ARRAY)) {
+			size_t n = dyn[DT_PREINIT_ARRAYSZ]/sizeof(size_t);
+			size_t *fn = laddr(p, dyn[DT_PREINIT_ARRAY]);
+			while (n--) ((void (*)(void))*fn++)();
+		}
 		if (dyn[0] & (1<<DT_INIT_ARRAY)) {
 			size_t n = dyn[DT_INIT_ARRAYSZ]/sizeof(size_t);
 			size_t *fn = laddr(p, dyn[DT_INIT_ARRAY]);
-- 
2.4.1

Re: [PATCH 2/2] add preinit_array support

[Szabolcs Nagy]

[-- Attachment #1: Type: text/plain, Size: 414 bytes --]

* Szabolcs Nagy <nsz@port70.net> [2015-11-29 16:33:22 +0100]:
> redefined DYN_CNT because 32 is not enough for DT_PREINIT_ARRAY.
> ---
>  src/env/__libc_start_main.c |  5 +++++
>  src/ldso/dynlink.c          | 12 ++++++++++--
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 

src/internal change was missing and legacy _init vs
preinit_array order was wrong.

attached new version, array type is fixed too.

[-- Attachment #2: 0002-add-preinit_array-support.patch --]
[-- Type: text/x-diff, Size: 3087 bytes --]

From 8820faea5a365387c8fe54c8ffa1f796c2084f83 Mon Sep 17 00:00:00 2001
From: Szabolcs Nagy <nsz@port70.net>
Date: Sun, 29 Nov 2015 16:36:24 +0000
Subject: [PATCH 2/2] add preinit_array support

handle DT_PREINIT_ARRAY{SZ} in case of dynamic linking and
__preinit_array_{start,end} in case of static linking.

redefined DYN_CNT because 32 is not enough for DT_PREINIT_ARRAY.
---
 src/env/__libc_start_main.c |  5 +++++
 src/internal/dynlink.h      |  2 +-
 src/ldso/dynlink.c          | 12 ++++++++++--
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/env/__libc_start_main.c b/src/env/__libc_start_main.c
index 14a2825..7502bc3 100644
--- a/src/env/__libc_start_main.c
+++ b/src/env/__libc_start_main.c
@@ -12,6 +12,9 @@ static void dummy(void) {}
 weak_alias(dummy, _init);
 
 __attribute__((__weak__, __visibility__("hidden")))
+extern void (*const __preinit_array_start)(void), (*const __preinit_array_end)(void);
+
+__attribute__((__weak__, __visibility__("hidden")))
 extern void (*const __init_array_start)(void), (*const __init_array_end)(void);
 
 static void dummy1(void *p) {}
@@ -56,6 +59,8 @@ void __init_libc(char **envp, char *pn)
 static void libc_start_init(void)
 {
 	void (*const *f)(void);
+	for (f=&__preinit_array_start; f<&__preinit_array_end; f++)
+		(*f)();
 	_init();
 	for (f=&__init_array_start; f<&__init_array_end; f++)
 		(*f)();
diff --git a/src/internal/dynlink.h b/src/internal/dynlink.h
index 9c494e4..fb706d3 100644
--- a/src/internal/dynlink.h
+++ b/src/internal/dynlink.h
@@ -84,7 +84,7 @@ struct fdpic_dummy_loadmap {
 #endif
 
 #define AUX_CNT 32
-#define DYN_CNT 32
+#define DYN_CNT DT_NUM
 
 typedef void (*stage2_func)(unsigned char *, size_t *);
 typedef _Noreturn void (*stage3_func)(size_t *);
diff --git a/src/ldso/dynlink.c b/src/ldso/dynlink.c
index 93e7d67..51da820 100644
--- a/src/ldso/dynlink.c
+++ b/src/ldso/dynlink.c
@@ -135,11 +135,14 @@ static struct fdpic_dummy_loadmap app_dummy_loadmap;
 struct debug *_dl_debug_addr = &debug;
 
 __attribute__((__visibility__("hidden")))
-void (*const __init_array_start)(void)=0, (*const __fini_array_start)(void)=0;
+void (*const __init_array_start)(void)=0, (*const __fini_array_start)(void)=0,
+(*const __preinit_array_start)(void)=0;
 
 __attribute__((__visibility__("hidden")))
-extern void (*const __init_array_end)(void), (*const __fini_array_end)(void);
+extern void (*const __init_array_end)(void), (*const __fini_array_end)(void),
+(*const __preinit_array_end)(void);
 
+weak_alias(__preinit_array_start, __preinit_array_end);
 weak_alias(__init_array_start, __init_array_end);
 weak_alias(__fini_array_start, __fini_array_end);
 
@@ -1225,6 +1228,11 @@ static void do_init_fini(struct dso *p)
 			p->fini_next = fini_head;
 			fini_head = p;
 		}
+		if (dyn[0] & (1UL<<DT_PREINIT_ARRAY)) {
+			size_t n = dyn[DT_PREINIT_ARRAYSZ]/sizeof(size_t);
+			size_t *fn = laddr(p, dyn[DT_PREINIT_ARRAY]);
+			while (n--) ((void (*)(void))*fn++)();
+		}
 #ifndef NO_LEGACY_INITFINI
 		if ((dyn[0] & (1<<DT_INIT)) && dyn[DT_INIT])
 			fpaddr(p, dyn[DT_INIT])();
-- 
2.4.1

Re: [PATCH 2/2] add preinit_array support

[Rich Felker]

On Sun, Nov 29, 2015 at 05:59:25PM +0100, Szabolcs Nagy wrote:
> * Szabolcs Nagy <nsz@port70.net> [2015-11-29 16:33:22 +0100]:
> > redefined DYN_CNT because 32 is not enough for DT_PREINIT_ARRAY.
> > ---
> >  src/env/__libc_start_main.c |  5 +++++
> >  src/ldso/dynlink.c          | 12 ++++++++++--
> >  2 files changed, 15 insertions(+), 2 deletions(-)
> > 
> 
> src/internal change was missing and legacy _init vs
> preinit_array order was wrong.
> 
> attached new version, array type is fixed too.

> >From 8820faea5a365387c8fe54c8ffa1f796c2084f83 Mon Sep 17 00:00:00 2001
> From: Szabolcs Nagy <nsz@port70.net>
> Date: Sun, 29 Nov 2015 16:36:24 +0000
> Subject: [PATCH 2/2] add preinit_array support
> 
> handle DT_PREINIT_ARRAY{SZ} in case of dynamic linking and
> __preinit_array_{start,end} in case of static linking.

Omission of preinit array support was intentional since it's reserved
for use by the [library] implementation and musl does not use it, and
I've tried to keep mandatory-linked startup code as small as possible
to keep the small-static-binaries people happy.

However on IRC we discussed that ASan and VTV and perhaps other
similar tools might use it, and such use makes sense, so we may need
to add it. I just don't want to get in a habit of adding whatever
random junk is in the ELF spec when there's no reasonable,
well-defined way for application code to use or depend on it.

> redefined DYN_CNT because 32 is not enough for DT_PREINIT_ARRAY.

This should be reviewed for possible problems; IIRC I reduced it to 32
to avoid having to worry about undefined bitshifts in some code that
can't do 64-bit shifts (because libgcc functions might not yet be
callable) but that code should really hard-code 32 rather than using
DYN_CNT if it's still a problem.

> ---
>  src/env/__libc_start_main.c |  5 +++++
>  src/internal/dynlink.h      |  2 +-
>  src/ldso/dynlink.c          | 12 ++++++++++--
>  3 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/src/env/__libc_start_main.c b/src/env/__libc_start_main.c
> index 14a2825..7502bc3 100644
> --- a/src/env/__libc_start_main.c
> +++ b/src/env/__libc_start_main.c
> @@ -12,6 +12,9 @@ static void dummy(void) {}
>  weak_alias(dummy, _init);
>  
>  __attribute__((__weak__, __visibility__("hidden")))
> +extern void (*const __preinit_array_start)(void), (*const __preinit_array_end)(void);
> +
> +__attribute__((__weak__, __visibility__("hidden")))
>  extern void (*const __init_array_start)(void), (*const __init_array_end)(void);
>  
>  static void dummy1(void *p) {}
> @@ -56,6 +59,8 @@ void __init_libc(char **envp, char *pn)
>  static void libc_start_init(void)
>  {
>  	void (*const *f)(void);
> +	for (f=&__preinit_array_start; f<&__preinit_array_end; f++)
> +		(*f)();
>  	_init();
>  	for (f=&__init_array_start; f<&__init_array_end; f++)
>  		(*f)();
> diff --git a/src/internal/dynlink.h b/src/internal/dynlink.h
> index 9c494e4..fb706d3 100644
> --- a/src/internal/dynlink.h
> +++ b/src/internal/dynlink.h
> @@ -84,7 +84,7 @@ struct fdpic_dummy_loadmap {
>  #endif
>  
>  #define AUX_CNT 32
> -#define DYN_CNT 32
> +#define DYN_CNT DT_NUM

DT_NUM is probably excessive and just increases stack usage with no
benefit.

>  typedef void (*stage2_func)(unsigned char *, size_t *);
>  typedef _Noreturn void (*stage3_func)(size_t *);
> diff --git a/src/ldso/dynlink.c b/src/ldso/dynlink.c
> index 93e7d67..51da820 100644
> --- a/src/ldso/dynlink.c
> +++ b/src/ldso/dynlink.c
> @@ -135,11 +135,14 @@ static struct fdpic_dummy_loadmap app_dummy_loadmap;
>  struct debug *_dl_debug_addr = &debug;
>  
>  __attribute__((__visibility__("hidden")))
> -void (*const __init_array_start)(void)=0, (*const __fini_array_start)(void)=0;
> +void (*const __init_array_start)(void)=0, (*const __fini_array_start)(void)=0,
> +(*const __preinit_array_start)(void)=0;
>  
>  __attribute__((__visibility__("hidden")))
> -extern void (*const __init_array_end)(void), (*const __fini_array_end)(void);
> +extern void (*const __init_array_end)(void), (*const __fini_array_end)(void),
> +(*const __preinit_array_end)(void);
>  
> +weak_alias(__preinit_array_start, __preinit_array_end);
>  weak_alias(__init_array_start, __init_array_end);
>  weak_alias(__fini_array_start, __fini_array_end);
>  
> @@ -1225,6 +1228,11 @@ static void do_init_fini(struct dso *p)
>  			p->fini_next = fini_head;
>  			fini_head = p;
>  		}
> +		if (dyn[0] & (1UL<<DT_PREINIT_ARRAY)) {

This invokes UB on 32-bit archs, and couldn't possibly work anyway
since the bit simply doesn't exist. Probably the test just needs to
be "if (dyn[DT_PREINIT_ARRAYSZ])" or similar. Alternatively,
search_vec could be used.

Further, I don't think having this code inside the do_init_fini loop
makes sense -- that loop runs backwards starting from the last-loaded
library, whereas preinit arrays are only valid in the main program (at
least according to ld, IIRC) and must run before any normal init
arrays or legacy _init functions (for any DSOs) are run.

Rich

Re: [PATCH 2/2] add preinit_array support

[Szabolcs Nagy]

[-- Attachment #1: Type: text/plain, Size: 1583 bytes --]

* Rich Felker <dalias@libc.org> [2015-11-29 18:43:09 -0500]:
> On Sun, Nov 29, 2015 at 05:59:25PM +0100, Szabolcs Nagy wrote:
> > * Szabolcs Nagy <nsz@port70.net> [2015-11-29 16:33:22 +0100]:
> > handle DT_PREINIT_ARRAY{SZ} in case of dynamic linking and
> > __preinit_array_{start,end} in case of static linking.
> 
> Omission of preinit array support was intentional since it's reserved
> for use by the [library] implementation and musl does not use it, and
> I've tried to keep mandatory-linked startup code as small as possible
> to keep the small-static-binaries people happy.
> 
> However on IRC we discussed that ASan and VTV and perhaps other
> similar tools might use it, and such use makes sense, so we may need
> to add it. I just don't want to get in a habit of adding whatever
> random junk is in the ELF spec when there's no reasonable,
> well-defined way for application code to use or depend on it.
> 
> > redefined DYN_CNT because 32 is not enough for DT_PREINIT_ARRAY.
> 
> This should be reviewed for possible problems; IIRC I reduced it to 32
> to avoid having to worry about undefined bitshifts in some code that
> can't do 64-bit shifts (because libgcc functions might not yet be
> callable) but that code should really hard-code 32 rather than using
> DYN_CNT if it's still a problem.
> 

attached another variant in case we will need it
(handles DT_PREINIT* separately).

i didn't handle the issue described in
http://git.musl-libc.org/cgit/musl/commit/?id=19caa25d0a8e587bb89b79c3f629085548709dd4
i don't know if that may apply to the preinit* symbols too.

[-- Attachment #2: 0001-add-preinit_array-support.patch --]
[-- Type: text/x-diff, Size: 1975 bytes --]

From 18cfe83a9f3afacb03f3cad8e8b9d78121179451 Mon Sep 17 00:00:00 2001
From: Szabolcs Nagy <nsz@port70.net>
Date: Sun, 15 May 2016 20:11:42 +0000
Subject: [PATCH] add preinit_array support

---
 ldso/dynlink.c              | 13 +++++++++++++
 src/env/__libc_start_main.c |  8 +++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/ldso/dynlink.c b/ldso/dynlink.c
index e458f38..38709ec 100644
--- a/ldso/dynlink.c
+++ b/ldso/dynlink.c
@@ -1207,6 +1207,18 @@ void __libc_exit_fini()
 	}
 }
 
+static void do_preinit(void)
+{
+	size_t *v, *fn, n=0;
+
+	for (v = head->dynv; v[0]; v+=2)
+		if (v[0] == DT_PREINIT_ARRAY)
+			fn = laddr(head, v[1]);
+		else if (v[0] == DT_PREINIT_ARRAYSZ)
+			n = v[1]/sizeof(size_t);
+	while (n--) ((void(*)(void))*fn++)();
+}
+
 static void do_init_fini(struct dso *p)
 {
 	size_t dyn[DYN_CNT];
@@ -1242,6 +1254,7 @@ static void do_init_fini(struct dso *p)
 
 void __libc_start_init(void)
 {
+	do_preinit();
 	do_init_fini(tail);
 }
 
diff --git a/src/env/__libc_start_main.c b/src/env/__libc_start_main.c
index 5c79be2..1397ec4 100644
--- a/src/env/__libc_start_main.c
+++ b/src/env/__libc_start_main.c
@@ -12,6 +12,9 @@ static void dummy(void) {}
 weak_alias(dummy, _init);
 
 __attribute__((__weak__, __visibility__("hidden")))
+extern void (*const __preinit_array_start)(void), (*const __preinit_array_end)(void);
+
+__attribute__((__weak__, __visibility__("hidden")))
 extern void (*const __init_array_start)(void), (*const __init_array_end)(void);
 
 static void dummy1(void *p) {}
@@ -55,8 +58,11 @@ void __init_libc(char **envp, char *pn)
 
 static void libc_start_init(void)
 {
+	uintptr_t a = (uintptr_t)&__preinit_array_start;
+	for (; a<(uintptr_t)&__preinit_array_end; a+=sizeof(void(*)()))
+		(*(void (**)())a)();
 	_init();
-	uintptr_t a = (uintptr_t)&__init_array_start;
+	a = (uintptr_t)&__init_array_start;
 	for (; a<(uintptr_t)&__init_array_end; a+=sizeof(void(*)()))
 		(*(void (**)())a)();
 }
-- 
2.8.1