This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 3/5][v2][BZ #15022] Correct global-scope dlopen issues in static executables
- From: Roland McGrath <roland at hack dot frob dot com>
- To: "Maciej W. Rozycki" <macro at codesourcery dot com>
- Cc: <libc-alpha at sourceware dot org>
- Date: Fri, 21 Jun 2013 15:37:29 -0700 (PDT)
- Subject: Re: [PATCH 3/5][v2][BZ #15022] Correct global-scope dlopen issues in static executables
- References: <alpine dot DEB dot 1 dot 10 dot 1301152056590 dot 4834 at tp dot orcam dot me dot uk> <20130116215545 dot 7A37A2C0B0 at topped-with-meat dot com> <alpine dot DEB dot 1 dot 10 dot 1301240655220 dot 4834 at tp dot orcam dot me dot uk> <20130531200059 dot C94C02C077 at topped-with-meat dot com> <alpine dot DEB dot 1 dot 10 dot 1306140202520 dot 16287 at tp dot orcam dot me dot uk>
> > It would be even better if as much of this as possible could be done with
> > static initialization rather than code that runs at startup. Can you try
> > to do it that way (in elf/dl-support.c) and see if it's not desperately
> > difficult?
>
> How about all static then?
That's ideal.
> + /* Try to map a module into the global scope. */
> + handle = dlopen ("modstatic3.so", RTLD_LAZY | RTLD_GLOBAL);
I guess this is testing just that having RTLD_GLOBAL there doesn't make it
crash? There is nothing in the test that actually makes using RTLD_GLOBAL
worthwhile, so the comment should say clearly why it's using the flag.
> +/* Global-scope DSO tests with a static executable.
This is the same comment as the first test. Is there something concise you
can say here that distinguishes why there are two tests? If it's just sort
of a basic test and an exhaustive test, I guess just adding ", more cases"
or something would be fine.
> + /* Try to map itself into the global scope. */
> + initial_handle = dlopen (NULL, RTLD_LAZY | RTLD_GLOBAL);
RTLD_GLOBAL isn't really meaningful here, is it?
The main program pseudo-object is the basis of the global scope.
> + printf ("*foop [initial]: got 0x%x, expected 0x%x\n", foo, MAGIC0);
Always use %#x in preference to 0x%x.
> + .l_libname = &(struct libname_list) { .name = "", .dont_free = 1 },
I didn't realize &compound_literal was kosher. Nice!
> +#if NO_TLS_OFFSET != 0
> + .l_tls_offset = NO_TLS_OFFSET,
> +#endif
Drop the #if. An explicit initializer to zero has no cost.
Aside from that one nit, the implementation itself looks great. I'll trust
you to do appropriate commenting and clean-up on the tests without another
round of review.
Thanks,
Roland