Re: [PATCH] ld.so: command argument "--preload"

[Florian Weimer <fweimer at redhat dot com>]

* David Newall:

> On 7/11/18 8:53 pm, Florian Weimer wrote:
>> Yes, as long as it's not text/html.  Please resend your patch as an
>> attachment; it looks like some lines were wrapped.
>
> I like HTML email (I know! and I've been doing UNIX since '80s) but
> this client is doing the movement no favour by mangling what I asked
> to be pre-formatted text.  Here it is as an attachment.

Much better, thanks.

> diff --git a/elf/Makefile b/elf/Makefile
> index d72e7b6..901e188 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -353,7 +353,8 @@ endif
>  
>  ifeq (yes,$(build-shared))
>  ifeq ($(run-built-tests),yes)
> -tests-special += $(objpfx)tst-pathopt.out $(objpfx)tst-rtld-load-self.out
> +tests-special += $(objpfx)tst-pathopt.out $(objpfx)tst-rtld-load-self.out \
> +		 $(objpfx)tst-rtld-preload.out
>  endif
>  tests-special += $(objpfx)check-textrel.out $(objpfx)check-execstack.out \
>  		 $(objpfx)check-localplt.out $(objpfx)check-initfini.out
> @@ -882,6 +883,13 @@ $(objpfx)tst-rtld-load-self.out: tst-rtld-load-self.sh $(objpfx)ld.so
>  	$(SHELL) $^ '$(test-wrapper)' '$(test-wrapper-env)' > $@; \
>  	$(evaluate-test)
>  
> +tst-rtld-preload-OBJS =  $(subst $(empty) ,:,$(strip $(preloadtest-preloads:=.so)))
> +$(objpfx)tst-rtld-preload.out: tst-rtld-preload.sh $(objpfx)ld.so \
> +			       $(objpfx)preloadtest

Missing dependency on the objects in $(tst-rtld-preload-OBJS), I think.
I expect you need to add the prefix $(objpfx) to the dependency.

> +	$(SHELL) $^ '$(test-wrapper)' '$(test-wrapper-env)' '$(run_program_env)' \
> +		    '$(rpath-link)' '$(tst-rtld-preload-OBJS)' > $@; \
> + 	$(evaluate-test)

There are two whitespace errors on this line (leading and trailing
space).

>  $(objpfx)initfirst: $(libdl)
>  $(objpfx)initfirst.out: $(objpfx)firstobj.so
>  
> diff --git a/elf/rtld.c b/elf/rtld.c
> index 1b0c747..f046f6e 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -826,6 +826,8 @@ static const char *library_path attribute_relro;
>  static const char *preloadlist attribute_relro;
>  /* Nonzero if information about versions has to be printed.  */
>  static int version_info attribute_relro;
> +/* The preload list passed as a command argument. */
> +static const char *preloadarg attribute_relro;
>  
>  /* The LD_PRELOAD environment variable gives list of libraries
>     separated by white space or colons that are loaded before the
> @@ -833,8 +835,9 @@ static int version_info attribute_relro;
>     (If the binary is running setuid all elements containing a '/' are
>     ignored since it is insecure.)  Return the number of preloads
>     performed.  */
> +/* Ditto for --preload command argument */

I would merge the last line into the existing comment, and there should
be a ".  " at the end (period followed by two spaces, as before).

>  unsigned int
> -handle_ld_preload (const char *preloadlist, struct link_map *main_map)
> +handle_preload_list (const char *preloadlist, struct link_map *main_map, const char *where)

Please wrap the at 79 characters.

Rest looks good to me.

Thanks,
Florian