Message ID | MWHPR2101MB073287753F9E8D211AF49DD3E8DA0@MWHPR2101MB0732.namprd21.prod.outlook.com |
---|---|
State | Committed |
Headers |
Received: (qmail 116178 invoked by alias); 5 Aug 2019 21:56:07 -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 116170 invoked by uid 89); 5 Aug 2019 21:56:06 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy= X-HELO: NAM03-CO1-obe.outbound.protection.outlook.com ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=PB8XqFtIEnfz3IGxgEQplhQrmpWHtIWLBwWp9dl5ezSMEU2luKboWzF7Z8CX7L8XOk3fNrjJily1bKM+szL82ilzNegxY1JVN4vWdGVwXQTjTnCu4Vtth49pUEIVpXaayCSSZ7dCB49uGmEHma9cbYBQtqs6HBBR7BKf8Okn+eSrxvclB7ZZ81Ld1KeskrzrBME/VnGR73hKPAVUk8LzpzUZYfcUwOJ2TPxrAOVrHYp+YcmzpSzNqJurBfXAjb9TSMnxjTza33sjubenyARP/GO1e+P0kAdxC0ciBkhhRXg2E4IbE2pVU7FEz+lkqFiRN01xmwhFak8xJciKClV11w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=O22ROZjOWstbdLYaavw746AHHNmkFgkxnmdWlj5ha14=; b=FEZNK+c5PFsOTGooWKfrrr95gWdvLZXGQwKSWkvb2PWnFIsWMLqqrC/CAYbj9DCZFa+FjCPtpiTEdfCm3Zo22aV6JaG5lGv0gCyR1YphywTS3CbgQRsfNYjS9V2Y1U8Teso7FJP9vvWnAC7kBz9E8emgkk8aKkfCUZgf9JV+aqo5vlnK6KuykVyGaAbNFA0x7FCiVuixREvCeOUJw5wylB2XeApnex7ZnP6t8WHDETpfdIGDVewm2wgOIaCKKNek28XUsNCwYeHBVjgTrfAg72bYcaXyAXnZ+r97gQ9LObkxefKf18rrkfI+sf05C6nuAOHwgft4wp8subP89rzyug== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=microsoft.com; dmarc=pass action=none header.from=microsoft.com; dkim=pass header.d=microsoft.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=O22ROZjOWstbdLYaavw746AHHNmkFgkxnmdWlj5ha14=; b=V1MWsELWzmvapcO6mfGdlu5KEd4ZizRwmIs++xIhOYyTlOiEa0xDCvL7aavQsEkrrE9q7kAGes6+j2vKUvyebqmO3BP5MULlgFZlJXWVkYxW9RiZ0Xx6p375jtlyWWqphZVveGaG5DCiiTg0S/nJabIlgelH2wcFFD4m3RLBWbY= From: Leandro Pereira <Leandro.Pereira@microsoft.com> To: "libc-alpha@sourceware.org" <libc-alpha@sourceware.org> Subject: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read() Date: Mon, 5 Aug 2019 21:56:03 +0000 Message-ID: <MWHPR2101MB073287753F9E8D211AF49DD3E8DA0@MWHPR2101MB0732.namprd21.prod.outlook.com> authentication-results: spf=none (sender IP is ) smtp.mailfrom=Leandro.Pereira@microsoft.com; x-ms-oob-tlc-oobclassifiers: OLM:1443; received-spf: None (protection.outlook.com: microsoft.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-ms-exchange-transport-forked: True Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: Emgg1wxuqW20iy/11si/edqDRb9TjAMPZ3tUmXZ1JITQI89E8f/7hBegPL3aYKRjxUwBho+Nfg490hIrfXA6YQ== |
Commit Message
Leandro Pereira
Aug. 5, 2019, 9:56 p.m. UTC
Transforms this, when linking in a shared object: openat(AT_FDCWD, "/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 read(3, "\177ELF\2\1\1\3"..., 832) = 832 lseek(3, 792, SEEK_SET) = 792 read(3, "\4\0\0\0\24\0\0\0"..., 68) = 68 fstat(3, {st_mode=S_IFREG|0755, st_size=6699224, ...}) = 0 lseek(3, 792, SEEK_SET) = 792 read(3, "\4\0\0\0\24\0\0\0"..., 68) = 68 lseek(3, 864, SEEK_SET) = 864 read(3, "\4\0\0\0\20\0\0\0"..., 32) = 32 Into this: openat(AT_FDCWD, "/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 read(3, "\177ELF\2\1\1\3"..., 832) = 832 pread(3, "\4\0\0\0\24\0\0\0"..., 68, 792) = 68 fstat(3, {st_mode=S_IFREG|0755, st_size=6699224, ...}) = 0 pread(3, "\4\0\0\0\24\0\0\0"..., 68, 792) = 68 pread(3, "\4\0\0\0\20\0\0\0"..., 32, 864) = 32 2019-08-05 Leandro Pereira <leandro.pereira@microsoft.com> * elf/dl-load.c: Use __pread64_nocancel() instead of __lseek()+ __read_nocancel(). * sysdeps/x86/dl-prop.h: Likewise. --- elf/dl-load.c | 9 +++------ sysdeps/x86/dl-prop.h | 3 +-- 2 files changed, 4 insertions(+), 8 deletions(-)
Comments
On 8/5/19 5:56 PM, Leandro Pereira wrote: > Transforms this, when linking in a shared object: > > openat(AT_FDCWD, "/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 > read(3, "\177ELF\2\1\1\3"..., 832) = 832 > lseek(3, 792, SEEK_SET) = 792 > read(3, "\4\0\0\0\24\0\0\0"..., 68) = 68 > fstat(3, {st_mode=S_IFREG|0755, st_size=6699224, ...}) = 0 > lseek(3, 792, SEEK_SET) = 792 > read(3, "\4\0\0\0\24\0\0\0"..., 68) = 68 > lseek(3, 864, SEEK_SET) = 864 > read(3, "\4\0\0\0\20\0\0\0"..., 32) = 32 > > Into this: > > openat(AT_FDCWD, "/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 > read(3, "\177ELF\2\1\1\3"..., 832) = 832 > pread(3, "\4\0\0\0\24\0\0\0"..., 68, 792) = 68 > fstat(3, {st_mode=S_IFREG|0755, st_size=6699224, ...}) = 0 > pread(3, "\4\0\0\0\24\0\0\0"..., 68, 792) = 68 > pread(3, "\4\0\0\0\20\0\0\0"..., 32, 864) = 32 > > 2019-08-05 Leandro Pereira <leandro.pereira@microsoft.com> > > * elf/dl-load.c: Use __pread64_nocancel() instead of __lseek()+ > __read_nocancel(). > * sysdeps/x86/dl-prop.h: Likewise. OK for master. I'll push after testing with bmg. No regressions on x86_64. Reviewed-by: Carlos O'Donell <carlos@redhat.com> > --- > elf/dl-load.c | 9 +++------ > sysdeps/x86/dl-prop.h | 3 +-- > 2 files changed, 4 insertions(+), 8 deletions(-) > > diff --git a/elf/dl-load.c b/elf/dl-load.c > index 5abeb867f1..462c425a13 100644 > --- a/elf/dl-load.c > +++ b/elf/dl-load.c > @@ -1005,8 +1005,7 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd, > else > { OK. I reviewed that all uses of fd did not depend on the SEEK_SET being set to a given position. > phdr = alloca (maplength); > - __lseek (fd, header->e_phoff, SEEK_SET); > - if ((size_t) __read_nocancel (fd, (void *) phdr, maplength) != maplength) > + if ((size_t) __pread64_nocancel (fd, (void *) phdr, maplength, header->e_phoff) != maplength) This line is too long, but I'll wrap it before commit, likewise for 2 other lines. > { > errstring = N_("cannot read file data"); > goto call_lose_errno; > @@ -1659,8 +1658,7 @@ open_verify (const char *name, int fd, > else > { > phdr = alloca (maplength); > - __lseek (fd, ehdr->e_phoff, SEEK_SET); > - if ((size_t) __read_nocancel (fd, (void *) phdr, maplength) > + if ((size_t) __pread64_nocancel (fd, (void *) phdr, maplength, ehdr->e_phoff) OK. Wrapped. > != maplength) > { > read_error: > @@ -1710,8 +1708,7 @@ open_verify (const char *name, int fd, > > abi_note = abi_note_malloced; > } > - __lseek (fd, ph->p_offset, SEEK_SET); > - if (__read_nocancel (fd, (void *) abi_note, size) != size) > + if (__pread64_nocancel (fd, (void *) abi_note, size, ph->p_offset) != size) OK. Wrapped. > { > free (abi_note_malloced); > goto read_error; > diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h > index 1b335ccbb3..080d66a971 100644 > --- a/sysdeps/x86/dl-prop.h > +++ b/sysdeps/x86/dl-prop.h > @@ -167,8 +167,7 @@ _dl_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph, > note_malloced = malloc (size); > note = note_malloced; > } > - __lseek (fd, ph->p_offset, SEEK_SET); > - if (__read_nocancel (fd, (void *) note, size) != size) > + if (__pread64_nocancel (fd, (void *) note, size, ph->p_offset) != size) OK. > { > if (note_malloced) > free (note_malloced); >
Hi, Le lundi 05 août 2019 à 21:56 +0000, Leandro Pereira a écrit : > Transforms this, when linking in a shared object: > > openat(AT_FDCWD, "/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 > read(3, "\177ELF\2\1\1\3"..., 832) = 832 > lseek(3, 792, SEEK_SET) = 792 > read(3, "\4\0\0\0\24\0\0\0"..., 68) = 68 > fstat(3, {st_mode=S_IFREG|0755, st_size=6699224, ...}) = 0 > lseek(3, 792, SEEK_SET) = 792 > read(3, "\4\0\0\0\24\0\0\0"..., 68) = 68 > lseek(3, 864, SEEK_SET) = 864 > read(3, "\4\0\0\0\20\0\0\0"..., 32) = 32 > > Into this: > > openat(AT_FDCWD, "/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 > read(3, "\177ELF\2\1\1\3"..., 832) = 832 > pread(3, "\4\0\0\0\24\0\0\0"..., 68, 792) = 68 > fstat(3, {st_mode=S_IFREG|0755, st_size=6699224, ...}) = 0 > pread(3, "\4\0\0\0\24\0\0\0"..., 68, 792) = 68 > pread(3, "\4\0\0\0\20\0\0\0"..., 32, 864) = 32 > I confirm the behavior. And the usefulness of the patch. Anyway I'm quite surprised ld.so is reading twice 68 bytes at the same 792 offset. Moreover, the first read already brought 28 bytes out of 68. Could these read() be replaced by a call to mmap(), then using mremap() to brought the shared object in memory after initial inspection ? Regards.
On 10/3/19 11:31 AM, Yann Droneaud wrote: > Hi, > > Le lundi 05 août 2019 à 21:56 +0000, Leandro Pereira a écrit : >> Transforms this, when linking in a shared object: >> >> openat(AT_FDCWD, "/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 >> read(3, "\177ELF\2\1\1\3"..., 832) = 832 >> lseek(3, 792, SEEK_SET) = 792 >> read(3, "\4\0\0\0\24\0\0\0"..., 68) = 68 >> fstat(3, {st_mode=S_IFREG|0755, st_size=6699224, ...}) = 0 >> lseek(3, 792, SEEK_SET) = 792 >> read(3, "\4\0\0\0\24\0\0\0"..., 68) = 68 >> lseek(3, 864, SEEK_SET) = 864 >> read(3, "\4\0\0\0\20\0\0\0"..., 32) = 32 >> >> Into this: >> >> openat(AT_FDCWD, "/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 >> read(3, "\177ELF\2\1\1\3"..., 832) = 832 >> pread(3, "\4\0\0\0\24\0\0\0"..., 68, 792) = 68 >> fstat(3, {st_mode=S_IFREG|0755, st_size=6699224, ...}) = 0 >> pread(3, "\4\0\0\0\24\0\0\0"..., 68, 792) = 68 >> pread(3, "\4\0\0\0\20\0\0\0"..., 32, 864) = 32 >> > > I confirm the behavior. And the usefulness of the patch. > > Anyway I'm quite surprised ld.so is reading twice 68 bytes at the same > 792 offset. Moreover, the first read already brought 28 bytes out of > 68. > > Could these read() be replaced by a call to mmap(), then using mremap() > to brought the shared object in memory after initial inspection ? It might be possible. It's an optimization that would probably require some refactoring and review of the current logic in the DSO loading. Happy to review patches :-)
On Okt 03 2019, Yann Droneaud <ydroneaud@opteya.com> wrote: > Anyway I'm quite surprised ld.so is reading twice 68 bytes at the same > 792 offset. Moreover, the first read already brought 28 bytes out of > 68. That only happens if you have a note segment that does not fit in the initial file buffer. That is rather unusual. Andreas.
Hi, Le jeudi 03 octobre 2019 à 19:59 +0200, Andreas Schwab a écrit : > On Okt 03 2019, Yann Droneaud <ydroneaud@opteya.com> wrote: > > > Anyway I'm quite surprised ld.so is reading twice 68 bytes at the > > same > > 792 offset. Moreover, the first read already brought 28 bytes out > > of > > 68. > > That only happens if you have a note segment that does not fit in the > initial file buffer. That is rather unusual. > Not sure about it being unusual: on Fedora 30, x86_64, I see this behavior with /bin/false /bin/true /bin/test /bin/uname openat(AT_FDCWD, "/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 read(3, "\177ELF\2\1\1\3"..., 832) = 832 lseek(3, 792, SEEK_SET) = 792 read(3, "\4\0\0\0\24\0\0\0"..., 68) = 68 fstat(3, {st_mode=S_IFREG|0755, st_size=6697832, ...}) = 0 mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f8bc04b5000 lseek(3, 792, SEEK_SET) = 792 read(3, "\4\0\0\0\24\0\0\0"..., 68) = 68 lseek(3, 864, SEEK_SET) = 864 read(3, "\4\0\0\0\20\0\0\0"..., 32) = 32 $ file /bin/false /bin/true /bin/test /bin/uname /bin/false: ELF 64-bit LSB pie executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 3.2.0, BuildID[sha1]=790d23abaaa68bb496873f5afc647dd6be30f94c, stripped, too many notes (256) /bin/true: ELF 64-bit LSB pie executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 3.2.0, BuildID[sha1]=8e348e43f020cfa72b8abed7e69b54558165fb2e, stripped, too many notes (256) /bin/test: ELF 64-bit LSB pie executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 3.2.0, BuildID[sha1]=5dee19457e437541fe30f58d3d4add118567e806, stripped, too many notes (256) /bin/uname: ELF 64-bit LSB pie executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 3.2.0, BuildID[sha1]=50b398bf94ad5a081eb4178d976d4f290977f99a, stripped, too many notes (256) Too many notes ? Regards.
On Okt 03 2019, Yann Droneaud <ydroneaud@opteya.com> wrote: > Not sure about it being unusual: on Fedora 30, x86_64, I see this > behavior with /bin/false /bin/true /bin/test /bin/uname I'd say that's a bug in Fedora, then. Andreas.
On 10/3/19 9:59 AM, Carlos O'Donell wrote: > On 8/5/19 5:56 PM, Leandro Pereira wrote: >> Transforms this, when linking in a shared object: >> >> openat(AT_FDCWD, "/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 >> read(3, "\177ELF\2\1\1\3"..., 832) = 832 >> lseek(3, 792, SEEK_SET) = 792 >> read(3, "\4\0\0\0\24\0\0\0"..., 68) = 68 >> fstat(3, {st_mode=S_IFREG|0755, st_size=6699224, ...}) = 0 >> lseek(3, 792, SEEK_SET) = 792 >> read(3, "\4\0\0\0\24\0\0\0"..., 68) = 68 >> lseek(3, 864, SEEK_SET) = 864 >> read(3, "\4\0\0\0\20\0\0\0"..., 32) = 32 >> >> Into this: >> >> openat(AT_FDCWD, "/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 >> read(3, "\177ELF\2\1\1\3"..., 832) = 832 >> pread(3, "\4\0\0\0\24\0\0\0"..., 68, 792) = 68 >> fstat(3, {st_mode=S_IFREG|0755, st_size=6699224, ...}) = 0 >> pread(3, "\4\0\0\0\24\0\0\0"..., 68, 792) = 68 >> pread(3, "\4\0\0\0\20\0\0\0"..., 32, 864) = 32 >> >> 2019-08-05 Leandro Pereira <leandro.pereira@microsoft.com> >> >> * elf/dl-load.c: Use __pread64_nocancel() instead of __lseek()+ >> __read_nocancel(). >> * sysdeps/x86/dl-prop.h: Likewise. > > OK for master. I'll push after testing with bmg. No regressions on x86_64. > > Reviewed-by: Carlos O'Donell <carlos@redhat.com> Pushed.
On Fri, 18 Oct 2019 16:02:22 -0400 Carlos O'Donell <carlos@redhat.com> wrote: > On 10/3/19 9:59 AM, Carlos O'Donell wrote: > > On 8/5/19 5:56 PM, Leandro Pereira wrote: > >> Transforms this, when linking in a shared object: > >> > >> openat(AT_FDCWD, "/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 > >> read(3, "\177ELF\2\1\1\3"..., 832) = 832 > >> lseek(3, 792, SEEK_SET) = 792 > >> read(3, "\4\0\0\0\24\0\0\0"..., 68) = 68 > >> fstat(3, {st_mode=S_IFREG|0755, st_size=6699224, ...}) = 0 > >> lseek(3, 792, SEEK_SET) = 792 > >> read(3, "\4\0\0\0\24\0\0\0"..., 68) = 68 > >> lseek(3, 864, SEEK_SET) = 864 > >> read(3, "\4\0\0\0\20\0\0\0"..., 32) = 32 > >> > >> Into this: > >> > >> openat(AT_FDCWD, "/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 > >> read(3, "\177ELF\2\1\1\3"..., 832) = 832 > >> pread(3, "\4\0\0\0\24\0\0\0"..., 68, 792) = 68 > >> fstat(3, {st_mode=S_IFREG|0755, st_size=6699224, ...}) = 0 > >> pread(3, "\4\0\0\0\24\0\0\0"..., 68, 792) = 68 > >> pread(3, "\4\0\0\0\20\0\0\0"..., 32, 864) = 32 > >> > >> 2019-08-05 Leandro Pereira <leandro.pereira@microsoft.com> > >> > >> * elf/dl-load.c: Use __pread64_nocancel() instead of > >> __lseek()+ __read_nocancel(). > >> * sysdeps/x86/dl-prop.h: Likewise. > > > > OK for master. I'll push after testing with bmg. No regressions on > > x86_64. > > > > Reviewed-by: Carlos O'Donell <carlos@redhat.com> > > Pushed. > Unfortunately, I've found a regression on HURD when using build-many-glibcs.py (for testing some other code): ../glibc-many-build/src/glibc/libio/../sysdeps/posix/libc_fatal.c:161: multiple definition of `__libc_fatal'; ../glibc-many-build/build/glibcs/i686-gnu/glibc/elf/dl-allobjs.os:/work/lukma/glibc/glibc-many-build/src/glibc/elf/dl-minimal.c\ :188: first defined here To reproduce: ../src/scripts/build-many-glibcs.py <path>/glibc/glibc-many-build glibcs i686-gnu emacs logs/glibcs/i686-gnu/004-glibcs-i686-gnu-build-log.txt Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On 10/20/19 5:37 PM, Lukasz Majewski wrote: > On Fri, 18 Oct 2019 16:02:22 -0400 > Carlos O'Donell <carlos@redhat.com> wrote: > >> On 10/3/19 9:59 AM, Carlos O'Donell wrote: >>> On 8/5/19 5:56 PM, Leandro Pereira wrote: >>>> Transforms this, when linking in a shared object: >>>> >>>> openat(AT_FDCWD, "/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 >>>> read(3, "\177ELF\2\1\1\3"..., 832) = 832 >>>> lseek(3, 792, SEEK_SET) = 792 >>>> read(3, "\4\0\0\0\24\0\0\0"..., 68) = 68 >>>> fstat(3, {st_mode=S_IFREG|0755, st_size=6699224, ...}) = 0 >>>> lseek(3, 792, SEEK_SET) = 792 >>>> read(3, "\4\0\0\0\24\0\0\0"..., 68) = 68 >>>> lseek(3, 864, SEEK_SET) = 864 >>>> read(3, "\4\0\0\0\20\0\0\0"..., 32) = 32 >>>> >>>> Into this: >>>> >>>> openat(AT_FDCWD, "/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 >>>> read(3, "\177ELF\2\1\1\3"..., 832) = 832 >>>> pread(3, "\4\0\0\0\24\0\0\0"..., 68, 792) = 68 >>>> fstat(3, {st_mode=S_IFREG|0755, st_size=6699224, ...}) = 0 >>>> pread(3, "\4\0\0\0\24\0\0\0"..., 68, 792) = 68 >>>> pread(3, "\4\0\0\0\20\0\0\0"..., 32, 864) = 32 >>>> >>>> 2019-08-05 Leandro Pereira <leandro.pereira@microsoft.com> >>>> >>>> * elf/dl-load.c: Use __pread64_nocancel() instead of >>>> __lseek()+ __read_nocancel(). >>>> * sysdeps/x86/dl-prop.h: Likewise. >>> >>> OK for master. I'll push after testing with bmg. No regressions on >>> x86_64. >>> >>> Reviewed-by: Carlos O'Donell <carlos@redhat.com> >> >> Pushed. >> > > Unfortunately, I've found a regression on HURD when using > build-many-glibcs.py (for testing some other code): > > ../glibc-many-build/src/glibc/libio/../sysdeps/posix/libc_fatal.c:161: > multiple definition of `__libc_fatal'; > ../glibc-many-build/build/glibcs/i686-gnu/glibc/elf/dl-allobjs.os:/work/lukma/glibc/glibc-many-build/src/glibc/elf/dl-minimal.c\ > :188: first defined here > > To reproduce: > ../src/scripts/build-many-glibcs.py > <path>/glibc/glibc-many-build glibcs i686-gnu > > emacs logs/glibcs/i686-gnu/004-glibcs-i686-gnu-build-log.txt Thanks. I didn't see this. Let me start another build.
* Carlos O'Donell: > On 10/20/19 5:37 PM, Lukasz Majewski wrote: >> On Fri, 18 Oct 2019 16:02:22 -0400 >> Carlos O'Donell <carlos@redhat.com> wrote: >> >>> On 10/3/19 9:59 AM, Carlos O'Donell wrote: >>>> On 8/5/19 5:56 PM, Leandro Pereira wrote: >>>>> Transforms this, when linking in a shared object: >>>>> >>>>> openat(AT_FDCWD, "/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 >>>>> read(3, "\177ELF\2\1\1\3"..., 832) = 832 >>>>> lseek(3, 792, SEEK_SET) = 792 >>>>> read(3, "\4\0\0\0\24\0\0\0"..., 68) = 68 >>>>> fstat(3, {st_mode=S_IFREG|0755, st_size=6699224, ...}) = 0 >>>>> lseek(3, 792, SEEK_SET) = 792 >>>>> read(3, "\4\0\0\0\24\0\0\0"..., 68) = 68 >>>>> lseek(3, 864, SEEK_SET) = 864 >>>>> read(3, "\4\0\0\0\20\0\0\0"..., 32) = 32 >>>>> >>>>> Into this: >>>>> >>>>> openat(AT_FDCWD, "/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 >>>>> read(3, "\177ELF\2\1\1\3"..., 832) = 832 >>>>> pread(3, "\4\0\0\0\24\0\0\0"..., 68, 792) = 68 >>>>> fstat(3, {st_mode=S_IFREG|0755, st_size=6699224, ...}) = 0 >>>>> pread(3, "\4\0\0\0\24\0\0\0"..., 68, 792) = 68 >>>>> pread(3, "\4\0\0\0\20\0\0\0"..., 32, 864) = 32 >>>>> >>>>> 2019-08-05 Leandro Pereira <leandro.pereira@microsoft.com> >>>>> >>>>> * elf/dl-load.c: Use __pread64_nocancel() instead of >>>>> __lseek()+ __read_nocancel(). >>>>> * sysdeps/x86/dl-prop.h: Likewise. >>>> >>>> OK for master. I'll push after testing with bmg. No regressions on >>>> x86_64. >>>> >>>> Reviewed-by: Carlos O'Donell <carlos@redhat.com> >>> >>> Pushed. >>> >> >> Unfortunately, I've found a regression on HURD when using >> build-many-glibcs.py (for testing some other code): >> >> ../glibc-many-build/src/glibc/libio/../sysdeps/posix/libc_fatal.c:161: >> multiple definition of `__libc_fatal'; >> ../glibc-many-build/build/glibcs/i686-gnu/glibc/elf/dl-allobjs.os:/work/lukma/glibc/glibc-many-build/src/glibc/elf/dl-minimal.c\ >> :188: first defined here >> >> To reproduce: >> ../src/scripts/build-many-glibcs.py >> <path>/glibc/glibc-many-build glibcs i686-gnu >> >> emacs logs/glibcs/i686-gnu/004-glibcs-i686-gnu-build-log.txt > > Thanks. I didn't see this. Let me start another build. I think I know what's going on and I'm looking to a fix. We shouldn't let such build regressions linger for so long.
On 10/24/19 8:08 AM, Florian Weimer wrote: > * Carlos O'Donell: > >> On 10/20/19 5:37 PM, Lukasz Majewski wrote: >>> On Fri, 18 Oct 2019 16:02:22 -0400 >>> Carlos O'Donell <carlos@redhat.com> wrote: >>> >>>> On 10/3/19 9:59 AM, Carlos O'Donell wrote: >>>>> On 8/5/19 5:56 PM, Leandro Pereira wrote: >>>>>> Transforms this, when linking in a shared object: >>>>>> >>>>>> openat(AT_FDCWD, "/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 >>>>>> read(3, "\177ELF\2\1\1\3"..., 832) = 832 >>>>>> lseek(3, 792, SEEK_SET) = 792 >>>>>> read(3, "\4\0\0\0\24\0\0\0"..., 68) = 68 >>>>>> fstat(3, {st_mode=S_IFREG|0755, st_size=6699224, ...}) = 0 >>>>>> lseek(3, 792, SEEK_SET) = 792 >>>>>> read(3, "\4\0\0\0\24\0\0\0"..., 68) = 68 >>>>>> lseek(3, 864, SEEK_SET) = 864 >>>>>> read(3, "\4\0\0\0\20\0\0\0"..., 32) = 32 >>>>>> >>>>>> Into this: >>>>>> >>>>>> openat(AT_FDCWD, "/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 >>>>>> read(3, "\177ELF\2\1\1\3"..., 832) = 832 >>>>>> pread(3, "\4\0\0\0\24\0\0\0"..., 68, 792) = 68 >>>>>> fstat(3, {st_mode=S_IFREG|0755, st_size=6699224, ...}) = 0 >>>>>> pread(3, "\4\0\0\0\24\0\0\0"..., 68, 792) = 68 >>>>>> pread(3, "\4\0\0\0\20\0\0\0"..., 32, 864) = 32 >>>>>> >>>>>> 2019-08-05 Leandro Pereira <leandro.pereira@microsoft.com> >>>>>> >>>>>> * elf/dl-load.c: Use __pread64_nocancel() instead of >>>>>> __lseek()+ __read_nocancel(). >>>>>> * sysdeps/x86/dl-prop.h: Likewise. >>>>> >>>>> OK for master. I'll push after testing with bmg. No regressions on >>>>> x86_64. >>>>> >>>>> Reviewed-by: Carlos O'Donell <carlos@redhat.com> >>>> >>>> Pushed. >>>> >>> >>> Unfortunately, I've found a regression on HURD when using >>> build-many-glibcs.py (for testing some other code): >>> >>> ../glibc-many-build/src/glibc/libio/../sysdeps/posix/libc_fatal.c:161: >>> multiple definition of `__libc_fatal'; >>> ../glibc-many-build/build/glibcs/i686-gnu/glibc/elf/dl-allobjs.os:/work/lukma/glibc/glibc-many-build/src/glibc/elf/dl-minimal.c\ >>> :188: first defined here >>> >>> To reproduce: >>> ../src/scripts/build-many-glibcs.py >>> <path>/glibc/glibc-many-build glibcs i686-gnu >>> >>> emacs logs/glibcs/i686-gnu/004-glibcs-i686-gnu-build-log.txt >> >> Thanks. I didn't see this. Let me start another build. > > I think I know what's going on and I'm looking to a fix. > > We shouldn't let such build regressions linger for so long. Sorry, you are right 4 days is too long. It speaks again for pre-commit CI since even straight forward things can have consequences like this. I ran b-m-g and didn't see this, but perhaps I made a mistake. At least twice I've botched the glibc branch checkout and tested b-m-g against upstream instead of upstream+patch. You and I have spoken about this off list and you have a patch, so I'm going to leave this with you to fix while I go review the CodeSourcery/Mentor patches.
* Carlos O'Donell: >>>> emacs logs/glibcs/i686-gnu/004-glibcs-i686-gnu-build-log.txt >>> >>> Thanks. I didn't see this. Let me start another build. >> >> I think I know what's going on and I'm looking to a fix. >> >> We shouldn't let such build regressions linger for so long. > > Sorry, you are right 4 days is too long. It speaks again for pre-commit > CI since even straight forward things can have consequences like this. > I ran b-m-g and didn't see this, but perhaps I made a mistake. At least > twice I've botched the glibc branch checkout and tested b-m-g against > upstream instead of upstream+patch. > > You and I have spoken about this off list and you have a patch, so I'm > going to leave this with you to fix while I go review the CodeSourcery/Mentor > patches. I posted a patch: <https://sourceware.org/ml/libc-alpha/2019-10/msg00706.html> Thanks, Florian
diff --git a/elf/dl-load.c b/elf/dl-load.c index 5abeb867f1..462c425a13 100644 --- a/elf/dl-load.c +++ b/elf/dl-load.c @@ -1005,8 +1005,7 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd, else { phdr = alloca (maplength); - __lseek (fd, header->e_phoff, SEEK_SET); - if ((size_t) __read_nocancel (fd, (void *) phdr, maplength) != maplength) + if ((size_t) __pread64_nocancel (fd, (void *) phdr, maplength, header->e_phoff) != maplength) { errstring = N_("cannot read file data"); goto call_lose_errno; @@ -1659,8 +1658,7 @@ open_verify (const char *name, int fd, else { phdr = alloca (maplength); - __lseek (fd, ehdr->e_phoff, SEEK_SET); - if ((size_t) __read_nocancel (fd, (void *) phdr, maplength) + if ((size_t) __pread64_nocancel (fd, (void *) phdr, maplength, ehdr->e_phoff) != maplength) { read_error: @@ -1710,8 +1708,7 @@ open_verify (const char *name, int fd, abi_note = abi_note_malloced; } - __lseek (fd, ph->p_offset, SEEK_SET); - if (__read_nocancel (fd, (void *) abi_note, size) != size) + if (__pread64_nocancel (fd, (void *) abi_note, size, ph->p_offset) != size) { free (abi_note_malloced); goto read_error; diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h index 1b335ccbb3..080d66a971 100644 --- a/sysdeps/x86/dl-prop.h +++ b/sysdeps/x86/dl-prop.h @@ -167,8 +167,7 @@ _dl_process_pt_note (struct link_map *l, const ElfW(Phdr) *ph, note_malloced = malloc (size); note = note_malloced; } - __lseek (fd, ph->p_offset, SEEK_SET); - if (__read_nocancel (fd, (void *) note, size) != size) + if (__pread64_nocancel (fd, (void *) note, size, ph->p_offset) != size) { if (note_malloced) free (note_malloced);