Re: [PATCH 1/2] init_array/fini_array cleanup

[Rich Felker <dalias@libc.org>]

On Sun, Nov 29, 2015 at 05:47:24PM +0100, Szabolcs Nagy wrote:
> * Szabolcs Nagy <nsz@port70.net> [2015-11-29 16:30:51 +0100]:
> > -extern void (*const __init_array_start)(void), (*const __init_array_end)(void);
> > +extern void (*const __init_array_start[])(void), (*const __init_array_end[])(void);
> 
> Alexander Monakov pointed out that the compiler may
> consider arrays to be separate objects and then
> f<__init_array_end would be ub.
> 
> attached an updated version, the same code is generated
> on x86_64 as before, except there is no xor %eax,%eax
> now before the call (because there is prototype).

> >From 3c29dc0d9bdb2e4d5f735d7b3d272d92cb82d2c6 Mon Sep 17 00:00:00 2001
> From: Szabolcs Nagy <nsz@port70.net>
> Date: Sun, 29 Nov 2015 16:24:17 +0000
> Subject: [PATCH 1/2] cleaner init/fini array handling
> 
> Avoid casts and make sure there is a prototype when init/fini
> functions are called.
> ---
>  src/env/__libc_start_main.c | 6 +++---
>  src/exit/exit.c             | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/src/env/__libc_start_main.c b/src/env/__libc_start_main.c
> index 5c79be2..14a2825 100644
> --- a/src/env/__libc_start_main.c
> +++ b/src/env/__libc_start_main.c
> @@ -55,10 +55,10 @@ void __init_libc(char **envp, char *pn)
>  
>  static void libc_start_init(void)
>  {
> +	void (*const *f)(void);
>  	_init();
> -	uintptr_t a = (uintptr_t)&__init_array_start;
> -	for (; a<(uintptr_t)&__init_array_end; a+=sizeof(void(*)()))
> -		(*(void (**)())a)();
> +	for (f=&__init_array_start; f<&__init_array_end; f++)
> +		(*f)();

This version still contains UB: since the initial f points to one
object (__init_array_start), the comparison against a different
object's address with < is UB, and after the first f++, all subsequent
f++ operations are also UB.

The original code was carefully written to avoid this by performing
all arithmetic in uintptr_t, but reportedly gcc and llvm even perform
origin analysis on integers, despite it being an invalid optimization,
so we may need to harden this further.

Using an array type (like your first patch) and using != instead of <
might be another suitable approach. Use of != would not be UB, and
since the result of equality comparisons between "one past the end" of
an array and a fixed other object must be consistent between multiple
ways of evaluating it, a compiler can't optimize out the !=. However,
this does not translate into a viable approach for the
opposite-direction iteration in the fini array.

I'm open to other ideas that are more robust against bad compiler
behavior.

Rich