[PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()

[PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()

[Leandro Pereira]

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

Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()

[Carlos O'Donell]

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.

Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()

[Yann Droneaud]

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

Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()

[Carlos O'Donell]

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.

Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()

[Andreas Schwab]

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."

Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()

[Yann Droneaud]

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

Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()

[Andreas Schwab]

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."

Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()

[Carlos O'Donell]

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.

Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()

[Lukasz Majewski]

[-- 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 --]

Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()

[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.

-- 
Cheers,
Carlos.

Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()

[Florian Weimer]

* 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.

Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()

[Carlos O'Donell]

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.

Re: [PATCH 2/2] elf: Use nocancel pread64() instead of lseek()+read()

[Florian Weimer]

* 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