* [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()
@ 2019-08-05 21:56 Leandro Pereira
2019-10-03 13:59 ` Carlos O'Donell
2019-10-03 15:31 ` Yann Droneaud
0 siblings, 2 replies; 13+ messages in thread
From: Leandro Pereira @ 2019-08-05 21:56 UTC (permalink / raw)
To: libc-alpha
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(-)
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);
--
2.21.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()
2019-08-05 21:56 [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read() Leandro Pereira
@ 2019-10-03 13:59 ` Carlos O'Donell
2019-10-18 20:02 ` Carlos O'Donell
2019-10-03 15:31 ` Yann Droneaud
1 sibling, 1 reply; 13+ messages in thread
From: Carlos O'Donell @ 2019-10-03 13:59 UTC (permalink / raw)
To: Leandro Pereira, libc-alpha
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);
>
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()
2019-08-05 21:56 [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read() Leandro Pereira
2019-10-03 13:59 ` Carlos O'Donell
@ 2019-10-03 15:31 ` Yann Droneaud
2019-10-03 17:10 ` Carlos O'Donell
2019-10-03 17:59 ` Andreas Schwab
1 sibling, 2 replies; 13+ messages in thread
From: Yann Droneaud @ 2019-10-03 15:31 UTC (permalink / raw)
To: libc-alpha; +Cc: Leandro Pereira
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.
--
Yann Droneaud
OPTEYA
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()
2019-10-03 15:31 ` Yann Droneaud
@ 2019-10-03 17:10 ` Carlos O'Donell
2019-10-03 17:59 ` Andreas Schwab
1 sibling, 0 replies; 13+ messages in thread
From: Carlos O'Donell @ 2019-10-03 17:10 UTC (permalink / raw)
To: Yann Droneaud, libc-alpha; +Cc: Leandro Pereira
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 :-)
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()
2019-10-03 15:31 ` Yann Droneaud
2019-10-03 17:10 ` Carlos O'Donell
@ 2019-10-03 17:59 ` Andreas Schwab
2019-10-03 19:04 ` Yann Droneaud
1 sibling, 1 reply; 13+ messages in thread
From: Andreas Schwab @ 2019-10-03 17:59 UTC (permalink / raw)
To: Yann Droneaud; +Cc: libc-alpha, Leandro Pereira
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.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()
2019-10-03 17:59 ` Andreas Schwab
@ 2019-10-03 19:04 ` Yann Droneaud
2019-10-03 19:16 ` Andreas Schwab
0 siblings, 1 reply; 13+ messages in thread
From: Yann Droneaud @ 2019-10-03 19:04 UTC (permalink / raw)
To: Andreas Schwab; +Cc: libc-alpha, Leandro Pereira
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.
--
Yann Droneaud
OPTEYA
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()
2019-10-03 19:04 ` Yann Droneaud
@ 2019-10-03 19:16 ` Andreas Schwab
0 siblings, 0 replies; 13+ messages in thread
From: Andreas Schwab @ 2019-10-03 19:16 UTC (permalink / raw)
To: Yann Droneaud; +Cc: libc-alpha, Leandro Pereira
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.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()
2019-10-03 13:59 ` Carlos O'Donell
@ 2019-10-18 20:02 ` Carlos O'Donell
2019-10-20 21:37 ` Lukasz Majewski
0 siblings, 1 reply; 13+ messages in thread
From: Carlos O'Donell @ 2019-10-18 20:02 UTC (permalink / raw)
To: Leandro Pereira, libc-alpha
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.
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()
2019-10-18 20:02 ` Carlos O'Donell
@ 2019-10-20 21:37 ` Lukasz Majewski
2019-10-21 13:24 ` Carlos O'Donell
0 siblings, 1 reply; 13+ messages in thread
From: Lukasz Majewski @ 2019-10-20 21:37 UTC (permalink / raw)
To: Carlos O'Donell; +Cc: Leandro Pereira, libc-alpha
[-- Attachment #1: Type: text/plain, Size: 2300 bytes --]
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()
2019-10-20 21:37 ` Lukasz Majewski
@ 2019-10-21 13:24 ` Carlos O'Donell
2019-10-24 12:08 ` Florian Weimer
0 siblings, 1 reply; 13+ messages in thread
From: Carlos O'Donell @ 2019-10-21 13:24 UTC (permalink / raw)
To: Lukasz Majewski; +Cc: Leandro Pereira, libc-alpha
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.
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()
2019-10-21 13:24 ` Carlos O'Donell
@ 2019-10-24 12:08 ` Florian Weimer
2019-10-24 13:32 ` Carlos O'Donell
0 siblings, 1 reply; 13+ messages in thread
From: Florian Weimer @ 2019-10-24 12:08 UTC (permalink / raw)
To: Carlos O'Donell; +Cc: Lukasz Majewski, Leandro Pereira, libc-alpha
* 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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()
2019-10-24 12:08 ` Florian Weimer
@ 2019-10-24 13:32 ` Carlos O'Donell
2019-10-24 13:46 ` Florian Weimer
0 siblings, 1 reply; 13+ messages in thread
From: Carlos O'Donell @ 2019-10-24 13:32 UTC (permalink / raw)
To: Florian Weimer; +Cc: Lukasz Majewski, Leandro Pereira, libc-alpha
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.
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()
2019-10-24 13:32 ` Carlos O'Donell
@ 2019-10-24 13:46 ` Florian Weimer
0 siblings, 0 replies; 13+ messages in thread
From: Florian Weimer @ 2019-10-24 13:46 UTC (permalink / raw)
To: Carlos O'Donell; +Cc: Lukasz Majewski, Leandro Pereira, libc-alpha
* 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
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-10-24 13:46 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-05 21:56 [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read() Leandro Pereira
2019-10-03 13:59 ` Carlos O'Donell
2019-10-18 20:02 ` Carlos O'Donell
2019-10-20 21:37 ` Lukasz Majewski
2019-10-21 13:24 ` Carlos O'Donell
2019-10-24 12:08 ` Florian Weimer
2019-10-24 13:32 ` Carlos O'Donell
2019-10-24 13:46 ` Florian Weimer
2019-10-03 15:31 ` Yann Droneaud
2019-10-03 17:10 ` Carlos O'Donell
2019-10-03 17:59 ` Andreas Schwab
2019-10-03 19:04 ` Yann Droneaud
2019-10-03 19:16 ` Andreas Schwab
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).