This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH][BZ #16046] Static dlopen correction fallout fixes
- From: "Maciej W. Rozycki" <macro at codesourcery dot com>
- To: Roland McGrath <roland at hack dot frob dot com>
- Cc: Allan McRae <allan at archlinux dot org>, Ondřej Bílka <neleai at seznam dot cz>, <libc-alpha at sourceware dot org>
- Date: Mon, 3 Feb 2014 13:18:46 +0000
- Subject: Re: [PATCH][BZ #16046] Static dlopen correction fallout fixes
- Authentication-results: sourceware.org; auth=none
- References: <20131017174710 dot GA4993 at domone dot podge> <20131025210328 dot 39E69746B6 at topped-with-meat dot com> <alpine dot DEB dot 1 dot 10 dot 1310252347350 dot 12843 at tp dot orcam dot me dot uk> <20140116203847 dot GB20838 at domone dot podge> <alpine dot DEB dot 1 dot 10 dot 1401172303320 dot 4268 at tp dot orcam dot me dot uk> <20140117233957 dot 64E307441B at topped-with-meat dot com> <alpine dot DEB dot 1 dot 10 dot 1401271320170 dot 4268 at tp dot orcam dot me dot uk> <alpine dot DEB dot 1 dot 10 dot 1401291054290 dot 4268 at tp dot orcam dot me dot uk> <20140129184954 dot 1BFFA74441 at topped-with-meat dot com> <alpine dot DEB dot 1 dot 10 dot 1401300707090 dot 4268 at tp dot orcam dot me dot uk> <20140130203835 dot EB0A874414 at topped-with-meat dot com> <alpine dot DEB dot 1 dot 10 dot 1401311837460 dot 4268 at tp dot orcam dot me dot uk> <20140131192948 dot 2F8A774430 at topped-with-meat dot com>
On Fri, 31 Jan 2014, Roland McGrath wrote:
> > Thinking more about the test case I have concluded that maybe a
> > path-of-least-resistance check like below would do. It verifies that
> > there's exactly one link map whose file name is null, as is the case with
> > the main executable only. This approach may not be particularly robust,
> > but it does cover what BZ #16046 is about.
>
> It's a start. Ideal would be to look at all the places you had to change
> and figure out what user-visible effects those changes had.
Only elf/dl-iteratephdr.c (covered here) and elf/dl-load.c (default lib
search path) changes can have any user-visible effect. The rest are
internal consistency updates, e.g. you can't really test an assertion
unless there's a separate bug to be fixed. I'll see if I can think of a
test case covering the default lib search path for static executables.
> > +static int count;
>
> Why have a static instead of passing the address of a local in the void *?
I didn't want to overcomplicate code, however if you prefer using the
iterator's argument, then I'm fine with that.
> > +static int
> > +callback (struct dl_phdr_info *info, size_t size, void *data)
> > +{
> > + if (!*info->dlpi_name)
>
> No implicit Boolean coercion please: (info->dlpi_name[0] == '\0') is the
> canonical way to write this expression.
>
> > +int
> > +main(void)
>
> Space before paren.
>
> It's also good to have a comment simply explaining what it's verifying,
> even though it is fairly obvious in this case.
The rest I agree make the piece a little bit better. I took the
opportunity and made further adjustments, specifically wrapped the case in
the test skeleton in case it loops or something and removed the unneeded
_GNU_SOURCE definition (already provided by include/libc-symbols.h).
Here's the resulting new version. Any further suggestions or comments?
2014-02-03 Maciej W. Rozycki <macro@codesourcery.com>
[BZ #16046]
elf/tst-dl-iter-static.c: New file.
elf/Makefile (tests-static): Add tst-dl-iter-static.
Maciej
glibc-static-dlopen-16046-test.diff
Index: glibc-fsf-trunk-quilt/elf/Makefile
===================================================================
--- glibc-fsf-trunk-quilt.orig/elf/Makefile 2014-01-29 12:29:54.000000000 +0000
+++ glibc-fsf-trunk-quilt/elf/Makefile 2014-01-31 18:09:48.501960005 +0000
@@ -123,7 +123,7 @@ tests = tst-tls1 tst-tls2 tst-tls9 tst-l
tst-auxv
tests-static = tst-tls1-static tst-tls2-static tst-stackguard1-static \
tst-leaks1-static tst-array1-static tst-array5-static \
- tst-ptrguard1-static
+ tst-ptrguard1-static tst-dl-iter-static
ifeq (yes,$(build-shared))
tests-static += tst-tls9-static
tst-tls9-static-ENV = \
Index: glibc-fsf-trunk-quilt/elf/tst-dl-iter-static.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ glibc-fsf-trunk-quilt/elf/tst-dl-iter-static.c 2014-02-03 11:31:51.541961661 +0000
@@ -0,0 +1,52 @@
+/* BZ #16046 dl_iterate_phdr static executable test.
+ Copyright (C) 2014 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <link.h>
+
+/* Check that the link map of the static executable itself is iterated
+ over exactly once. */
+
+static int
+callback (struct dl_phdr_info *info, size_t size, void *data)
+{
+ union
+ {
+ void *data;
+ int *count;
+ }
+ u = { .data = data };
+
+ if (info->dlpi_name[0] == '\0')
+ (*u.count)++;
+
+ return 0;
+}
+
+static int
+do_test (void)
+{
+ int count = 0;
+ int status;
+
+ status = dl_iterate_phdr (callback, &count);
+
+ return status || count != 1;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"