Message ID | da438731-e148-0097-cd30-76f8b4175ad2@linaro.org |
---|---|
State | Dropped |
Headers |
Received: (qmail 58675 invoked by alias); 17 Apr 2019 13:59:21 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 58665 invoked by uid 89); 17 Apr 2019 13:59:21 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-19.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mail-vs1-f43.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=to:references:from:openpgp:autocrypt:subject:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=1csZVQC//B/hyKyd6jJFtUw8m+CTdTYKAdwi6k9OaPk=; b=HeZbFkTzAXZco61VPRp9YF6qMeR59TVcKwlW8ul/DlK7wezovNo8YkxBWxwS+A00yP OFOl4JlfznOv/3c26g8GPXROEbRZHZQsx5HY1qpvVhIkEgb2A7/5hXo+APvMTlWdhv1c n5Fk+PbmqTudQ6eHTcttz+xmWU39I55PYVi+/8o5MPaiEYVWiQQ6ZbeD9vlcsZ2/lB1L Q8BztXefIPfk5tCUQz70JYtu7NxALV+PgCBxG3ImTp+s2G00bctF1MZjFUP2AdYT4sSX 0mWtoqmxBcRgicnOhxedszA7bw/b5EObxzFincEu0w1XvtsV0ANVNlY8tfokygQSCJDE YHRA== Return-Path: <adhemerval.zanella@linaro.org> To: Carlos O'Donell <codonell@redhat.com>, libc-alpha@sourceware.org References: <20190416212713.24659-1-adhemerval.zanella@linaro.org> <20190416212713.24659-2-adhemerval.zanella@linaro.org> <c5448741-fbbe-c0f2-d5f4-e3ab1e5c87c9@redhat.com> From: Adhemerval Zanella <adhemerval.zanella@linaro.org> Openpgp: preference=signencrypt Subject: Re: [PATCH 2/2] elf: Fix pldd (BZ#18035) Message-ID: <da438731-e148-0097-cd30-76f8b4175ad2@linaro.org> Date: Wed, 17 Apr 2019 10:59:11 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <c5448741-fbbe-c0f2-d5f4-e3ab1e5c87c9@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit |
Commit Message
Adhemerval Zanella
April 17, 2019, 1:59 p.m. UTC
On 17/04/2019 01:05, Carlos O'Donell wrote: > On 4/16/19 5:27 PM, Adhemerval Zanella wrote: >> Since 9182aa67994 (Fix vDSO l_name for GDB's, BZ#387) the initial link_map >> for executable itself and loader will have both l_name and l_libname->name >> holding the same value due: > > Thanks for tracking this down! > > Please post v2: > - consider removal of more braces. > - add a comment where the old code was removed to warn against looking > at l_libname and the problem therein. > - consider adjusted comment. I will sent it, thanks for reviewing it. > >> >> elf/dl-object.c >> >> 95 new->l_name = *realname ? realname : (char *) newname->name + libname_len - 1; >> >> Since newname->name points to new->l_libname->name. >> >> This leads to pldd to an infinite call at: >> >> elf/pldd-xx.c >> >> 203 again: >> 204 while (1) >> 205 { >> 206 ssize_t n = pread64 (memfd, tmpbuf.data, tmpbuf.length, name_offset); >> >> 228 /* Try the l_libname element. */ >> 229 struct E(libname_list) ln; >> 230 if (pread64 (memfd, &ln, sizeof (ln), m.l_libname) == sizeof (ln)) >> 231 { >> 232 name_offset = ln.name; >> 233 goto again; >> 234 } >> >> Since the value at ln.name (l_libname->name) will be the same as previously >> read. The straightforward fix is just avoid the check and read the new list >> entry. >> >> I checked also against binaries issues with old loaders with fix for BZ#387, >> and pldd could dump the shared objects. >> >> Checked on x86_64-linux-gnu, i686-linux-gnu, aarch64-linux-gnu, and >> powerpc64le-linux-gnu. >> >> [BZ #18035] >> * elf/Makefile (tests-container): Add tst-pldd. >> * elf/pldd-xx.c: Use _Static_assert in of pldd_assert. >> (E(find_maps)): Avoid use alloca, use default read file operations >> instead of explicit LFS names, and fix infinite loop. >> * elf/pldd.c: Explicit set _FILE_OFFSET_BITS, cleanup headers. >> (get_process_info): Use _Static_assert instead of assert, use default >> directory operations instead of explicit LFS names, and free some >> leadek pointers. >> * elf/tst-pldd.c: New file. >> * elf/tst-pldd.root/tst-pldd.script: Likewise. > > This has a lot of unrelated cleanup to pldd which does much more than just > fix pldd. However, the cleanups are all OK IMO, and if someone needs to > backport just the required fix, they can do the removal of t Right, the fix itself is quite simple for a backport (more below). > > >> --- >> elf/Makefile | 1 + >> elf/pldd-xx.c | 106 ++++++++++----------------- >> elf/pldd.c | 64 ++++++++-------- >> elf/tst-pldd.c | 118 ++++++++++++++++++++++++++++++ >> elf/tst-pldd.root/tst-pldd.script | 1 + >> posix/tst-pldd.c | 9 +++ >> 6 files changed, 196 insertions(+), 103 deletions(-) >> create mode 100644 elf/tst-pldd.c >> create mode 100644 elf/tst-pldd.root/tst-pldd.script >> create mode 100644 posix/tst-pldd.c >> >> diff --git a/elf/Makefile b/elf/Makefile >> index 310a37cc13..0b4a877880 100644 >> --- a/elf/Makefile >> +++ b/elf/Makefile >> @@ -194,6 +194,7 @@ tests-internal += loadtest unload unload2 circleload1 \ >> tst-tls3 tst-tls6 tst-tls7 tst-tls8 tst-dlmopen2 \ >> tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym \ >> tst-create_format1 >> +tests-container += tst-pldd > > OK. > >> ifeq ($(build-hardcoded-path-in-tests),yes) >> tests += tst-dlopen-aout >> tst-dlopen-aout-no-pie = yes >> diff --git a/elf/pldd-xx.c b/elf/pldd-xx.c >> index 547f840ee1..53858b9932 100644 >> --- a/elf/pldd-xx.c >> +++ b/elf/pldd-xx.c >> @@ -23,10 +23,6 @@ >> #define EW_(e, w, t) EW__(e, w, _##t) >> #define EW__(e, w, t) e##w##t >> -#define pldd_assert(name, exp) \ >> - typedef int __assert_##name[((exp) != 0) - 1] >> - >> - > > OK. > >> struct E(link_map) >> { >> EW(Addr) l_addr; >> @@ -39,12 +35,12 @@ struct E(link_map) >> EW(Addr) l_libname; >> }; >> #if CLASS == __ELF_NATIVE_CLASS >> -pldd_assert (l_addr, (offsetof (struct link_map, l_addr) >> - == offsetof (struct E(link_map), l_addr))); >> -pldd_assert (l_name, (offsetof (struct link_map, l_name) >> - == offsetof (struct E(link_map), l_name))); >> -pldd_assert (l_next, (offsetof (struct link_map, l_next) >> - == offsetof (struct E(link_map), l_next))); >> +_Static_assert (offsetof (struct link_map, l_addr) >> + == offsetof (struct E(link_map), l_addr), "l_addr"); >> +_Static_assert (offsetof (struct link_map, l_name) >> + == offsetof (struct E(link_map), l_name), "l_name"); >> +_Static_assert (offsetof (struct link_map, l_next) >> + == offsetof (struct E(link_map), l_next), "l_next"); > > OK. > >> #endif >> @@ -54,10 +50,10 @@ struct E(libname_list) >> EW(Addr) next; >> }; >> #if CLASS == __ELF_NATIVE_CLASS >> -pldd_assert (name, (offsetof (struct libname_list, name) >> - == offsetof (struct E(libname_list), name))); >> -pldd_assert (next, (offsetof (struct libname_list, next) >> - == offsetof (struct E(libname_list), next))); >> +_Static_assert (offsetof (struct libname_list, name) >> + == offsetof (struct E(libname_list), name), "name"); >> +_Static_assert (offsetof (struct libname_list, next) >> + == offsetof (struct E(libname_list), next), "next"); > > OK. > >> #endif >> struct E(r_debug) >> @@ -69,16 +65,17 @@ struct E(r_debug) >> EW(Addr) r_map; >> }; >> #if CLASS == __ELF_NATIVE_CLASS >> -pldd_assert (r_version, (offsetof (struct r_debug, r_version) >> - == offsetof (struct E(r_debug), r_version))); >> -pldd_assert (r_map, (offsetof (struct r_debug, r_map) >> - == offsetof (struct E(r_debug), r_map))); >> +_Static_assert (offsetof (struct r_debug, r_version) >> + == offsetof (struct E(r_debug), r_version), "r_version"); >> +_Static_assert (offsetof (struct r_debug, r_map) >> + == offsetof (struct E(r_debug), r_map), "r_map"); > > OK. > >> #endif >> static int >> -E(find_maps) (pid_t pid, void *auxv, size_t auxv_size) >> +E(find_maps) (const char *exe, int memfd, pid_t pid, void *auxv, >> + size_t auxv_size) >> { >> EW(Addr) phdr = 0; >> unsigned int phnum = 0; >> @@ -104,12 +101,9 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size) >> if (phdr == 0 || phnum == 0 || phent == 0) >> error (EXIT_FAILURE, 0, gettext ("cannot find program header of process")); >> - EW(Phdr) *p = alloca (phnum * phent); >> - if (pread64 (memfd, p, phnum * phent, phdr) != phnum * phent) >> - { >> - error (0, 0, gettext ("cannot read program header")); >> - return EXIT_FAILURE; >> - } >> + EW(Phdr) *p = xmalloc (phnum * phent); >> + if (pread (memfd, p, phnum * phent, phdr) != phnum * phent) >> + error (EXIT_FAILURE, 0, gettext ("cannot read program header")); > > OK. > >> /* Determine the load offset. We need this for interpreting the >> other program header entries so we do this in a separate loop. >> @@ -129,11 +123,10 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size) >> if (p[i].p_type == PT_DYNAMIC) >> { >> EW(Dyn) *dyn = xmalloc (p[i].p_filesz); >> - if (pread64 (memfd, dyn, p[i].p_filesz, offset + p[i].p_vaddr) >> + if (pread (memfd, dyn, p[i].p_filesz, offset + p[i].p_vaddr) > > OK. > >> != p[i].p_filesz) >> { >> - error (0, 0, gettext ("cannot read dynamic section")); >> - return EXIT_FAILURE; >> + error (EXIT_FAILURE, 0, gettext ("cannot read dynamic section")); > > OK. Consider removing braces? Ack. > >> } >> /* Search for the DT_DEBUG entry. */ >> @@ -141,11 +134,10 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size) >> if (dyn[j].d_tag == DT_DEBUG && dyn[j].d_un.d_ptr != 0) >> { >> struct E(r_debug) r; >> - if (pread64 (memfd, &r, sizeof (r), dyn[j].d_un.d_ptr) >> + if (pread (memfd, &r, sizeof (r), dyn[j].d_un.d_ptr) > > OK. > >> != sizeof (r)) >> { >> - error (0, 0, gettext ("cannot read r_debug")); >> - return EXIT_FAILURE; >> + error (EXIT_FAILURE, 0, gettext ("cannot read r_debug")); > > OK. Consider removing braces? Ack. > >> } >> if (r.r_map != 0) >> @@ -160,12 +152,11 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size) >> } >> else if (p[i].p_type == PT_INTERP) >> { >> - interp = alloca (p[i].p_filesz); >> - if (pread64 (memfd, interp, p[i].p_filesz, offset + p[i].p_vaddr) >> + interp = xmalloc (p[i].p_filesz); >> + if (pread (memfd, interp, p[i].p_filesz, offset + p[i].p_vaddr) > > OK. > >> != p[i].p_filesz) >> { >> - error (0, 0, gettext ("cannot read program interpreter")); >> - return EXIT_FAILURE; >> + error (EXIT_FAILURE, 0, gettext ("cannot read program interpreter")); > > OK. Consider removing braces? Ack. > >> } >> } >> @@ -174,14 +165,16 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size) >> if (interp == NULL) >> { >> // XXX check whether the executable itself is the loader >> - return EXIT_FAILURE; >> + exit (EXIT_FAILURE); > > OK. > >> } >> // XXX perhaps try finding ld.so and _r_debug in it >> - >> - return EXIT_FAILURE; >> + exit (EXIT_FAILURE); > > OK. > >> } >> + free (p); >> + free (interp); > > OK. Both were changed from alloca to xmalloc. > >> + >> /* Print the PID and program name first. */ >> printf ("%lu:\t%s\n", (unsigned long int) pid, exe); >> @@ -192,46 +185,22 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size) >> do >> { >> struct E(link_map) m; >> - if (pread64 (memfd, &m, sizeof (m), list) != sizeof (m)) >> - { >> - error (0, 0, gettext ("cannot read link map")); >> - status = EXIT_FAILURE; >> - goto out; >> - } >> + if (pread (memfd, &m, sizeof (m), list) != sizeof (m)) >> + error (EXIT_FAILURE, 0, gettext ("cannot read link map")); > > OK. > >> EW(Addr) name_offset = m.l_name; >> - again: >> while (1) >> { >> - ssize_t n = pread64 (memfd, tmpbuf.data, tmpbuf.length, name_offset); >> + ssize_t n = pread (memfd, tmpbuf.data, tmpbuf.length, name_offset); >> if (n == -1) >> - { >> - error (0, 0, gettext ("cannot read object name")); >> - status = EXIT_FAILURE; >> - goto out; >> - } >> + error (EXIT_FAILURE, 0, gettext ("cannot read object name")); > > OK. > >> if (memchr (tmpbuf.data, '\0', n) != NULL) >> break; >> if (!scratch_buffer_grow (&tmpbuf)) >> - { >> - error (0, 0, gettext ("cannot allocate buffer for object name")); >> - status = EXIT_FAILURE; >> - goto out; >> - } >> - } >> - >> - if (((char *)tmpbuf.data)[0] == '\0' && name_offset == m.l_name >> - && m.l_libname != 0) >> - { >> - /* Try the l_libname element. */ >> - struct E(libname_list) ln; >> - if (pread64 (memfd, &ln, sizeof (ln), m.l_libname) == sizeof (ln)) >> - { >> - name_offset = ln.name; >> - goto again; >> - } >> + error (EXIT_FAILURE, 0, >> + gettext ("cannot allocate buffer for object name")); > > This is the real fix right? We remove the iteration into l_libname? > Why was this code ever needed in the first place? Yes, the core issue is l_name and l_libname->name for loaders linkmap points to same value since BZ#387 fix. This makes 'm.l_libname' being the same address as the m.l_name, thus creating an infinite loop. A more self-contained patch would be: --- --- > >> } >> /* Skip over the executable. */ >> @@ -242,7 +211,6 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size) >> } >> while (list != 0); >> - out: > > OK. > >> scratch_buffer_free (&tmpbuf); >> return status; >> } >> diff --git a/elf/pldd.c b/elf/pldd.c >> index f3fac4e487..69629bd5d2 100644 >> --- a/elf/pldd.c >> +++ b/elf/pldd.c >> @@ -17,23 +17,17 @@ >> License along with the GNU C Library; if not, see >> <http://www.gnu.org/licenses/>. */ >> -#include <alloca.h> >> +#define _FILE_OFFSET_BITS 64 >> + >> #include <argp.h> >> -#include <assert.h> >> #include <dirent.h> >> -#include <elf.h> >> -#include <errno.h> >> #include <error.h> >> #include <fcntl.h> >> #include <libintl.h> >> -#include <link.h> >> -#include <stddef.h> >> #include <stdio.h> >> #include <stdlib.h> >> -#include <string.h> >> #include <unistd.h> >> #include <sys/ptrace.h> >> -#include <sys/stat.h> > > OK. > >> #include <sys/wait.h> >> #include <scratch_buffer.h> >> @@ -76,14 +70,9 @@ static struct argp argp = >> options, parse_opt, args_doc, doc, NULL, more_help, NULL >> }; >> -// File descriptor of /proc/*/mem file. >> -static int memfd; >> - >> -/* Name of the executable */ >> -static char *exe; > > OK. > >> /* Local functions. */ >> -static int get_process_info (int dfd, long int pid); >> +static int get_process_info (const char *exe, int dfd, long int pid); > > OK. > >> static void wait_for_ptrace_stop (long int pid); >> @@ -102,8 +91,10 @@ main (int argc, char *argv[]) >> return 1; >> } >> - assert (sizeof (pid_t) == sizeof (int) >> - || sizeof (pid_t) == sizeof (long int)); >> + _Static_assert (sizeof (pid_t) == sizeof (int) >> + || sizeof (pid_t) == sizeof (long int), >> + "sizeof (pid_t) != sizeof (int) or sizeof (long int)"); > > OK. > >> + >> char *endp; >> errno = 0; >> long int pid = strtol (argv[remaining], &endp, 10); >> @@ -119,25 +110,24 @@ main (int argc, char *argv[]) >> if (dfd == -1) >> error (EXIT_FAILURE, errno, gettext ("cannot open %s"), buf); >> - struct scratch_buffer exebuf; >> - scratch_buffer_init (&exebuf); >> + /* Name of the executable */ >> + struct scratch_buffer exe; >> + scratch_buffer_init (&exe); >> ssize_t nexe; >> while ((nexe = readlinkat (dfd, "exe", >> - exebuf.data, exebuf.length)) == exebuf.length) >> + exe.data, exe.length)) == exe.length) > > OK. > >> { >> - if (!scratch_buffer_grow (&exebuf)) >> + if (!scratch_buffer_grow (&exe)) > > OK. > >> { >> nexe = -1; >> break; >> } >> } >> if (nexe == -1) >> - exe = (char *) "<program name undetermined>"; >> + /* Default stack allocation is at least 1024. */ >> + snprintf (exe.data, exe.length, "<program name undetermined>"); > > OK. > >> else >> - { >> - exe = exebuf.data; >> - exe[nexe] = '\0'; >> - } >> + ((char*)exe.data)[nexe] = '\0'; > > OK. > >> /* Stop all threads since otherwise the list of loaded modules might >> change while we are reading it. */ >> @@ -155,8 +145,8 @@ main (int argc, char *argv[]) >> error (EXIT_FAILURE, errno, gettext ("cannot prepare reading %s/task"), >> buf); >> - struct dirent64 *d; >> - while ((d = readdir64 (dir)) != NULL) >> + struct dirent *d; >> + while ((d = readdir (dir)) != NULL) >> { >> if (! isdigit (d->d_name[0])) >> continue; >> @@ -182,7 +172,7 @@ main (int argc, char *argv[]) >> wait_for_ptrace_stop (tid); >> - struct thread_list *newp = alloca (sizeof (*newp)); >> + struct thread_list *newp = xmalloc (sizeof (*newp)); > > OK. > >> newp->tid = tid; >> newp->next = thread_list; >> thread_list = newp; >> @@ -190,17 +180,22 @@ main (int argc, char *argv[]) >> closedir (dir); >> - int status = get_process_info (dfd, pid); >> + if (thread_list == NULL) >> + error (EXIT_FAILURE, 0, gettext ("no valid %s/task entries"), buf); >> + >> + int status = get_process_info (exe.data, dfd, pid); > > OK. > >> - assert (thread_list != NULL); >> do >> { >> ptrace (PTRACE_DETACH, thread_list->tid, NULL, NULL); >> + struct thread_list *prev = thread_list; >> thread_list = thread_list->next; >> + free (prev); > > OK. > >> } >> while (thread_list != NULL); >> close (dfd); >> + scratch_buffer_free (&exe); > > OK. > >> return status; >> } >> @@ -281,9 +276,10 @@ warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.\n\ >> static int >> -get_process_info (int dfd, long int pid) >> +get_process_info (const char *exe, int dfd, long int pid) >> { >> - memfd = openat (dfd, "mem", O_RDONLY); >> + /* File descriptor of /proc/<pid>/mem file. */ >> + int memfd = openat (dfd, "mem", O_RDONLY); >> if (memfd == -1) >> goto no_info; >> @@ -333,9 +329,9 @@ get_process_info (int dfd, long int pid) >> int retval; >> if (e_ident[EI_CLASS] == ELFCLASS32) >> - retval = find_maps32 (pid, auxv, auxv_size); >> + retval = find_maps32 (exe, memfd, pid, auxv, auxv_size); >> else >> - retval = find_maps64 (pid, auxv, auxv_size); >> + retval = find_maps64 (exe, memfd, pid, auxv, auxv_size); > > OK. > >> free (auxv); >> close (memfd); >> diff --git a/elf/tst-pldd.c b/elf/tst-pldd.c >> new file mode 100644 >> index 0000000000..9b568ca4eb >> --- /dev/null >> +++ b/elf/tst-pldd.c >> @@ -0,0 +1,118 @@ >> +/* Basic tests for pldd program. > > OK. > >> + Copyright (C) 2019 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 <stdio.h> >> +#include <string.h> >> +#include <unistd.h> >> +#include <stdint.h> >> +#include <libgen.h> >> +#include <stdbool.h> >> + >> +#include <array_length.h> >> +#include <gnu/lib-names.h> >> + >> +#include <support/subprocess.h> >> +#include <support/capture_subprocess.h> >> +#include <support/check.h> >> + >> +static void >> +target_process (void *arg) >> +{ >> + pause (); >> +} >> + >> +/* The test runs on container because currently pldd does not support trace >> + a binary issues with loader itself (as with testrun.sh). */ > > Suggest: > > /* The test runs in a container because pldd does not support tracing > a binary started by the loader iself (as with testrun.sh). */ Ack. > >> + >> +static int >> +do_test (void) >> +{ >> + /* Create a copy of current test to check with pldd. */ >> + struct support_subprocess target = support_subprocess (target_process, NULL); >> + >> + /* Run 'pldd' on test subprocess. */ >> + struct support_capture_subprocess pldd; >> + { >> + /* Three digits per byte plus null terminator. */ >> + char pid[3 * sizeof (uint32_t) + 1]; >> + snprintf (pid, array_length (pid), "%d", target.pid); >> + >> + const char prog[] = "/usr/bin/pldd"; >> + >> + pldd = support_capture_subprogram (prog, >> + (char *const []) { (char *) prog, pid, NULL }); >> + >> + support_capture_subprocess_check (&pldd, "pldd", 0, sc_allow_stdout); >> + } >> + >> + /* Check 'pldd' output. The test is expected to be linked against only >> + loader and libc. */ >> + { >> + pid_t pid; >> + char buffer[512]; >> +#define STRINPUT(size) "%" # size "s" >> + >> + FILE *out = fmemopen (pldd.out.buffer, pldd.out.length, "r"); >> + TEST_VERIFY (out != NULL); >> + >> + /* First line is in the form of <pid>: <full path of executable> */ >> + TEST_COMPARE (fscanf (out, "%u: " STRINPUT (512), &pid, buffer), 2); >> + >> + TEST_COMPARE (pid, target.pid); >> + TEST_COMPARE (strcmp (basename (buffer), "tst-pldd"), 0); >> + >> + /* It expects only one loader and libc loaded by the program. */ >> + bool interpreter_found = false, libc_found = false; >> + while (fgets (buffer, array_length (buffer), out) != NULL) >> + { >> + /* Ignore vDSO. */ >> + if (buffer[0] != '/') >> + continue; >> + >> + /* Remove newline so baseline (buffer) can compare against the >> + LD_SO and LIBC_SO macros unmodified. */ >> + if (buffer[strlen(buffer)-1] == '\n') >> + buffer[strlen(buffer)-1] = '\0'; >> + >> + if (strcmp (basename (buffer), LD_SO) == 0) >> + { >> + TEST_COMPARE (interpreter_found, false); >> + interpreter_found = true; >> + continue; >> + } >> + >> + if (strcmp (basename (buffer), LIBC_SO) == 0) >> + { >> + TEST_COMPARE (libc_found, false); >> + libc_found = true; >> + continue; >> + } >> + } >> + TEST_COMPARE (interpreter_found, true); >> + TEST_COMPARE (libc_found, true); >> + >> + fclose (out); >> + } >> + >> + support_capture_subprocess_free (&pldd); >> + support_process_terminate (&target); >> + >> + return 0; >> +} >> + >> +#include <support/test-driver.c> > > OK. > >> diff --git a/elf/tst-pldd.root/tst-pldd.script b/elf/tst-pldd.root/tst-pldd.script >> new file mode 100644 >> index 0000000000..5a197b2ed0 >> --- /dev/null >> +++ b/elf/tst-pldd.root/tst-pldd.script >> @@ -0,0 +1 @@ >> +cp $B/elf/pldd $I/bin/pldd > > OK. > >> diff --git a/posix/tst-pldd.c b/posix/tst-pldd.c >> new file mode 100644 >> index 0000000000..edff4b9d07 >> --- /dev/null >> +++ b/posix/tst-pldd.c >> @@ -0,0 +1,9 @@ >> +#include <unistd.h> >> +#include <stdio.h> >> + >> +int main (int argc, char *argv[]) >> +{ >> + printf ("pid=%d\n", (int) getpid()); >> + pause (); >> + return 0; >> +} > > OK. This is in fact an artifact that should not be on this patch, I will remove it.
diff --git a/elf/pldd-xx.c b/elf/pldd-xx.c index 547f840ee1..995a731d43 100644 --- a/elf/pldd-xx.c +++ b/elf/pldd-xx.c @@ -200,7 +200,6 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size) } EW(Addr) name_offset = m.l_name; - again: while (1) { ssize_t n = pread64 (memfd, tmpbuf.data, tmpbuf.length, name_offset); @@ -222,18 +221,6 @@ E(find_maps) (pid_t pid, void *auxv, size_t auxv_size) } } - if (((char *)tmpbuf.data)[0] == '\0' && name_offset == m.l_name - && m.l_libname != 0) - { - /* Try the l_libname element. */ - struct E(libname_list) ln; - if (pread64 (memfd, &ln, sizeof (ln), m.l_libname) == sizeof (ln)) - { - name_offset = ln.name; - goto again; - } - } - /* Skip over the executable. */ if (((char *)tmpbuf.data)[0] != '\0') printf ("%s\n", (char *)tmpbuf.data);