public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v12 0/6] elf: Use mmap to map in section contents
@ 2024-03-17 12:19 H.J. Lu
  2024-03-17 12:19 ` [PATCH v12 1/6] elf: Use mmap to map in read-only sections H.J. Lu
                   ` (7 more replies)
  0 siblings, 8 replies; 22+ messages in thread
From: H.J. Lu @ 2024-03-17 12:19 UTC (permalink / raw)
  To: binutils; +Cc: goldstein.w.n, sam, amodra

Changes in v12:

1. Don't call _bfd_elf_link_keep_memory for x86 since it always returns
false.
2. Make _bfd_elf_link_keep_memory static.

Changes in v11:

1. Update _bfd_mmap_read_temporary to allocate buffer as needed.
2. Don't allocate buffer when _bfd_mmap_read_temporary is called.

Changes in v10:

1. Malloc a 4 * page size buffer for external relocations for the final
link to avoid mmap/munmap on small relocation sections.
2. Malloc a buffer for input section contents which are smaller than
4 * page size or can't be mapped for the final link.

Changes in v9:

1. Use MAP_FAILED for mmap failure.

Changes in v8:

1. Rebase against master branch.
2. Add _bfd_elf_link_mmap_section_contents and
_bfd_elf_link_munmap_section_contents.

Changes in v7:

1. Don't add the --keep-memory linker option.

Changes in v6:

1. Add the --keep-memory linker option and always cache symbol and
relocation tables for --keep-memory.
2. Always keep symbol table and relocation info for eh_frame to speedup
the Rust binary build by ~300x:

https://sourceware.org/bugzilla/show_bug.cgi?id=31466

Changes in v5:

1. Drop 2 patches which have been merged onto master branch.
2. Rename _bfd_elf_mmap_section to _bfd_elf_mmap_section_contents.
3. Rename _bfd_mmap_readonly_tracked, _bfd_mmap_readonly_untracked,
_bfd_munmap_readonly_untracked, _bfd_mmap_read_untracked to
_bfd_mmap_readonly_persistent, _bfd_mmap_readonly_temporary,
_bfd_munmap_readonly_temporary and _bfd_mmap_read_temporary.
4. Drop the setup_group change.
5. Fix a typo.
6. Update comments.

Changes in v4:

1. Change don't cache symbol nor relocation tables with mmap to opt-in.

Changes in v3:

1. Fix non-mmap build.
2. Change the argument name of bfd_mmap_local from flags to prot since
its values are PROT_XXX.

Changes in v2:

1. Don't hard-code BFD_JUMP_TABLE_COPY in bfd so that elf-bfd.h can be
included in libbfd.c.
2. Change the --with-mmap default to true.
3. Check USE_MMAP instead of HAVE_MMAP.
4. Remove the asize parameter to _bfd_mmap_readonly_tracked.
5. Add contents_addr and contents_size to bfd_elf_section_data.
6. Rename _bfd_link_keep_memory to _bfd_elf_link_keep_memory.

---
We can use mmap to map in ELF section contents, instead of copying them
into memory by hand.  We don't need to cache symbol nor relocation tables
if they are mapped in.  Data to link the 3.5GB clang executable in LLVM
17 debug build on Linux/x86-64 with 32GB RAM is:

		stdio		mmap		improvement
user		86.73		87.02		-0.3%
system		9.55		9.21		3.6%
total		100.40		97.66		0.7%
maximum set(GB)	17.34		13.14		24%
page faults	4047667		3042877		25%

and data to link the 275M cc1plus executable in GCC 14 stage 1 build is:

user		5.41		5.44		-0.5%
system		0.80		0.76		5%
total		6.25		6.26		-0.2%
maximum set(MB)	1323		968		27%
page faults	323451		236371		27%

Data shows that these won't improve the single copy linker performance.
But they improve the overall system performance when linker is used by
reducing linker memory usage and page faults.  They allow more parallel
linker jobs on LLVM debug build.

Here is a quote from Noah Goldstein: "on a large project they are an
extremely large speedup".

H.J. Lu (6):
  elf: Use mmap to map in read-only sections
  elf: Add _bfd_elf_m[un]map_section_contents
  elf: Use mmap to map in symbol and relocation tables
  elf: Don't cache symbol nor relocation tables with mmap
  elf: Always keep symbol table and relocation info for eh_frame
  elf: Add _bfd_elf_link_m[un]map_section_contents

 bfd/bfd-in2.h      |  24 ++++-
 bfd/bfd.c          |  17 +++
 bfd/bfdwin.c       |   8 +-
 bfd/cache.c        |   7 +-
 bfd/compress.c     |   2 +-
 bfd/elf-bfd.h      |  24 +++++
 bfd/elf-eh-frame.c |   4 +-
 bfd/elf-sframe.c   |   4 +-
 bfd/elf.c          | 246 ++++++++++++++++++++++++++++++++++--------
 bfd/elf32-i386.c   |   8 +-
 bfd/elf64-x86-64.c |  12 +--
 bfd/elfcode.h      |   7 +-
 bfd/elflink.c      | 213 ++++++++++++++++++++++++------------
 bfd/elfxx-target.h |   6 +-
 bfd/elfxx-x86.c    |   8 +-
 bfd/elfxx-x86.h    |   1 +
 bfd/libbfd-in.h    |  33 +++++-
 bfd/libbfd.c       | 262 ++++++++++++++++++++++++++++++++++++++++++++-
 bfd/libbfd.h       |  33 +++++-
 bfd/linker.c       |  35 ------
 bfd/lynx-core.c    |   2 +-
 bfd/opncls.c       |  21 ++++
 bfd/section.c      |   9 +-
 bfd/sysdep.h       |   4 +
 24 files changed, 795 insertions(+), 195 deletions(-)

-- 
2.44.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v12 1/6] elf: Use mmap to map in read-only sections
  2024-03-17 12:19 [PATCH v12 0/6] elf: Use mmap to map in section contents H.J. Lu
@ 2024-03-17 12:19 ` H.J. Lu
  2024-04-08  3:57   ` Simon Marchi
  2024-03-17 12:19 ` [PATCH v12 2/6] elf: Add _bfd_elf_m[un]map_section_contents H.J. Lu
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: H.J. Lu @ 2024-03-17 12:19 UTC (permalink / raw)
  To: binutils; +Cc: goldstein.w.n, sam, amodra

There are many linker input files in LLVM debug build with huge string
sections.  All these string sections can be treated as read-only.  But
linker copies all of them into memory which consumes huge amount of
memory and slows down linker significantly.

Add _bfd_mmap_readonly_persistent and _bfd_mmap_readonly_temporary to
mmap in reado-only sections with size >= 4 * page size.

NB: All string sections in valid ELF inputs must be null terminated.
There is no need to terminate it again and string sections are mmapped
as read-only.

	* bfd.c (bfd_mmapped_entry): New.
	(bfd_mmapped): Likewise.
	(bfd): Add mmapped.
	* bfdwin.c (bfd_get_file_window): Use _bfd_pagesize.
	* cache.c (cache_bmmap): Remove pagesize_m1 and use pagesize_m1
	instead.
	* elf.c (bfd_elf_get_str_section): Call
	_bfd_mmap_readonly_persistent instead of _bfd_alloc_and_read.
	Don't terminate the string section again.
	(get_hash_table_data): Call _bfd_mmap_readonly_temporary and
	_bfd_munmap_readonly_temporary instead of _bfd_malloc_and_read
	and free.
	(_bfd_elf_get_dynamic_symbols): Call _bfd_mmap_readonly_persistent
	instead of _bfd_alloc_and_read.  Don't terminate the string
	section again.  Call _bfd_mmap_readonly_temporary and
	_bfd_munmap_readonly_temporary instead of _bfd_malloc_and_read
	and free.
	(_bfd_elf_slurp_version_tables): Call _bfd_mmap_readonly_temporary
	and _bfd_munmap_readonly_temporary instead of _bfd_malloc_and_read
	and free.
	* elflink.c (bfd_elf_link_record_dynamic_symbol): Use bfd_malloc
	to get the unversioned symbol.
	* libbfd-in.h (_bfd_pagesize): New.
	(_bfd_pagesize_m1): Likewise.
	(_bfd_minimum_mmap_size): Likewise.
	(_bfd_mmap_readonly_persistent): Likewise.
	(_bfd_mmap_readonly_temporary): Likewise.
	(_bfd_munmap_readonly_temporary): Likewise.
	* libbfd.c
	(bfd_allocate_mmapped_page): New.
	(_bfd_mmap_readonly_temporary): Likewise.
	(_bfd_munmap_readonly_temporary): Likewise.
	(_bfd_mmap_readonly_persistent): Likewise.
	(_bfd_pagesize): Likewise.
	(_bfd_pagesize_m1): Likewise.
	(_bfd_minimum_mmap_size): Likewise.
	(bfd_init_pagesize): Likewise.
	* lynx-core.c (lynx_core_file_p): Use _bfd_pagesize.
	* opncls.c (_bfd_delete_bfd): Munmap tracked mmapped memories.
	* sysdep.h (MAP_ANONYMOUS): New. Define if undefined.
	* bfd-in2.h: Regenerated.
	* libbfd.h: Likewise.
---
 bfd/bfd-in2.h   |  17 ++++++
 bfd/bfd.c       |  17 ++++++
 bfd/bfdwin.c    |   8 +--
 bfd/cache.c     |   7 +--
 bfd/elf.c       |  79 +++++++++++++++++--------
 bfd/elflink.c   |  16 ++---
 bfd/libbfd-in.h |  29 ++++++++++
 bfd/libbfd.c    | 151 ++++++++++++++++++++++++++++++++++++++++++++++++
 bfd/libbfd.h    |  29 ++++++++++
 bfd/lynx-core.c |   2 +-
 bfd/opncls.c    |  12 ++++
 bfd/sysdep.h    |   4 ++
 12 files changed, 328 insertions(+), 43 deletions(-)

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 29602e054da..544422a9522 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -1951,6 +1951,20 @@ struct bfd_build_id
     bfd_byte data[1];
   };
 
+struct bfd_mmapped_entry
+  {
+    void *addr;
+    size_t size;
+  };
+
+struct bfd_mmapped
+  {
+    struct bfd_mmapped *next;
+    unsigned int max_entry;
+    unsigned int next_entry;
+    struct bfd_mmapped_entry entries[1];
+  };
+
 struct bfd
 {
   /* The filename the application opened the BFD with.  */
@@ -2280,6 +2294,9 @@ struct bfd
 
   /* For input BFDs, the build ID, if the object has one. */
   const struct bfd_build_id *build_id;
+
+  /* For input BFDs, mmapped entries. */
+  struct bfd_mmapped *mmapped;
 };
 
 static inline const char *
diff --git a/bfd/bfd.c b/bfd/bfd.c
index 54061a34240..8302013f1b4 100644
--- a/bfd/bfd.c
+++ b/bfd/bfd.c
@@ -74,6 +74,20 @@ EXTERNAL
 .    bfd_byte data[1];
 .  };
 .
+.struct bfd_mmapped_entry
+.  {
+.    void *addr;
+.    size_t size;
+.  };
+.
+.struct bfd_mmapped
+.  {
+.    struct bfd_mmapped *next;
+.    unsigned int max_entry;
+.    unsigned int next_entry;
+.    struct bfd_mmapped_entry entries[1];
+.  };
+.
 
 CODE_FRAGMENT
 .struct bfd
@@ -406,6 +420,9 @@ CODE_FRAGMENT
 .
 .  {* For input BFDs, the build ID, if the object has one. *}
 .  const struct bfd_build_id *build_id;
+.
+.  {* For input BFDs, mmapped entries. *}
+.  struct bfd_mmapped *mmapped;
 .};
 .
 
diff --git a/bfd/bfdwin.c b/bfd/bfdwin.c
index 2919c71b3cb..73e44635bcb 100644
--- a/bfd/bfdwin.c
+++ b/bfd/bfdwin.c
@@ -157,7 +157,7 @@ bfd_get_file_window (bfd *abfd,
 		     bool writable)
 {
   static int ok_to_map = 1;
-  static size_t pagesize;
+  size_t pagesize = _bfd_pagesize;
   bfd_window_internal *i = windowp->i;
   bfd_size_type size_to_alloc = size;
 
@@ -167,12 +167,6 @@ bfd_get_file_window (bfd *abfd,
 	     windowp, windowp->data, (unsigned long) windowp->size,
 	     windowp->i, writable);
 
-  /* Make sure we know the page size, so we can be friendly to mmap.  */
-  if (pagesize == 0)
-    pagesize = getpagesize ();
-  if (pagesize == 0)
-    abort ();
-
   if (i == NULL)
     {
       i = bfd_zmalloc (sizeof (bfd_window_internal));
diff --git a/bfd/cache.c b/bfd/cache.c
index d0e7be293a5..0f994c74239 100644
--- a/bfd/cache.c
+++ b/bfd/cache.c
@@ -494,10 +494,10 @@ cache_bmmap (struct bfd *abfd ATTRIBUTE_UNUSED,
 #ifdef HAVE_MMAP
   else
     {
-      static uintptr_t pagesize_m1;
+      uintptr_t pagesize_m1 = _bfd_pagesize_m1;
       FILE *f;
       file_ptr pg_offset;
-      bfd_size_type pg_len;
+      size_t pg_len;
 
       f = bfd_cache_lookup (abfd, CACHE_NO_SEEK_ERROR);
       if (f == NULL)
@@ -506,9 +506,6 @@ cache_bmmap (struct bfd *abfd ATTRIBUTE_UNUSED,
 	  return ret;
 	}
 
-      if (pagesize_m1 == 0)
-	pagesize_m1 = getpagesize () - 1;
-
       /* Align.  */
       pg_offset = offset & ~pagesize_m1;
       pg_len = (len + (offset - pg_offset) + pagesize_m1) & ~pagesize_m1;
diff --git a/bfd/elf.c b/bfd/elf.c
index 8bffd3c5141..c80fff47b45 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -289,16 +289,23 @@ bfd_elf_get_str_section (bfd *abfd, unsigned int shindex)
 	 in case the string table is not terminated.  */
       if (shstrtabsize + 1 <= 1
 	  || bfd_seek (abfd, offset, SEEK_SET) != 0
-	  || (shstrtab = _bfd_alloc_and_read (abfd, shstrtabsize + 1,
-					      shstrtabsize)) == NULL)
+	  || (shstrtab
+	      = _bfd_mmap_readonly_persistent (abfd, shstrtabsize)) == NULL)
 	{
 	  /* Once we've failed to read it, make sure we don't keep
 	     trying.  Otherwise, we'll keep allocating space for
 	     the string table over and over.  */
 	  i_shdrp[shindex]->sh_size = 0;
 	}
-      else
-	shstrtab[shstrtabsize] = '\0';
+      else if (shstrtab[shstrtabsize - 1] != '\0')
+	{
+	  /* It is an error if a string table isn't terminated.  */
+	  _bfd_error_handler
+	    /* xgettext:c-format */
+	    (_("%pB(%pA): string table is corrupt"),
+	     abfd, i_shdrp[shindex]->bfd_section);
+	  return NULL;
+	}
       i_shdrp[shindex]->contents = shstrtab;
     }
   return (char *) shstrtab;
@@ -1919,6 +1926,8 @@ get_hash_table_data (bfd *abfd, bfd_size_type number,
   unsigned char *e_data = NULL;
   bfd_vma *i_data = NULL;
   bfd_size_type size;
+  void *e_data_addr;
+  size_t e_data_size ATTRIBUTE_UNUSED;
 
   if (ent_size != 4 && ent_size != 8)
     return NULL;
@@ -1940,7 +1949,8 @@ get_hash_table_data (bfd *abfd, bfd_size_type number,
       return NULL;
     }
 
-  e_data = _bfd_malloc_and_read (abfd, size, size);
+  e_data = _bfd_mmap_readonly_temporary (abfd, size, &e_data_addr,
+					 &e_data_size);
   if (e_data == NULL)
     return NULL;
 
@@ -1958,7 +1968,7 @@ get_hash_table_data (bfd *abfd, bfd_size_type number,
     while (number--)
       i_data[number] = bfd_get_64 (abfd, e_data + number * ent_size);
 
-  free (e_data);
+  _bfd_munmap_readonly_temporary (e_data_addr, e_data_size);
   return i_data;
 }
 
@@ -2007,6 +2017,10 @@ _bfd_elf_get_dynamic_symbols (bfd *abfd, Elf_Internal_Phdr *phdr,
   size_t verneed_size = 0;
   size_t extsym_size;
   const struct elf_backend_data *bed;
+  void *dynbuf_addr = NULL;
+  void *esymbuf_addr = NULL;
+  size_t dynbuf_size = 0;
+  size_t esymbuf_size = 0;
 
   /* Return TRUE if symbol table is bad.  */
   if (elf_bad_symtab (abfd))
@@ -2024,7 +2038,9 @@ _bfd_elf_get_dynamic_symbols (bfd *abfd, Elf_Internal_Phdr *phdr,
   if (bfd_seek (abfd, phdr->p_offset, SEEK_SET) != 0)
     goto error_return;
 
-  dynbuf = _bfd_malloc_and_read (abfd, phdr->p_filesz, phdr->p_filesz);
+  dynbuf_size = phdr->p_filesz;
+  dynbuf = _bfd_mmap_readonly_temporary (abfd, dynbuf_size,
+					 &dynbuf_addr, &dynbuf_size);
   if (dynbuf == NULL)
     goto error_return;
 
@@ -2102,11 +2118,17 @@ _bfd_elf_get_dynamic_symbols (bfd *abfd, Elf_Internal_Phdr *phdr,
     goto error_return;
 
   /* Dynamic string table must be valid until ABFD is closed.  */
-  strbuf = (char *) _bfd_alloc_and_read (abfd, dt_strsz + 1, dt_strsz);
+  strbuf = (char *) _bfd_mmap_readonly_persistent (abfd, dt_strsz);
   if (strbuf == NULL)
     goto error_return;
-  /* Since this is a string table, make sure that it is terminated.  */
-  strbuf[dt_strsz] = 0;
+  if (strbuf[dt_strsz - 1] != 0)
+    {
+      /* It is an error if a string table is't terminated.  */
+      _bfd_error_handler
+	/* xgettext:c-format */
+	(_("%pB: DT_STRTAB table is corrupt"), abfd);
+      goto error_return;
+    }
 
   /* Get the real symbol count from DT_HASH or DT_GNU_HASH.  Prefer
      DT_HASH since it is simpler than DT_GNU_HASH.  */
@@ -2281,7 +2303,10 @@ _bfd_elf_get_dynamic_symbols (bfd *abfd, Elf_Internal_Phdr *phdr,
   if (filepos == (file_ptr) -1
       || bfd_seek (abfd, filepos, SEEK_SET) != 0)
     goto error_return;
-  esymbuf = _bfd_malloc_and_read (abfd, amt, amt);
+  esymbuf_size = amt;
+  esymbuf = _bfd_mmap_readonly_temporary (abfd, esymbuf_size,
+					  &esymbuf_addr,
+					  &esymbuf_size);
   if (esymbuf == NULL)
     goto error_return;
 
@@ -2325,7 +2350,7 @@ _bfd_elf_get_dynamic_symbols (bfd *abfd, Elf_Internal_Phdr *phdr,
 	goto error_return;
 
       /* DT_VERSYM info must be valid until ABFD is closed.  */
-      versym = _bfd_alloc_and_read (abfd, amt, amt);
+      versym = _bfd_mmap_readonly_persistent (abfd, amt);
 
       if (dt_verdef)
 	{
@@ -2337,8 +2362,7 @@ _bfd_elf_get_dynamic_symbols (bfd *abfd, Elf_Internal_Phdr *phdr,
 	    goto error_return;
 
 	  /* DT_VERDEF info must be valid until ABFD is closed.  */
-	  verdef = _bfd_alloc_and_read (abfd, verdef_size,
-					verdef_size);
+	  verdef = _bfd_mmap_readonly_persistent (abfd, verdef_size);
 	}
 
       if (dt_verneed)
@@ -2351,8 +2375,7 @@ _bfd_elf_get_dynamic_symbols (bfd *abfd, Elf_Internal_Phdr *phdr,
 	    goto error_return;
 
 	  /* DT_VERNEED info must be valid until ABFD is closed.  */
-	  verneed = _bfd_alloc_and_read (abfd, verneed_size,
-					 verneed_size);
+	  verneed = _bfd_mmap_readonly_persistent (abfd, verneed_size);
 	}
     }
 
@@ -2375,8 +2398,8 @@ _bfd_elf_get_dynamic_symbols (bfd *abfd, Elf_Internal_Phdr *phdr,
   /* Restore file position for elf_object_p.  */
   if (bfd_seek (abfd, saved_filepos, SEEK_SET) != 0)
     res = false;
-  free (dynbuf);
-  free (esymbuf);
+  _bfd_munmap_readonly_temporary (dynbuf_addr, dynbuf_size);
+  _bfd_munmap_readonly_temporary (esymbuf_addr, esymbuf_size);
   free (gnubuckets);
   free (gnuchains);
   free (mipsxlat);
@@ -9435,6 +9458,8 @@ _bfd_elf_slurp_version_tables (bfd *abfd, bool default_imported_symver)
   bfd_byte *contents = NULL;
   unsigned int freeidx = 0;
   size_t amt;
+  void *contents_addr = NULL;
+  size_t contents_size = 0;
 
   if (elf_dynverref (abfd) != 0 || elf_tdata (abfd)->dt_verneed != NULL)
     {
@@ -9471,7 +9496,10 @@ _bfd_elf_slurp_version_tables (bfd *abfd, bool default_imported_symver)
 
 	  if (bfd_seek (abfd, hdr->sh_offset, SEEK_SET) != 0)
 	    goto error_return_verref;
-	  contents = _bfd_malloc_and_read (abfd, hdr->sh_size, hdr->sh_size);
+	  contents_size = hdr->sh_size;
+	  contents = _bfd_mmap_readonly_temporary (abfd, contents_size,
+						   &contents_addr,
+						   &contents_size);
 	  if (contents == NULL)
 	    goto error_return_verref;
 
@@ -9604,8 +9632,9 @@ _bfd_elf_slurp_version_tables (bfd *abfd, bool default_imported_symver)
       elf_tdata (abfd)->cverrefs = i;
 
       if (contents != elf_tdata (abfd)->dt_verneed)
-	free (contents);
+	_bfd_munmap_readonly_temporary (contents_addr, contents_size);
       contents = NULL;
+      contents_addr = NULL;
     }
 
   if (elf_dynverdef (abfd) != 0 || elf_tdata (abfd)->dt_verdef != NULL)
@@ -9646,7 +9675,10 @@ _bfd_elf_slurp_version_tables (bfd *abfd, bool default_imported_symver)
 
 	  if (bfd_seek (abfd, hdr->sh_offset, SEEK_SET) != 0)
 	    goto error_return_verdef;
-	  contents = _bfd_malloc_and_read (abfd, hdr->sh_size, hdr->sh_size);
+	  contents_size = hdr->sh_size;
+	  contents = _bfd_mmap_readonly_temporary (abfd, contents_size,
+						   &contents_addr,
+						   &contents_size);
 	  if (contents == NULL)
 	    goto error_return_verdef;
 
@@ -9800,8 +9832,9 @@ _bfd_elf_slurp_version_tables (bfd *abfd, bool default_imported_symver)
 	}
 
       if (contents != elf_tdata (abfd)->dt_verdef)
-	free (contents);
+	_bfd_munmap_readonly_temporary (contents_addr, contents_size);
       contents = NULL;
+      contents_addr = NULL;
     }
   else if (default_imported_symver)
     {
@@ -9857,7 +9890,7 @@ _bfd_elf_slurp_version_tables (bfd *abfd, bool default_imported_symver)
  error_return:
   if (contents != elf_tdata (abfd)->dt_verneed
       && contents != elf_tdata (abfd)->dt_verdef)
-    free (contents);
+    _bfd_munmap_readonly_temporary (contents_addr, contents_size);
   return false;
 }
 \f
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 5a6cb07b2ce..42029f29f7a 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -549,22 +549,24 @@ bfd_elf_link_record_dynamic_symbol (struct bfd_link_info *info,
 	    return false;
 	}
 
+      char *unversioned_name = NULL;
+
       /* We don't put any version information in the dynamic string
 	 table.  */
       name = h->root.root.string;
       p = strchr (name, ELF_VER_CHR);
       if (p != NULL)
-	/* We know that the p points into writable memory.  In fact,
-	   there are only a few symbols that have read-only names, being
-	   those like _GLOBAL_OFFSET_TABLE_ that are created specially
-	   by the backends.  Most symbols will have names pointing into
-	   an ELF string table read from a file, or to objalloc memory.  */
-	*p = 0;
+	{
+	  unversioned_name = bfd_malloc (p - name + 1);
+	  memcpy (unversioned_name, name, p - name);
+	  unversioned_name[p - name] = 0;
+	  name = unversioned_name;
+	}
 
       indx = _bfd_elf_strtab_add (dynstr, name, p != NULL);
 
       if (p != NULL)
-	*p = ELF_VER_CHR;
+	free (unversioned_name);
 
       if (indx == (size_t) -1)
 	return false;
diff --git a/bfd/libbfd-in.h b/bfd/libbfd-in.h
index b8b2ce7ba09..c5a79cf932c 100644
--- a/bfd/libbfd-in.h
+++ b/bfd/libbfd-in.h
@@ -851,6 +851,10 @@ extern struct bfd_link_info *_bfd_get_link_info (bfd *)
 extern bool _bfd_link_keep_memory (struct bfd_link_info *)
   ATTRIBUTE_HIDDEN;
 
+extern uintptr_t _bfd_pagesize ATTRIBUTE_HIDDEN;
+extern uintptr_t _bfd_pagesize_m1 ATTRIBUTE_HIDDEN;
+extern uintptr_t _bfd_minimum_mmap_size ATTRIBUTE_HIDDEN;
+
 #if GCC_VERSION >= 7000
 #define _bfd_mul_overflow(a, b, res) __builtin_mul_overflow (a, b, res)
 #else
@@ -888,6 +892,19 @@ _bfd_alloc_and_read (bfd *abfd, bfd_size_type asize, bfd_size_type rsize)
   return NULL;
 }
 
+#ifdef USE_MMAP
+extern void *_bfd_mmap_readonly_persistent
+  (bfd *, size_t) ATTRIBUTE_HIDDEN;
+extern void *_bfd_mmap_readonly_temporary
+  (bfd *, size_t, void **, size_t *) ATTRIBUTE_HIDDEN;
+extern void _bfd_munmap_readonly_temporary
+  (void *, size_t) ATTRIBUTE_HIDDEN;
+#else
+#define _bfd_mmap_readonly_persistent(abfd, rsize) \
+  _bfd_alloc_and_read (abfd, rsize, rsize)
+#define _bfd_munmap_readonly_temporary(ptr, rsize) free (ptr)
+#endif
+
 static inline void *
 _bfd_malloc_and_read (bfd *abfd, bfd_size_type asize, bfd_size_type rsize)
 {
@@ -910,3 +927,15 @@ _bfd_malloc_and_read (bfd *abfd, bfd_size_type asize, bfd_size_type rsize)
     }
   return NULL;
 }
+
+#ifndef USE_MMAP
+static inline void *
+_bfd_mmap_readonly_temporary (bfd *abfd, size_t rsize, void **map_addr,
+			      size_t *map_size)
+{
+  void *mem = _bfd_malloc_and_read (abfd, rsize, rsize);
+  *map_addr = mem;
+  *map_size = rsize;
+  return mem;
+}
+#endif
diff --git a/bfd/libbfd.c b/bfd/libbfd.c
index f8d148c9677..a79c814a0dc 100644
--- a/bfd/libbfd.c
+++ b/bfd/libbfd.c
@@ -1038,6 +1038,141 @@ bfd_get_bits (const void *p, int bits, bool big_p)
   return data;
 }
 \f
+#ifdef USE_MMAP
+/* Allocate a page to track mmapped memory and return the page and
+   the first entry.  Return NULL if mmap fails.  */
+
+static struct bfd_mmapped *
+bfd_allocate_mmapped_page (bfd *abfd, struct bfd_mmapped_entry **entry)
+{
+  struct bfd_mmapped * mmapped
+    = (struct bfd_mmapped *) mmap (NULL, _bfd_pagesize,
+				   PROT_READ | PROT_WRITE,
+				   MAP_PRIVATE | MAP_ANONYMOUS,
+				   -1, 0);
+  if (mmapped == MAP_FAILED)
+    return NULL;
+
+  mmapped->next = abfd->mmapped;
+  mmapped->max_entry
+    = ((_bfd_pagesize - offsetof (struct bfd_mmapped, entries))
+       / sizeof (struct bfd_mmapped_entry));
+  mmapped->next_entry = 1;
+  abfd->mmapped = mmapped;
+  *entry = mmapped->entries;
+  return mmapped;
+}
+
+/* Mmap a memory region of RSIZE bytes with PROT at the current offset.
+   Return mmap address and size in MAP_ADDR and MAP_SIZE.  Return NULL
+   on invalid input and MAP_FAILED for mmap failure.  */
+
+static void *
+bfd_mmap_local (bfd *abfd, size_t rsize, int prot, void **map_addr,
+		size_t *map_size)
+{
+  if (!_bfd_constant_p (rsize))
+    {
+      ufile_ptr filesize = bfd_get_file_size (abfd);
+      if (filesize != 0 && rsize > filesize)
+	{
+	  bfd_set_error (bfd_error_file_truncated);
+	  return NULL;
+	}
+    }
+
+  void *mem;
+  ufile_ptr offset = bfd_tell (abfd);
+  mem = bfd_mmap (abfd, NULL, rsize, prot, MAP_PRIVATE, offset,
+		  map_addr, map_size);
+  return mem;
+}
+
+/* Mmap a readonly memory region of RSIZE bytes at the current offset.
+   Return mmap address and size in MAP_ADDR and MAP_SIZE.  Return NULL
+   on invalid input and MAP_FAILED for mmap failure.  */
+
+void *
+_bfd_mmap_readonly_temporary (bfd *abfd, size_t rsize, void **map_addr,
+			      size_t *map_size)
+{
+  /* Use mmap only if section size >= the minimum mmap section size.  */
+  if (rsize < _bfd_minimum_mmap_size)
+    {
+      void *mem = _bfd_malloc_and_read (abfd, rsize, rsize);
+      /* NB: Set *MAP_ADDR to MEM and *MAP_SIZE to 0 to indicate that
+	 _bfd_malloc_and_read is called.  */
+      *map_addr = mem;
+      *map_size = 0;
+      return mem;
+    }
+
+  return bfd_mmap_local (abfd, rsize, PROT_READ, map_addr, map_size);
+}
+
+/* Munmap RSIZE bytes at PTR.  */
+
+void
+_bfd_munmap_readonly_temporary (void *ptr, size_t rsize)
+{
+  /* NB: Since _bfd_munmap_readonly_temporary is called like free, PTR
+     may be NULL.  Otherwise, PTR and RSIZE must be valid.  If RSIZE is
+     0, _bfd_malloc_and_read is called.  */
+  if (ptr == NULL)
+    return;
+  if (rsize != 0)
+    {
+      if (munmap (ptr, rsize) != 0)
+	abort ();
+    }
+  else
+    free (ptr);
+}
+
+/* Mmap a readonly memory region of RSIZE bytes at the current offset.
+   Return NULL on invalid input or mmap failure.  */
+
+void *
+_bfd_mmap_readonly_persistent (bfd *abfd, size_t rsize)
+{
+  /* Use mmap only if section size >= the minimum mmap section size.  */
+  if (rsize < _bfd_minimum_mmap_size)
+    return _bfd_alloc_and_read (abfd, rsize, rsize);
+
+  void *mem, *map_addr;
+  size_t map_size;
+  mem = bfd_mmap_local (abfd, rsize, PROT_READ, &map_addr, &map_size);
+  if (mem == NULL)
+    return mem;
+  if (mem == MAP_FAILED)
+    return _bfd_alloc_and_read (abfd, rsize, rsize);
+
+  struct bfd_mmapped_entry *entry;
+  unsigned int next_entry;
+  struct bfd_mmapped *mmapped = abfd->mmapped;
+  if (mmapped != NULL
+      && (next_entry = mmapped->next_entry) < mmapped->max_entry)
+    {
+      entry = &mmapped->entries[next_entry];
+      mmapped->next_entry++;
+    }
+  else
+    {
+      mmapped = bfd_allocate_mmapped_page (abfd, &entry);
+      if (mmapped == NULL)
+	{
+	  munmap (map_addr, map_size);
+	  return NULL;
+	}
+    }
+
+  entry->addr = map_addr;
+  entry->size = map_size;
+
+  return mem;
+}
+#endif
+
 /* Default implementation */
 
 bool
@@ -1326,3 +1461,19 @@ _bfd_generic_init_private_section_data (bfd *ibfd ATTRIBUTE_UNUSED,
 {
   return true;
 }
+
+uintptr_t _bfd_pagesize;
+uintptr_t _bfd_pagesize_m1;
+uintptr_t _bfd_minimum_mmap_size;
+
+__attribute__ ((unused, constructor))
+static void
+bfd_init_pagesize (void)
+{
+  _bfd_pagesize = getpagesize ();
+  if (_bfd_pagesize == 0)
+    abort ();
+  _bfd_pagesize_m1 = _bfd_pagesize - 1;
+  /* The minimum section size to use mmap.  */
+  _bfd_minimum_mmap_size = _bfd_pagesize * 4;
+}
diff --git a/bfd/libbfd.h b/bfd/libbfd.h
index f15b5f27db8..47f40889a95 100644
--- a/bfd/libbfd.h
+++ b/bfd/libbfd.h
@@ -857,6 +857,10 @@ extern struct bfd_link_info *_bfd_get_link_info (bfd *)
 extern bool _bfd_link_keep_memory (struct bfd_link_info *)
   ATTRIBUTE_HIDDEN;
 
+extern uintptr_t _bfd_pagesize ATTRIBUTE_HIDDEN;
+extern uintptr_t _bfd_pagesize_m1 ATTRIBUTE_HIDDEN;
+extern uintptr_t _bfd_minimum_mmap_size ATTRIBUTE_HIDDEN;
+
 #if GCC_VERSION >= 7000
 #define _bfd_mul_overflow(a, b, res) __builtin_mul_overflow (a, b, res)
 #else
@@ -894,6 +898,19 @@ _bfd_alloc_and_read (bfd *abfd, bfd_size_type asize, bfd_size_type rsize)
   return NULL;
 }
 
+#ifdef USE_MMAP
+extern void *_bfd_mmap_readonly_persistent
+  (bfd *, size_t) ATTRIBUTE_HIDDEN;
+extern void *_bfd_mmap_readonly_temporary
+  (bfd *, size_t, void **, size_t *) ATTRIBUTE_HIDDEN;
+extern void _bfd_munmap_readonly_temporary
+  (void *, size_t) ATTRIBUTE_HIDDEN;
+#else
+#define _bfd_mmap_readonly_persistent(abfd, rsize) \
+  _bfd_alloc_and_read (abfd, rsize, rsize)
+#define _bfd_munmap_readonly_temporary(ptr, rsize) free (ptr)
+#endif
+
 static inline void *
 _bfd_malloc_and_read (bfd *abfd, bfd_size_type asize, bfd_size_type rsize)
 {
@@ -916,6 +933,18 @@ _bfd_malloc_and_read (bfd *abfd, bfd_size_type asize, bfd_size_type rsize)
     }
   return NULL;
 }
+
+#ifndef USE_MMAP
+static inline void *
+_bfd_mmap_readonly_temporary (bfd *abfd, size_t rsize, void **map_addr,
+			      size_t *map_size)
+{
+  void *mem = _bfd_malloc_and_read (abfd, rsize, rsize);
+  *map_addr = mem;
+  *map_size = rsize;
+  return mem;
+}
+#endif
 /* Extracted from libbfd.c.  */
 void *bfd_malloc (bfd_size_type /*size*/) ATTRIBUTE_HIDDEN;
 
diff --git a/bfd/lynx-core.c b/bfd/lynx-core.c
index 44d94ad8745..9ec5a0d2028 100644
--- a/bfd/lynx-core.c
+++ b/bfd/lynx-core.c
@@ -96,7 +96,7 @@ lynx_core_file_p (bfd *abfd)
   asection *newsect;
   size_t amt;
 
-  pagesize = getpagesize ();	/* Serious cross-target issue here...  This
+  pagesize = _bfd_pagesize;	/* Serious cross-target issue here...  This
 				   really needs to come from a system-specific
 				   header file.  */
 
diff --git a/bfd/opncls.c b/bfd/opncls.c
index c764d204831..e6337b88e18 100644
--- a/bfd/opncls.c
+++ b/bfd/opncls.c
@@ -163,6 +163,18 @@ _bfd_new_bfd_contained_in (bfd *obfd)
 static void
 _bfd_delete_bfd (bfd *abfd)
 {
+#ifdef USE_MMAP
+  struct bfd_mmapped *mmapped, *next;
+  for (mmapped = abfd->mmapped; mmapped != NULL; mmapped = next)
+    {
+      struct bfd_mmapped_entry *entries = mmapped->entries;
+      next = mmapped->next;
+      for (unsigned int i = 0; i < mmapped->next_entry; i++)
+	munmap (entries[i].addr, entries[i].size);
+      munmap (mmapped, _bfd_pagesize);
+    }
+#endif
+
   /* Give the target _bfd_free_cached_info a chance to free memory.  */
   if (abfd->memory && abfd->xvec)
     bfd_free_cached_info (abfd);
diff --git a/bfd/sysdep.h b/bfd/sysdep.h
index b907bc26a09..173f5804df3 100644
--- a/bfd/sysdep.h
+++ b/bfd/sysdep.h
@@ -98,6 +98,10 @@
 #define MAP_FAILED ((void *) -1)
 #endif
 
+#ifndef MAP_ANONYMOUS
+#define MAP_ANONYMOUS  MAP_ANON
+#endif
+
 #include "filenames.h"
 
 #if !HAVE_DECL_FFS
-- 
2.44.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v12 2/6] elf: Add _bfd_elf_m[un]map_section_contents
  2024-03-17 12:19 [PATCH v12 0/6] elf: Use mmap to map in section contents H.J. Lu
  2024-03-17 12:19 ` [PATCH v12 1/6] elf: Use mmap to map in read-only sections H.J. Lu
@ 2024-03-17 12:19 ` H.J. Lu
  2024-03-17 12:19 ` [PATCH v12 3/6] elf: Use mmap to map in symbol and relocation tables H.J. Lu
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 22+ messages in thread
From: H.J. Lu @ 2024-03-17 12:19 UTC (permalink / raw)
  To: binutils; +Cc: goldstein.w.n, sam, amodra

Add _bfd_elf_mmap_section_contents and _bfd_elf_munmap_section_contents.
A backend must opt-in to use mmap.  It should replace

bfd_malloc_and_get_section -> _bfd_elf_mmap_section_contents
free			   -> _bfd_elf_munmap_section_contents

on section contents.

	* compress.c (bfd_get_full_section_contents): Don't allocate
	buffer if mmapped_p is true.
	* elf-bfd.h (elf_backend_data): Add use_mmap.
	(bfd_elf_section_data): Add contents_addr and contents_size.
	(_bfd_elf_mmap_section_contents): New.
	(_bfd_elf_munmap_section_contents): Likewise.
	* elf-eh-frame.c (_bfd_elf_parse_eh_frame): Replace
	bfd_malloc_and_get_section and free with
	_bfd_elf_mmap_section_contents and _bfd_elf_munmap_section_contents
	on section contents.
	* elf-sframe.c (_bfd_elf_parse_sframe): Likewise.
	* elf.c (_bfd_elf_make_section_from_shdr): Replace
	bfd_malloc_and_get_section and free with
	_bfd_elf_mmap_section_contents and _bfd_elf_munmap_section_contents
	on section contents.
	(_bfd_elf_print_private_bfd_data): Likewise.
	(_bfd_elf_mmap_section_contents): New.
	(_bfd_elf_munmap_section_contents): Likewise.
	* elf32-i386.c (elf_i386_scan_relocs): Replace
	bfd_malloc_and_get_section and free with
	_bfd_elf_mmap_section_contents and _bfd_elf_munmap_section_contents
	on section contents.
	* elf64-x86-64.c (elf_x86_64_scan_relocs): Likewise.
	(elf_x86_64_get_synthetic_symtab): Likewise.
	* elfcode.h (elf_checksum_contents): Likewise.
	* elflink.c (elf_link_add_object_symbols): Likewise.
	(bfd_elf_get_bfd_needed_list): Likewise.
	* elfxx-target.h (elf_backend_use_mmap): New.
	(elfNN_bed): Add elf_backend_use_mmap.
	* elfxx-x86.c (elf_x86_size_or_finish_relative_reloc): Replace
	bfd_malloc_and_get_section and free with
	_bfd_elf_mmap_section_contents and _bfd_elf_munmap_section_contents
	on section contents.
	(_bfd_x86_elf_get_synthetic_symtab): Replace free with
	_bfd_elf_munmap_section_contents.
	* elfxx-x86.h (elf_backend_use_mmap): New.
	* libbfd.c: Include "elf-bfd.h".
	(_bfd_generic_get_section_contents): Call bfd_mmap_local for
	mmapped_p.
	* opncls.c (_bfd_delete_bfd): Also munmap ELF section contents.
	* section.c (asection): Add mmapped_p.
	(BFD_FAKE_SECTION): Updated.
	(bfd_malloc_and_get_section): Add a sanity check for not
	mmapped_p.
	* bfd-in2.h: Regenerated.
---
 bfd/bfd-in2.h      |  7 ++--
 bfd/compress.c     |  2 +-
 bfd/elf-bfd.h      | 20 +++++++++++
 bfd/elf-eh-frame.c |  4 +--
 bfd/elf-sframe.c   |  4 +--
 bfd/elf.c          | 87 +++++++++++++++++++++++++++++++++++++++++++---
 bfd/elf32-i386.c   |  6 ++--
 bfd/elf64-x86-64.c | 10 +++---
 bfd/elfcode.h      |  7 ++--
 bfd/elflink.c      | 12 +++----
 bfd/elfxx-target.h |  6 +++-
 bfd/elfxx-x86.c    |  8 ++---
 bfd/elfxx-x86.h    |  1 +
 bfd/libbfd.c       | 59 +++++++++++++++++++++++++++++--
 bfd/opncls.c       |  9 +++++
 bfd/section.c      |  9 +++--
 16 files changed, 212 insertions(+), 39 deletions(-)

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 544422a9522..dc6bceb6a96 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -688,6 +688,9 @@ typedef struct bfd_section
   /* Nonzero if this section uses RELA relocations, rather than REL.  */
   unsigned int use_rela_p:1;
 
+  /* Nonzero if this section contents are mmapped, rather than malloced.  */
+  unsigned int mmapped_p:1;
+
   /* Bits used by various backends.  The generic code doesn't touch
      these fields.  */
 
@@ -975,8 +978,8 @@ discarded_section (const asection *sec)
   /* linker_mark, linker_has_input, gc_mark, decompress_status,     */ \
      0,           0,                1,       0,                        \
 								       \
-  /* segment_mark, sec_info_type, use_rela_p,                       */ \
-     0,            0,             0,                                   \
+  /* segment_mark, sec_info_type, use_rela_p, mmapped_p,           */  \
+     0,            0,             0,          0,                       \
 								       \
   /* sec_flg0, sec_flg1, sec_flg2, sec_flg3, sec_flg4, sec_flg5,    */ \
      0,        0,        0,        0,        0,        0,              \
diff --git a/bfd/compress.c b/bfd/compress.c
index 8bc44de813b..6c211843e60 100644
--- a/bfd/compress.c
+++ b/bfd/compress.c
@@ -749,7 +749,7 @@ bfd_get_full_section_contents (bfd *abfd, sec_ptr sec, bfd_byte **ptr)
   switch (compress_status)
     {
     case COMPRESS_SECTION_NONE:
-      if (p == NULL)
+      if (p == NULL && !sec->mmapped_p)
 	{
 	  p = (bfd_byte *) bfd_malloc (allocsz);
 	  if (p == NULL)
diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index c5d325435b6..e7c2cd19bed 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -1775,6 +1775,12 @@ struct elf_backend_data
   /* True if the 64-bit Linux PRPSINFO structure's `pr_uid' and `pr_gid'
      members use a 16-bit data type.  */
   unsigned linux_prpsinfo64_ugid16 : 1;
+
+  /* True if the backend can use mmap to map in all input section
+     contents.  All bfd_malloc_and_get_section and free usages on
+     section contents must be replaced by _bfd_elf_mmap_section_contents
+     and _bfd_elf_munmap_section_contents.  */
+  unsigned use_mmap : 1;
 };
 
 /* Information about reloc sections associated with a bfd_elf_section_data
@@ -1856,6 +1862,15 @@ struct bfd_elf_section_data
   /* Link from a text section to its .eh_frame_entry section.  */
   asection *eh_frame_entry;
 
+  /* If the mmapped_p flag is set, this points to the actual mmapped
+     address of contents.  If it is set to NULL, contents isn't
+     mmapped.  */
+  void *contents_addr;
+
+  /* If the mmapped_p flag is set, this is the actual mmapped size of
+     contents.  */
+  size_t contents_size;
+
   /* TRUE if the section has secondary reloc sections associated with it.
      FIXME: In the future it might be better to change this into a list
      of secondary reloc sections, making lookup easier and faster.  */
@@ -3124,6 +3139,11 @@ extern bool _bfd_elf_maybe_set_textrel
 extern bool _bfd_elf_add_dynamic_tags
   (bfd *, struct bfd_link_info *, bool);
 
+extern bool _bfd_elf_mmap_section_contents
+  (bfd *abfd, asection *section, bfd_byte **buf);
+extern void _bfd_elf_munmap_section_contents
+  (asection *, void *);
+
 /* Large common section.  */
 extern asection _bfd_elf_large_com_section;
 
diff --git a/bfd/elf-eh-frame.c b/bfd/elf-eh-frame.c
index 9a504234163..902d7c16334 100644
--- a/bfd/elf-eh-frame.c
+++ b/bfd/elf-eh-frame.c
@@ -618,7 +618,7 @@ _bfd_elf_parse_eh_frame (bfd *abfd, struct bfd_link_info *info,
 
   /* Read the frame unwind information from abfd.  */
 
-  REQUIRE (bfd_malloc_and_get_section (abfd, sec, &ehbuf));
+  REQUIRE (_bfd_elf_mmap_section_contents (abfd, sec, &ehbuf));
 
   /* If .eh_frame section size doesn't fit into int, we cannot handle
      it (it would need to use 64-bit .eh_frame format anyway).  */
@@ -1052,7 +1052,7 @@ _bfd_elf_parse_eh_frame (bfd *abfd, struct bfd_link_info *info,
   hdr_info->u.dwarf.table = false;
   free (sec_info);
  success:
-  free (ehbuf);
+  _bfd_elf_munmap_section_contents (sec, ehbuf);
   free (local_cies);
 #undef REQUIRE
 }
diff --git a/bfd/elf-sframe.c b/bfd/elf-sframe.c
index bfc875cd9fc..a4e3143e9d2 100644
--- a/bfd/elf-sframe.c
+++ b/bfd/elf-sframe.c
@@ -208,7 +208,7 @@ _bfd_elf_parse_sframe (bfd *abfd,
     }
 
   /* Read the SFrame stack trace information from abfd.  */
-  if (!bfd_malloc_and_get_section (abfd, sec, &sfbuf))
+  if (!_bfd_elf_mmap_section_contents (abfd, sec, &sfbuf))
     goto fail_no_free;
 
   /* Decode the buffer and keep decoded contents for later use.
@@ -241,7 +241,7 @@ fail_no_free:
     abfd, sec);
   return false;
 success:
-  free (sfbuf);
+  _bfd_elf_munmap_section_contents (sec, sfbuf);
   return true;
 }
 
diff --git a/bfd/elf.c b/bfd/elf.c
index c80fff47b45..557c1ac5f4a 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -1123,12 +1123,12 @@ _bfd_elf_make_section_from_shdr (bfd *abfd,
     {
       bfd_byte *contents;
 
-      if (!bfd_malloc_and_get_section (abfd, newsect, &contents))
+      if (!_bfd_elf_mmap_section_contents (abfd, newsect, &contents))
 	return false;
 
       elf_parse_notes (abfd, (char *) contents, hdr->sh_size,
 		       hdr->sh_offset, hdr->sh_addralign);
-      free (contents);
+      _bfd_elf_munmap_section_contents (newsect, contents);
     }
 
   if ((newsect->flags & SEC_ALLOC) != 0)
@@ -1708,7 +1708,7 @@ _bfd_elf_print_private_bfd_data (bfd *abfd, void *farg)
 
       fprintf (f, _("\nDynamic Section:\n"));
 
-      if (!bfd_malloc_and_get_section (abfd, s, &dynbuf))
+      if (!_bfd_elf_mmap_section_contents (abfd, s, &dynbuf))
 	goto error_return;
 
       elfsec = _bfd_elf_section_from_bfd_section (abfd, s);
@@ -1830,7 +1830,7 @@ _bfd_elf_print_private_bfd_data (bfd *abfd, void *farg)
 	  fprintf (f, "\n");
 	}
 
-      free (dynbuf);
+      _bfd_elf_munmap_section_contents (s, dynbuf);
       dynbuf = NULL;
     }
 
@@ -1887,7 +1887,7 @@ _bfd_elf_print_private_bfd_data (bfd *abfd, void *farg)
   return true;
 
  error_return:
-  free (dynbuf);
+  _bfd_elf_munmap_section_contents (s, dynbuf);
   return false;
 }
 
@@ -14392,3 +14392,80 @@ _bfd_elf_write_secondary_reloc_section (bfd *abfd, asection *sec)
 
   return result;
 }
+
+/* Mmap in section contents.  */
+
+bool
+_bfd_elf_mmap_section_contents (bfd *abfd, sec_ptr sec, bfd_byte **buf)
+{
+#ifdef USE_MMAP
+  const struct elf_backend_data *bed = get_elf_backend_data (abfd);
+  if (bed->use_mmap
+      && sec->compress_status == COMPRESS_SECTION_NONE
+      && (sec->flags & SEC_LINKER_CREATED) == 0)
+    {
+      /* Use mmap only if section size >= the minimum mmap section
+	 size.  */
+      size_t readsz = bfd_get_section_limit_octets (abfd, sec);
+      size_t allocsz = bfd_get_section_alloc_size (abfd, sec);
+      if (readsz == allocsz && readsz >= _bfd_minimum_mmap_size)
+	{
+	  if (sec->contents != NULL)
+	    {
+	      if (!sec->mmapped_p)
+		abort ();
+	      *buf = sec->contents;
+	      return true;
+	    }
+	  if (sec->mmapped_p)
+	    abort ();
+	  sec->mmapped_p = 1;
+	}
+    }
+#endif
+  *buf = NULL;
+  bool ret = bfd_get_full_section_contents (abfd, sec, buf);
+  if (ret && sec->mmapped_p)
+    *buf = sec->contents;
+  return ret;
+}
+
+/* Munmap section contents.  */
+
+void
+_bfd_elf_munmap_section_contents (asection *sec ATTRIBUTE_UNUSED,
+				  void *contents)
+{
+  /* NB: Since _bfd_elf_munmap_section_contents is called like free,
+     CONTENTS may be NULL.  */
+  if (contents == NULL)
+    return;
+
+#ifdef USE_MMAP
+  if (sec->mmapped_p)
+    {
+      /* _bfd_elf_mmap_section_contents may return the previously
+	 mapped section contents.  Munmap the section contents only
+	 if they haven't been cached.  */
+      if (elf_section_data (sec)->this_hdr.contents == contents)
+	return;
+
+      /* When _bfd_elf_mmap_section_contents returns CONTENTS as
+	 malloced, CONTENTS_ADDR is set to NULL.  */
+      if (elf_section_data (sec)->contents_addr != NULL)
+	{
+	  /* NB: CONTENTS_ADDR and CONTENTS_SIZE must be valid.  */
+	  if (munmap (elf_section_data (sec)->contents_addr,
+		      elf_section_data (sec)->contents_size) != 0)
+	    abort ();
+	  sec->mmapped_p = 0;
+	  sec->contents = NULL;
+	  elf_section_data (sec)->contents_addr = NULL;
+	  elf_section_data (sec)->contents_size = 0;
+	  return;
+	}
+    }
+#endif
+
+  free (contents);
+}
diff --git a/bfd/elf32-i386.c b/bfd/elf32-i386.c
index 703a48c2c0a..0f963c373be 100644
--- a/bfd/elf32-i386.c
+++ b/bfd/elf32-i386.c
@@ -1499,7 +1499,7 @@ elf_i386_scan_relocs (bfd *abfd,
   /* Get the section contents.  */
   if (elf_section_data (sec)->this_hdr.contents != NULL)
     contents = elf_section_data (sec)->this_hdr.contents;
-  else if (!bfd_malloc_and_get_section (abfd, sec, &contents))
+  else if (!_bfd_elf_mmap_section_contents (abfd, sec, &contents))
     {
       sec->check_relocs_failed = 1;
       return false;
@@ -1933,7 +1933,7 @@ elf_i386_scan_relocs (bfd *abfd,
   if (elf_section_data (sec)->this_hdr.contents != contents)
     {
       if (!converted && !_bfd_link_keep_memory (info))
-	free (contents);
+	_bfd_elf_munmap_section_contents (sec, contents);
       else
 	{
 	  /* Cache the section contents for elf_link_input_bfd if any
@@ -1951,7 +1951,7 @@ elf_i386_scan_relocs (bfd *abfd,
 
  error_return:
   if (elf_section_data (sec)->this_hdr.contents != contents)
-    free (contents);
+    _bfd_elf_munmap_section_contents (sec, contents);
   sec->check_relocs_failed = 1;
   return false;
 }
diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
index 3300a2017bd..2e3f86840e5 100644
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -2057,7 +2057,7 @@ elf_x86_64_scan_relocs (bfd *abfd, struct bfd_link_info *info,
   /* Get the section contents.  */
   if (elf_section_data (sec)->this_hdr.contents != NULL)
     contents = elf_section_data (sec)->this_hdr.contents;
-  else if (!bfd_malloc_and_get_section (abfd, sec, &contents))
+  else if (!_bfd_elf_mmap_section_contents (abfd, sec, &contents))
     {
       sec->check_relocs_failed = 1;
       return false;
@@ -2591,7 +2591,7 @@ elf_x86_64_scan_relocs (bfd *abfd, struct bfd_link_info *info,
   if (elf_section_data (sec)->this_hdr.contents != contents)
     {
       if (!converted && !_bfd_link_keep_memory (info))
-	free (contents);
+	_bfd_elf_munmap_section_contents (sec, contents);
       else
 	{
 	  /* Cache the section contents for elf_link_input_bfd if any
@@ -2609,7 +2609,7 @@ elf_x86_64_scan_relocs (bfd *abfd, struct bfd_link_info *info,
 
  error_return:
   if (elf_section_data (sec)->this_hdr.contents != contents)
-    free (contents);
+    _bfd_elf_munmap_section_contents (sec, contents);
   sec->check_relocs_failed = 1;
   return false;
 }
@@ -5274,7 +5274,7 @@ elf_x86_64_get_synthetic_symtab (bfd *abfd,
 	continue;
 
       /* Get the PLT section contents.  */
-      if (!bfd_malloc_and_get_section (abfd, plt, &plt_contents))
+      if (!_bfd_elf_mmap_section_contents (abfd, plt, &plt_contents))
 	break;
 
       /* Check what kind of PLT it is.  */
@@ -5367,7 +5367,7 @@ elf_x86_64_get_synthetic_symtab (bfd *abfd,
 
       if (plt_type == plt_unknown)
 	{
-	  free (plt_contents);
+	  _bfd_elf_munmap_section_contents (plt, plt_contents);
 	  continue;
 	}
 
diff --git a/bfd/elfcode.h b/bfd/elfcode.h
index 1e0784611bc..39dfe0ba234 100644
--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -1195,6 +1195,7 @@ elf_checksum_contents (bfd *abfd,
       Elf_Internal_Shdr i_shdr;
       Elf_External_Shdr x_shdr;
       bfd_byte *contents, *free_contents;
+      asection *sec = NULL;
 
       i_shdr = *i_shdrp[count];
       i_shdr.sh_offset = 0;
@@ -1210,8 +1211,6 @@ elf_checksum_contents (bfd *abfd,
       contents = i_shdr.contents;
       if (contents == NULL)
 	{
-	  asection *sec;
-
 	  sec = bfd_section_from_elf_index (abfd, count);
 	  if (sec != NULL)
 	    {
@@ -1220,7 +1219,7 @@ elf_checksum_contents (bfd *abfd,
 		{
 		  /* Force rereading from file.  */
 		  sec->flags &= ~SEC_IN_MEMORY;
-		  if (!bfd_malloc_and_get_section (abfd, sec, &free_contents))
+		  if (!_bfd_elf_mmap_section_contents (abfd, sec, &free_contents))
 		    continue;
 		  contents = free_contents;
 		}
@@ -1229,7 +1228,7 @@ elf_checksum_contents (bfd *abfd,
       if (contents != NULL)
 	{
 	  (*process) (contents, i_shdr.sh_size, arg);
-	  free (free_contents);
+	  _bfd_elf_munmap_section_contents (sec, free_contents);
 	}
     }
 
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 42029f29f7a..216c124b207 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -4426,10 +4426,10 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
 	  unsigned int elfsec;
 	  unsigned long shlink;
 
-	  if (!bfd_malloc_and_get_section (abfd, s, &dynbuf))
+	  if (!_bfd_elf_mmap_section_contents (abfd, s, &dynbuf))
 	    {
 	    error_free_dyn:
-	      free (dynbuf);
+	      _bfd_elf_munmap_section_contents (s, dynbuf);
 	      goto error_return;
 	    }
 
@@ -4535,7 +4535,7 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
 		elf_tdata (abfd)->is_pie = (dyn.d_un.d_val & DF_1_PIE) != 0;
 	    }
 
-	  free (dynbuf);
+	  _bfd_elf_munmap_section_contents (s, dynbuf);
 	}
 
       /* DT_RUNPATH overrides DT_RPATH.  Do _NOT_ bfd_release, as that
@@ -8283,7 +8283,7 @@ bfd_elf_get_bfd_needed_list (bfd *abfd,
   if (s == NULL || s->size == 0 || (s->flags & SEC_HAS_CONTENTS) == 0)
     return true;
 
-  if (!bfd_malloc_and_get_section (abfd, s, &dynbuf))
+  if (!_bfd_elf_mmap_section_contents (abfd, s, &dynbuf))
     goto error_return;
 
   elfsec = _bfd_elf_section_from_bfd_section (abfd, s);
@@ -8329,12 +8329,12 @@ bfd_elf_get_bfd_needed_list (bfd *abfd,
 	}
     }
 
-  free (dynbuf);
+  _bfd_elf_munmap_section_contents (s, dynbuf);
 
   return true;
 
  error_return:
-  free (dynbuf);
+  _bfd_elf_munmap_section_contents (s, dynbuf);
   return false;
 }
 
diff --git a/bfd/elfxx-target.h b/bfd/elfxx-target.h
index 1e6992b5793..89e3d36adb2 100644
--- a/bfd/elfxx-target.h
+++ b/bfd/elfxx-target.h
@@ -148,6 +148,9 @@
 #ifndef elf_backend_strtab_flags
 #define elf_backend_strtab_flags 0
 #endif
+#ifndef elf_backend_use_mmap
+#define elf_backend_use_mmap false
+#endif
 
 #define bfd_elfNN_bfd_debug_info_start		_bfd_void_bfd
 #define bfd_elfNN_bfd_debug_info_end		_bfd_void_bfd
@@ -974,7 +977,8 @@ static const struct elf_backend_data elfNN_bed =
   elf_backend_extern_protected_data,
   elf_backend_always_renumber_dynsyms,
   elf_backend_linux_prpsinfo32_ugid16,
-  elf_backend_linux_prpsinfo64_ugid16
+  elf_backend_linux_prpsinfo64_ugid16,
+  elf_backend_use_mmap
 };
 
 /* Forward declaration for use when initialising alternative_target field.  */
diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c
index 508fd771da3..b6904273c42 100644
--- a/bfd/elfxx-x86.c
+++ b/bfd/elfxx-x86.c
@@ -1566,9 +1566,9 @@ elf_x86_size_or_finish_relative_reloc
 			  = elf_section_data (sec)->this_hdr.contents;
 		      else
 			{
-			  if (!bfd_malloc_and_get_section (sec->owner,
-							   sec,
-							   &contents))
+			  if (!_bfd_elf_mmap_section_contents (sec->owner,
+							       sec,
+							       &contents))
 			    info->callbacks->einfo
 			      /* xgettext:c-format */
 			      (_("%F%P: %pB: failed to allocate memory for section `%pA'\n"),
@@ -3789,7 +3789,7 @@ _bfd_x86_elf_get_synthetic_symtab (bfd *abfd,
     count = n;
 
   for (j = 0; plts[j].name != NULL; j++)
-    free (plts[j].contents);
+    _bfd_elf_munmap_section_contents (plts[j].sec, plts[j].contents);
 
   free (dynrelbuf);
 
diff --git a/bfd/elfxx-x86.h b/bfd/elfxx-x86.h
index b3af9b841ba..8680e3c29b5 100644
--- a/bfd/elfxx-x86.h
+++ b/bfd/elfxx-x86.h
@@ -960,6 +960,7 @@ extern void _bfd_x86_elf_link_report_relative_reloc
   _bfd_elf_x86_size_relative_relocs
 #define elf_backend_finish_relative_relocs \
   _bfd_elf_x86_finish_relative_relocs
+#define elf_backend_use_mmap true
 
 #define ELF_P_ALIGN ELF_MINPAGESIZE
 
diff --git a/bfd/libbfd.c b/bfd/libbfd.c
index a79c814a0dc..e5147a29d69 100644
--- a/bfd/libbfd.c
+++ b/bfd/libbfd.c
@@ -21,6 +21,7 @@
 
 #include "sysdep.h"
 #include "bfd.h"
+#include "elf-bfd.h"
 #include "libbfd.h"
 #include "objalloc.h"
 
@@ -1196,6 +1197,19 @@ _bfd_generic_get_section_contents (bfd *abfd,
       return false;
     }
 
+#ifdef USE_MMAP
+  if (section->mmapped_p
+      && (section->contents != NULL || location != NULL))
+    {
+      _bfd_error_handler
+	/* xgettext:c-format */
+	(_("%pB: mapped section %pA has non-NULL buffer"),
+	 abfd, section);
+      bfd_set_error (bfd_error_invalid_operation);
+      return false;
+    }
+#endif
+
   sz = bfd_get_section_limit_octets (abfd, section);
   if (offset + count < count
       || offset + count > sz
@@ -1208,8 +1222,49 @@ _bfd_generic_get_section_contents (bfd *abfd,
       return false;
     }
 
-  if (bfd_seek (abfd, section->filepos + offset, SEEK_SET) != 0
-      || bfd_read (location, count, abfd) != count)
+  if (bfd_seek (abfd, section->filepos + offset, SEEK_SET) != 0)
+    return false;
+
+#ifdef USE_MMAP
+  if (section->mmapped_p)
+    {
+      if (location != 0
+	  || bfd_get_flavour (abfd) != bfd_target_elf_flavour)
+	abort ();
+
+      int prot = ((section->reloc_count == 0)
+		  ? PROT_READ : PROT_READ | PROT_WRITE);
+
+      location = bfd_mmap_local
+	(abfd, count, prot, &elf_section_data (section)->contents_addr,
+	 &elf_section_data (section)->contents_size);
+
+      if (location == NULL)
+	return false;
+
+      /* Check for iovec not supporting mmap.  */
+      if (location != MAP_FAILED)
+	{
+	  section->contents = location;
+	  return true;
+	}
+
+      /* Malloc the buffer and call bfd_read.  */
+      location = (bfd_byte *) bfd_malloc (count);
+      if (location == NULL)
+	{
+	  if (bfd_get_error () == bfd_error_no_memory)
+	    _bfd_error_handler
+	      /* xgettext:c-format */
+	      (_("error: %pB(%pA) is too large (%#" PRIx64 " bytes)"),
+	       abfd, section, (uint64_t) count);
+	  return false;
+	}
+      section->contents = location;
+    }
+#endif
+
+  if (bfd_read (location, count, abfd) != count)
     return false;
 
   return true;
diff --git a/bfd/opncls.c b/bfd/opncls.c
index e6337b88e18..5efec37175e 100644
--- a/bfd/opncls.c
+++ b/bfd/opncls.c
@@ -164,6 +164,15 @@ static void
 _bfd_delete_bfd (bfd *abfd)
 {
 #ifdef USE_MMAP
+  if (bfd_get_flavour (abfd) == bfd_target_elf_flavour)
+    {
+      asection *sec;
+      for (sec = abfd->sections; sec != NULL; sec = sec->next)
+	if (sec->mmapped_p)
+	  munmap (elf_section_data (sec)->contents_addr,
+		  elf_section_data (sec)->contents_size);
+    }
+
   struct bfd_mmapped *mmapped, *next;
   for (mmapped = abfd->mmapped; mmapped != NULL; mmapped = next)
     {
diff --git a/bfd/section.c b/bfd/section.c
index 8cd30e80f2b..604105b39c4 100644
--- a/bfd/section.c
+++ b/bfd/section.c
@@ -422,6 +422,9 @@ CODE_FRAGMENT
 .  {* Nonzero if this section uses RELA relocations, rather than REL.  *}
 .  unsigned int use_rela_p:1;
 .
+.  {* Nonzero if this section contents are mmapped, rather than malloced.  *}
+.  unsigned int mmapped_p:1;
+.
 .  {* Bits used by various backends.  The generic code doesn't touch
 .     these fields.  *}
 .
@@ -711,8 +714,8 @@ EXTERNAL
 .  {* linker_mark, linker_has_input, gc_mark, decompress_status,     *}	\
 .     0,           0,                1,       0,			\
 .									\
-.  {* segment_mark, sec_info_type, use_rela_p,                       *}	\
-.     0,            0,             0,					\
+.  {* segment_mark, sec_info_type, use_rela_p, mmapped_p,           *}	\
+.     0,            0,             0,	       0,			\
 .									\
 .  {* sec_flg0, sec_flg1, sec_flg2, sec_flg3, sec_flg4, sec_flg5,    *}	\
 .     0,        0,        0,        0,        0,        0,		\
@@ -1625,6 +1628,8 @@ DESCRIPTION
 bool
 bfd_malloc_and_get_section (bfd *abfd, sec_ptr sec, bfd_byte **buf)
 {
+  if (sec->mmapped_p)
+    abort ();
   *buf = NULL;
   return bfd_get_full_section_contents (abfd, sec, buf);
 }
-- 
2.44.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v12 3/6] elf: Use mmap to map in symbol and relocation tables
  2024-03-17 12:19 [PATCH v12 0/6] elf: Use mmap to map in section contents H.J. Lu
  2024-03-17 12:19 ` [PATCH v12 1/6] elf: Use mmap to map in read-only sections H.J. Lu
  2024-03-17 12:19 ` [PATCH v12 2/6] elf: Add _bfd_elf_m[un]map_section_contents H.J. Lu
@ 2024-03-17 12:19 ` H.J. Lu
  2024-03-17 12:19 ` [PATCH v12 4/6] elf: Don't cache symbol nor relocation tables with mmap H.J. Lu
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 22+ messages in thread
From: H.J. Lu @ 2024-03-17 12:19 UTC (permalink / raw)
  To: binutils; +Cc: goldstein.w.n, sam, amodra

Add _bfd_mmap_read_temporary to mmap in symbol tables and relocations
whose sizes >= 4 * page size.  For the final link, allocate an external
relocation buffer of 4 * page size to avoid using mmap and munmap on
smaller relocation sections.  Since _bfd_mmap_read_temporary allocates
buffer as needed, its callers don't need to.

When mmap is used to map in all ELF sections, data to link the 3.5GB
clang executable in LLVM 17 debug build on Linux/x86-64 with 32GB RAM
is:

		stdio		mmap		improvement
user		84.79		85.27		-0.5%
system		10.95		9.09		17%
total		97.91		94.90		3%
page faults	4837944		4033778		17%

and data to link the 275M cc1plus executable in GCC 14 stage 1 build
is:

user		5.31		5.33		-0.4%
system		0.86		0.76		12%
total		6.19		6.13		1%
page faults	361273		322491		11%

	* elf.c (bfd_elf_get_elf_syms): Don't allocate buffer for external
	symbol table.  Replace bfd_read with _bfd_mmap_read_temporary.
	* elflink.c (elf_link_read_relocs_from_section): Add 2 arguments
	to return mmap memory address and size.
	(_bfd_elf_link_info_read_relocs): Don't allocate buffer for
	external relocation information.  Replace bfd_read with
	_bfd_mmap_read_temporary.
	(bfd_elf_final_link): Cache external relocations up to
	_bfd_minimum_mmap_size bytes when mmap is used.
	* libbfd.c (_bfd_mmap_read_temporary): New.
	* libbfd-in.h (_bfd_mmap_read_temporary): Likewise.
	* libbfd.h: Regenerated.
---
 bfd/elf.c       | 31 +++++++++++++----------------
 bfd/elflink.c   | 50 +++++++++++++++++++++++++----------------------
 bfd/libbfd-in.h |  3 +++
 bfd/libbfd.c    | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
 bfd/libbfd.h    |  3 +++
 5 files changed, 98 insertions(+), 41 deletions(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index 557c1ac5f4a..cb47c499704 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -460,19 +460,16 @@ bfd_elf_get_elf_syms (bfd *ibfd,
       goto out;
     }
   pos = symtab_hdr->sh_offset + symoffset * extsym_size;
-  if (extsym_buf == NULL)
-    {
-      alloc_ext = bfd_malloc (amt);
-      extsym_buf = alloc_ext;
-    }
-  if (extsym_buf == NULL
-      || bfd_seek (ibfd, pos, SEEK_SET) != 0
-      || bfd_read (extsym_buf, amt, ibfd) != amt)
+  size_t alloc_ext_size = amt;
+  if (bfd_seek (ibfd, pos, SEEK_SET) != 0
+      || !_bfd_mmap_read_temporary (&extsym_buf, &alloc_ext_size,
+				    &alloc_ext, ibfd, false))
     {
       intsym_buf = NULL;
       goto out;
     }
 
+  size_t alloc_extshndx_size = 0;
   if (shndx_hdr == NULL || shndx_hdr->sh_size == 0)
     extshndx_buf = NULL;
   else
@@ -483,15 +480,13 @@ bfd_elf_get_elf_syms (bfd *ibfd,
 	  intsym_buf = NULL;
 	  goto out;
 	}
+      alloc_extshndx_size = amt;
       pos = shndx_hdr->sh_offset + symoffset * sizeof (Elf_External_Sym_Shndx);
-      if (extshndx_buf == NULL)
-	{
-	  alloc_extshndx = (Elf_External_Sym_Shndx *) bfd_malloc (amt);
-	  extshndx_buf = alloc_extshndx;
-	}
-      if (extshndx_buf == NULL
-	  || bfd_seek (ibfd, pos, SEEK_SET) != 0
-	  || bfd_read (extshndx_buf, amt, ibfd) != amt)
+      if (bfd_seek (ibfd, pos, SEEK_SET) != 0
+	  || !_bfd_mmap_read_temporary ((void **) &extshndx_buf,
+					&alloc_extshndx_size,
+					(void **) &alloc_extshndx,
+					ibfd, false))
 	{
 	  intsym_buf = NULL;
 	  goto out;
@@ -530,8 +525,8 @@ bfd_elf_get_elf_syms (bfd *ibfd,
       }
 
  out:
-  free (alloc_ext);
-  free (alloc_extshndx);
+  _bfd_munmap_readonly_temporary (alloc_ext, alloc_ext_size);
+  _bfd_munmap_readonly_temporary (alloc_extshndx, alloc_extshndx_size);
 
   return intsym_buf;
 }
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 216c124b207..9162df60bfa 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -2644,8 +2644,11 @@ _bfd_elf_link_assign_sym_version (struct elf_link_hash_entry *h, void *data)
    may be either a REL or a RELA section.  The relocations are
    translated into RELA relocations and stored in INTERNAL_RELOCS,
    which should have already been allocated to contain enough space.
-   The EXTERNAL_RELOCS are a buffer where the external form of the
-   relocations should be stored.
+   The *EXTERNAL_RELOCS_P are a buffer where the external form of the
+   relocations should be stored.  If *EXTERNAL_RELOCS_ADDR is NULL,
+   *EXTERNAL_RELOCS_ADDR and *EXTERNAL_RELOCS_SIZE returns the mmap
+   memory address and size.  Otherwise, *EXTERNAL_RELOCS_ADDR is
+   unchanged and *EXTERNAL_RELOCS_SIZE returns 0.
 
    Returns FALSE if something goes wrong.  */
 
@@ -2653,7 +2656,8 @@ static bool
 elf_link_read_relocs_from_section (bfd *abfd,
 				   asection *sec,
 				   Elf_Internal_Shdr *shdr,
-				   void *external_relocs,
+				   void **external_relocs_addr,
+				   size_t *external_relocs_size,
 				   Elf_Internal_Rela *internal_relocs)
 {
   const struct elf_backend_data *bed;
@@ -2663,13 +2667,17 @@ elf_link_read_relocs_from_section (bfd *abfd,
   Elf_Internal_Rela *irela;
   Elf_Internal_Shdr *symtab_hdr;
   size_t nsyms;
+  void *external_relocs = *external_relocs_addr;
 
   /* Position ourselves at the start of the section.  */
   if (bfd_seek (abfd, shdr->sh_offset, SEEK_SET) != 0)
     return false;
 
   /* Read the relocations.  */
-  if (bfd_read (external_relocs, shdr->sh_size, abfd) != shdr->sh_size)
+  *external_relocs_size = shdr->sh_size;
+  if (!_bfd_mmap_read_temporary (&external_relocs,
+				 external_relocs_size,
+				 external_relocs_addr, abfd, true))
     return false;
 
   symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
@@ -2754,6 +2762,7 @@ _bfd_elf_link_info_read_relocs (bfd *abfd,
 				bool keep_memory)
 {
   void *alloc1 = NULL;
+  size_t alloc1_size;
   Elf_Internal_Rela *alloc2 = NULL;
   const struct elf_backend_data *bed = get_elf_backend_data (abfd);
   struct bfd_elf_section_data *esdo = elf_section_data (o);
@@ -2782,26 +2791,12 @@ _bfd_elf_link_info_read_relocs (bfd *abfd,
 	goto error_return;
     }
 
-  if (external_relocs == NULL)
-    {
-      bfd_size_type size = 0;
-
-      if (esdo->rel.hdr)
-	size += esdo->rel.hdr->sh_size;
-      if (esdo->rela.hdr)
-	size += esdo->rela.hdr->sh_size;
-
-      alloc1 = bfd_malloc (size);
-      if (alloc1 == NULL)
-	goto error_return;
-      external_relocs = alloc1;
-    }
-
+  alloc1 = external_relocs;
   internal_rela_relocs = internal_relocs;
   if (esdo->rel.hdr)
     {
       if (!elf_link_read_relocs_from_section (abfd, o, esdo->rel.hdr,
-					      external_relocs,
+					      &alloc1, &alloc1_size,
 					      internal_relocs))
 	goto error_return;
       external_relocs = (((bfd_byte *) external_relocs)
@@ -2812,7 +2807,7 @@ _bfd_elf_link_info_read_relocs (bfd *abfd,
 
   if (esdo->rela.hdr
       && (!elf_link_read_relocs_from_section (abfd, o, esdo->rela.hdr,
-					      external_relocs,
+					      &alloc1, &alloc1_size,
 					      internal_rela_relocs)))
     goto error_return;
 
@@ -2820,7 +2815,7 @@ _bfd_elf_link_info_read_relocs (bfd *abfd,
   if (keep_memory)
     esdo->relocs = internal_relocs;
 
-  free (alloc1);
+  _bfd_munmap_readonly_temporary (alloc1, alloc1_size);
 
   /* Don't free alloc2, since if it was allocated we are passing it
      back (under the name of internal_relocs).  */
@@ -2828,7 +2823,7 @@ _bfd_elf_link_info_read_relocs (bfd *abfd,
   return internal_relocs;
 
  error_return:
-  free (alloc1);
+  _bfd_munmap_readonly_temporary (alloc1, alloc1_size);
   if (alloc2 != NULL)
     {
       if (keep_memory)
@@ -12444,7 +12439,14 @@ bfd_elf_final_link (bfd *abfd, struct bfd_link_info *info)
      section, so that we know the sizes of the reloc sections.  We
      also figure out some maximum sizes.  */
   max_contents_size = 0;
+#ifdef USE_MMAP
+  /* Mmap is used only if section size >= the minimum mmap section
+     size.  max_external_reloc_size covers all relocation sections
+     smaller than the minimum mmap section size.   */
+  max_external_reloc_size = _bfd_minimum_mmap_size;
+#else
   max_external_reloc_size = 0;
+#endif
   max_internal_reloc_count = 0;
   max_sym_count = 0;
   max_sym_shndx_count = 0;
@@ -12533,8 +12535,10 @@ bfd_elf_final_link (bfd *abfd, struct bfd_link_info *info)
 		      if (esdi->rela.hdr != NULL)
 			ext_size += esdi->rela.hdr->sh_size;
 
+#ifndef USE_MMAP
 		      if (ext_size > max_external_reloc_size)
 			max_external_reloc_size = ext_size;
+#endif
 		      if (sec->reloc_count > max_internal_reloc_count)
 			max_internal_reloc_count = sec->reloc_count;
 		    }
diff --git a/bfd/libbfd-in.h b/bfd/libbfd-in.h
index c5a79cf932c..889b221a950 100644
--- a/bfd/libbfd-in.h
+++ b/bfd/libbfd-in.h
@@ -905,6 +905,9 @@ extern void _bfd_munmap_readonly_temporary
 #define _bfd_munmap_readonly_temporary(ptr, rsize) free (ptr)
 #endif
 
+extern bool _bfd_mmap_read_temporary
+  (void **, size_t *, void **, bfd *, bool) ATTRIBUTE_HIDDEN;
+
 static inline void *
 _bfd_malloc_and_read (bfd *abfd, bfd_size_type asize, bfd_size_type rsize)
 {
diff --git a/bfd/libbfd.c b/bfd/libbfd.c
index e5147a29d69..869f0ed5c66 100644
--- a/bfd/libbfd.c
+++ b/bfd/libbfd.c
@@ -1174,6 +1174,58 @@ _bfd_mmap_readonly_persistent (bfd *abfd, size_t rsize)
 }
 #endif
 
+/* Attempt to read *SIZE_P bytes from ABFD's iostream to *DATA_P.
+   Return true if the full the amount has been read.  If *DATA_P is
+   NULL, mmap should be used, return the memory address at the
+   current offset in *DATA_P as well as return mmap address and size
+   in *MMAP_BASE and *SIZE_P.  Otherwise, return NULL in *MMAP_BASE
+   and 0 in *SIZE_P.  If FINAL_LINK is true, this is called from
+   elf_link_read_relocs_from_section.  */
+
+bool
+_bfd_mmap_read_temporary (void **data_p, size_t *size_p,
+			  void **mmap_base, bfd *abfd,
+			  bool final_link ATTRIBUTE_UNUSED)
+{
+  void *data = *data_p;
+  size_t size = *size_p;
+
+#ifdef USE_MMAP
+  /* NB: When FINAL_LINK is true, the size of the preallocated buffer
+     is _bfd_minimum_mmap_size and use mmap if the data size >=
+     _bfd_minimum_mmap_size.  Otherwise, use mmap if ABFD isn't an IR
+     input or the data size >= _bfd_minimum_mmap_size.  */
+  bool use_mmmap;
+  bool mmap_size = size >= _bfd_minimum_mmap_size;
+  if (final_link)
+    use_mmmap = mmap_size;
+  else
+    use_mmmap = (mmap_size
+		 && data == NULL
+		 && (abfd->flags & BFD_PLUGIN) == 0);
+  if (use_mmmap)
+    {
+      data = _bfd_mmap_readonly_temporary (abfd, size, mmap_base,
+					   size_p);
+      if (data == NULL || data == MAP_FAILED)
+	abort ();
+      *data_p = data;
+      return true;
+    }
+#endif
+
+  if (data == NULL)
+    {
+      data = bfd_malloc (size);
+      if (data == NULL)
+	return false;
+      *data_p = data;
+    }
+  *mmap_base = NULL;
+  *size_p = 0;
+  return bfd_read (data, size, abfd) == size;
+}
+
 /* Default implementation */
 
 bool
diff --git a/bfd/libbfd.h b/bfd/libbfd.h
index 47f40889a95..876c1f223c7 100644
--- a/bfd/libbfd.h
+++ b/bfd/libbfd.h
@@ -911,6 +911,9 @@ extern void _bfd_munmap_readonly_temporary
 #define _bfd_munmap_readonly_temporary(ptr, rsize) free (ptr)
 #endif
 
+extern bool _bfd_mmap_read_temporary
+  (void **, size_t *, void **, bfd *, bool) ATTRIBUTE_HIDDEN;
+
 static inline void *
 _bfd_malloc_and_read (bfd *abfd, bfd_size_type asize, bfd_size_type rsize)
 {
-- 
2.44.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v12 4/6] elf: Don't cache symbol nor relocation tables with mmap
  2024-03-17 12:19 [PATCH v12 0/6] elf: Use mmap to map in section contents H.J. Lu
                   ` (2 preceding siblings ...)
  2024-03-17 12:19 ` [PATCH v12 3/6] elf: Use mmap to map in symbol and relocation tables H.J. Lu
@ 2024-03-17 12:19 ` H.J. Lu
  2024-03-17 12:19 ` [PATCH v12 5/6] elf: Always keep symbol table and relocation info for eh_frame H.J. Lu
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 22+ messages in thread
From: H.J. Lu @ 2024-03-17 12:19 UTC (permalink / raw)
  To: binutils; +Cc: goldstein.w.n, sam, amodra

During a "-j 8" LLVM 17 debug build on a machine with 32GB RAM and 16GB
swap, ld was killed by kernel because of out of memory:

[79437.949336] oom-kill:constraint=CONSTRAINT_NONE,nodemask=(null),cpuset=/,mems_allowed=0,global_oom,task_memcg=/user.slice/user-1000.slice/session-9.scope,task=ld,pid=797431,uid=1000
[79437.949349] Out of memory: Killed process 797431 (ld) total-vm:9219600kB, anon-rss:6558156kB, file-rss:1792kB, shmem-rss:0kB, UID:1000 pgtables:17552kB oom_score_adj:0

Don't cache symbol nor relocation tables if they are mapped in.  Data to
link the 3.5GB clang executable in LLVM 17 debug build on Linux/x86-64
with 32GB RAM is:

		stdio		mmap		improvement
user		86.73		87.02		-0.3%
system		9.55		9.21		3.6%
total		100.40		97.66		0.7%
maximum set(GB)	17.34		13.14		24%
page faults	4047667		3042877		25%

and data to link the 275M cc1plus executable in GCC 14 stage 1 build is:

user		5.41		5.44		-0.5%
system		0.80		0.76		5%
total		6.25		6.26		-0.2%
maximum set(MB)	1323		968		27%
page faults	323451		236371		27%

These improve the overall system performance for parallel build by
reducing memory usage and page faults.

Also rename _bfd_link_keep_memory to _bfd_elf_link_keep_memory.  Since
the --no-keep-memory linker option causes:

https://sourceware.org/bugzilla/show_bug.cgi?id=31458

this is opt-in by each backend.

bfd/

	* elf32-i386.c (elf_i386_scan_relocs): Remove
	_bfd_link_keep_memory.
	* elf64-x86-64.c (elf_x86_64_scan_relocs): Likewise.
	* elflink.c (_bfd_elf_link_keep_memory): New.
	(_bfd_elf_link_iterate_on_relocs): Replace _bfd_link_keep_memory
	with _bfd_elf_link_keep_memory.
	(elf_link_add_object_symbols): Likewise.
	(init_reloc_cookie): Likewise.
	(init_reloc_cookie_rels): Likewise.
	* libbfd-in.h (_bfd_link_keep_memory): Removed.
	* linker.c (_bfd_link_keep_memory): Likewise.
	* libbfd.h: Regenerated.
---
 bfd/elf32-i386.c   |  2 +-
 bfd/elf64-x86-64.c |  2 +-
 bfd/elflink.c      | 69 ++++++++++++++++++++++++++++++++++++++--------
 bfd/libbfd-in.h    |  3 --
 bfd/libbfd.h       |  3 --
 bfd/linker.c       | 35 -----------------------
 6 files changed, 59 insertions(+), 55 deletions(-)

diff --git a/bfd/elf32-i386.c b/bfd/elf32-i386.c
index 0f963c373be..8a6c0fa9060 100644
--- a/bfd/elf32-i386.c
+++ b/bfd/elf32-i386.c
@@ -1932,7 +1932,7 @@ elf_i386_scan_relocs (bfd *abfd,
 
   if (elf_section_data (sec)->this_hdr.contents != contents)
     {
-      if (!converted && !_bfd_link_keep_memory (info))
+      if (!converted)
 	_bfd_elf_munmap_section_contents (sec, contents);
       else
 	{
diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
index 2e3f86840e5..7756c5b1744 100644
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -2590,7 +2590,7 @@ elf_x86_64_scan_relocs (bfd *abfd, struct bfd_link_info *info,
 
   if (elf_section_data (sec)->this_hdr.contents != contents)
     {
-      if (!converted && !_bfd_link_keep_memory (info))
+      if (!converted)
 	_bfd_elf_munmap_section_contents (sec, contents);
       else
 	{
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 9162df60bfa..dcbe6e1a18c 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -49,6 +49,53 @@ struct elf_info_failed
 static bool _bfd_elf_fix_symbol_flags
   (struct elf_link_hash_entry *, struct elf_info_failed *);
 
+/* Return false if linker should avoid caching relocation information
+   and symbol tables of input files in memory.  */
+
+static bool
+_bfd_elf_link_keep_memory (struct bfd_link_info *info)
+{
+#ifdef USE_MMAP
+  /* Don't cache symbol nor relocation tables if they are mapped in.
+     NB: Since the --no-keep-memory linker option causes:
+
+     https://sourceware.org/bugzilla/show_bug.cgi?id=31458
+
+     this is opt-in by each backend.  */
+  const struct elf_backend_data *bed
+    = get_elf_backend_data (info->output_bfd);
+  if (bed->use_mmap)
+    return false;
+#endif
+  bfd *abfd;
+  bfd_size_type size;
+
+  if (!info->keep_memory)
+    return false;
+
+  if (info->max_cache_size == (bfd_size_type) -1)
+    return true;
+
+  abfd = info->input_bfds;
+  size = info->cache_size;
+  do
+    {
+      if (size >= info->max_cache_size)
+	{
+	  /* Over the limit.  Reduce the memory usage.  */
+	  info->keep_memory = false;
+	  return false;
+	}
+      if (!abfd)
+	break;
+      size += abfd->alloc_size;
+      abfd = abfd->link.next;
+    }
+  while (1);
+
+  return true;
+}
+
 asection *
 _bfd_elf_section_for_symbol (struct elf_reloc_cookie *cookie,
 			     unsigned long r_symndx,
@@ -4182,10 +4229,9 @@ _bfd_elf_link_iterate_on_relocs
 	      || bfd_is_abs_section (o->output_section))
 	    continue;
 
-	  internal_relocs = _bfd_elf_link_info_read_relocs (abfd, info,
-							    o, NULL,
-							    NULL,
-							    _bfd_link_keep_memory (info));
+	  internal_relocs = _bfd_elf_link_info_read_relocs
+	    (abfd, info, o, NULL, NULL,
+	     _bfd_elf_link_keep_memory (info));
 	  if (internal_relocs == NULL)
 	    return false;
 
@@ -5551,10 +5597,9 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
 		  && (s->flags & SEC_DEBUGGING) != 0))
 	    continue;
 
-	  internal_relocs = _bfd_elf_link_info_read_relocs (abfd, info,
-							    s, NULL,
-							    NULL,
-							    _bfd_link_keep_memory (info));
+	  internal_relocs = _bfd_elf_link_info_read_relocs
+	    (abfd, info, s, NULL, NULL,
+	     _bfd_elf_link_keep_memory (info));
 	  if (internal_relocs == NULL)
 	    goto error_free_vers;
 
@@ -13614,7 +13659,7 @@ init_reloc_cookie (struct elf_reloc_cookie *cookie,
 	  info->callbacks->einfo (_("%P%X: can not read symbols: %E\n"));
 	  return false;
 	}
-      if (_bfd_link_keep_memory (info) )
+      if (_bfd_elf_link_keep_memory (info) )
 	{
 	  symtab_hdr->contents = (bfd_byte *) cookie->locsyms;
 	  info->cache_size += (cookie->locsymcount
@@ -13651,9 +13696,9 @@ init_reloc_cookie_rels (struct elf_reloc_cookie *cookie,
     }
   else
     {
-      cookie->rels = _bfd_elf_link_info_read_relocs (abfd, info, sec,
-						     NULL, NULL,
-						     _bfd_link_keep_memory (info));
+      cookie->rels = _bfd_elf_link_info_read_relocs
+	(abfd, info, sec, NULL, NULL,
+	 _bfd_elf_link_keep_memory (info));
       if (cookie->rels == NULL)
 	return false;
       cookie->rel = cookie->rels;
diff --git a/bfd/libbfd-in.h b/bfd/libbfd-in.h
index 889b221a950..7bfc58f12d8 100644
--- a/bfd/libbfd-in.h
+++ b/bfd/libbfd-in.h
@@ -848,9 +848,6 @@ extern bfd_byte * _bfd_write_unsigned_leb128
 extern struct bfd_link_info *_bfd_get_link_info (bfd *)
   ATTRIBUTE_HIDDEN;
 
-extern bool _bfd_link_keep_memory (struct bfd_link_info *)
-  ATTRIBUTE_HIDDEN;
-
 extern uintptr_t _bfd_pagesize ATTRIBUTE_HIDDEN;
 extern uintptr_t _bfd_pagesize_m1 ATTRIBUTE_HIDDEN;
 extern uintptr_t _bfd_minimum_mmap_size ATTRIBUTE_HIDDEN;
diff --git a/bfd/libbfd.h b/bfd/libbfd.h
index 876c1f223c7..36fe6c0ebd0 100644
--- a/bfd/libbfd.h
+++ b/bfd/libbfd.h
@@ -854,9 +854,6 @@ extern bfd_byte * _bfd_write_unsigned_leb128
 extern struct bfd_link_info *_bfd_get_link_info (bfd *)
   ATTRIBUTE_HIDDEN;
 
-extern bool _bfd_link_keep_memory (struct bfd_link_info *)
-  ATTRIBUTE_HIDDEN;
-
 extern uintptr_t _bfd_pagesize ATTRIBUTE_HIDDEN;
 extern uintptr_t _bfd_pagesize_m1 ATTRIBUTE_HIDDEN;
 extern uintptr_t _bfd_minimum_mmap_size ATTRIBUTE_HIDDEN;
diff --git a/bfd/linker.c b/bfd/linker.c
index 36cca9624c2..eb42a78b622 100644
--- a/bfd/linker.c
+++ b/bfd/linker.c
@@ -3556,38 +3556,3 @@ _bfd_nolink_bfd_define_start_stop (struct bfd_link_info *info ATTRIBUTE_UNUSED,
 {
   return (struct bfd_link_hash_entry *) _bfd_ptr_bfd_null_error (sec->owner);
 }
-
-/* Return false if linker should avoid caching relocation infomation
-   and symbol tables of input files in memory.  */
-
-bool
-_bfd_link_keep_memory (struct bfd_link_info * info)
-{
-  bfd *abfd;
-  bfd_size_type size;
-
-  if (!info->keep_memory)
-    return false;
-
-  if (info->max_cache_size == (bfd_size_type) -1)
-    return true;
-
-  abfd = info->input_bfds;
-  size = info->cache_size;
-  do
-    {
-      if (size >= info->max_cache_size)
-	{
-	  /* Over the limit.  Reduce the memory usage.  */
-	  info->keep_memory = false;
-	  return false;
-	}
-      if (!abfd)
-	break;
-      size += abfd->alloc_size;
-      abfd = abfd->link.next;
-    }
-  while (1);
-
-  return true;
-}
-- 
2.44.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v12 5/6] elf: Always keep symbol table and relocation info for eh_frame
  2024-03-17 12:19 [PATCH v12 0/6] elf: Use mmap to map in section contents H.J. Lu
                   ` (3 preceding siblings ...)
  2024-03-17 12:19 ` [PATCH v12 4/6] elf: Don't cache symbol nor relocation tables with mmap H.J. Lu
@ 2024-03-17 12:19 ` H.J. Lu
  2024-03-17 12:19 ` [PATCH v12 6/6] elf: Add _bfd_elf_link_m[un]map_section_contents H.J. Lu
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 22+ messages in thread
From: H.J. Lu @ 2024-03-17 12:19 UTC (permalink / raw)
  To: binutils; +Cc: goldstein.w.n, sam, amodra

When --no-keep-memory is used, the symbol table and relocation info for
eh_frame are freed after they are retrieved for each text section in the
input object.  If an input object has many text sections, the same data
is retrieved and freed many times which can take a very long time.
Update _bfd_elf_gc_mark to keep the symbol table and relocation info for
eh_frame to avoid it.  Data to link the 3.5GB clang executable in LLVM
17 debug build on Linux/x86-64 with 32GB RAM is:

		before		after		improvement
user		86.31		86.44		-0.2%
system		8.77		8.63		1.6%
total		95.58		96.81		-1.3%
maximum set(GB)	13.1		13.1		0%
page faults	3024752		3028699		-1.3%

and data to link the 275M cc1plus executable in GCC 14 stage 1 build is:

user		5.49		5.46		-0.5%
system		0.73		0.73		0%
total		6.26		6.25		0.3%
maximum set(MB)	964		964		0%
page faults	235173		235796		-0.3%

The memory usage impact is minimum and the link time of the Rust binary
in

https://sourceware.org/bugzilla/show_bug.cgi?id=31466

is reduced from 500+ seconds to 1.44 seconds, a 300x speedup.

	PR ld/31466
	* elflink.c (init_reloc_cookie): Add a bool argument, keep_memory,
	for keeping memory.  Always keep memory if keep_memory is true.
	(init_reloc_cookie_rels): Likewise
	(init_reloc_cookie_for_section): Add a bool argument for keeping
	memory and pass it to init_reloc_cookie and
	init_reloc_cookie_rels.
	(_bfd_elf_gc_mark_reloc): Pass false to _bfd_elf_gc_mark.
	(_bfd_elf_gc_mark): Pass true to init_reloc_cookie_for_section
	for the eh_frame section.  Pass false to
	init_reloc_cookie_for_section for other sections.
	(_bfd_elf_gc_mark_extra_sections): Add Add a bool argument for
	keeping memory and pass it to _bfd_elf_gc_mark.
	(bfd_elf_parse_eh_frame_entries): Pass false to init_reloc_cookie
	and init_reloc_cookie_rels.
	(bfd_elf_gc_sections): Pass false to init_reloc_cookie_for_section
	and _bfd_elf_gc_mark.
	(bfd_elf_discard_info): Pass false to
	init_reloc_cookie_for_section and init_reloc_cookie.
---
 bfd/elflink.c | 43 +++++++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/bfd/elflink.c b/bfd/elflink.c
index dcbe6e1a18c..601d15e9cef 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -13621,7 +13621,8 @@ bfd_elf_final_link (bfd *abfd, struct bfd_link_info *info)
 
 static bool
 init_reloc_cookie (struct elf_reloc_cookie *cookie,
-		   struct bfd_link_info *info, bfd *abfd)
+		   struct bfd_link_info *info, bfd *abfd,
+		   bool keep_memory)
 {
   Elf_Internal_Shdr *symtab_hdr;
   const struct elf_backend_data *bed;
@@ -13659,7 +13660,7 @@ init_reloc_cookie (struct elf_reloc_cookie *cookie,
 	  info->callbacks->einfo (_("%P%X: can not read symbols: %E\n"));
 	  return false;
 	}
-      if (_bfd_elf_link_keep_memory (info) )
+      if (keep_memory || _bfd_elf_link_keep_memory (info))
 	{
 	  symtab_hdr->contents = (bfd_byte *) cookie->locsyms;
 	  info->cache_size += (cookie->locsymcount
@@ -13687,7 +13688,7 @@ fini_reloc_cookie (struct elf_reloc_cookie *cookie, bfd *abfd)
 static bool
 init_reloc_cookie_rels (struct elf_reloc_cookie *cookie,
 			struct bfd_link_info *info, bfd *abfd,
-			asection *sec)
+			asection *sec, bool keep_memory)
 {
   if (sec->reloc_count == 0)
     {
@@ -13698,7 +13699,7 @@ init_reloc_cookie_rels (struct elf_reloc_cookie *cookie,
     {
       cookie->rels = _bfd_elf_link_info_read_relocs
 	(abfd, info, sec, NULL, NULL,
-	 _bfd_elf_link_keep_memory (info));
+	 keep_memory || _bfd_elf_link_keep_memory (info));
       if (cookie->rels == NULL)
 	return false;
       cookie->rel = cookie->rels;
@@ -13724,11 +13725,12 @@ fini_reloc_cookie_rels (struct elf_reloc_cookie *cookie,
 static bool
 init_reloc_cookie_for_section (struct elf_reloc_cookie *cookie,
 			       struct bfd_link_info *info,
-			       asection *sec)
+			       asection *sec, bool keep_memory)
 {
-  if (!init_reloc_cookie (cookie, info, sec->owner))
+  if (!init_reloc_cookie (cookie, info, sec->owner, keep_memory))
     goto error1;
-  if (!init_reloc_cookie_rels (cookie, info, sec->owner, sec))
+  if (!init_reloc_cookie_rels (cookie, info, sec->owner, sec,
+			       keep_memory))
     goto error2;
   return true;
 
@@ -13939,7 +13941,7 @@ _bfd_elf_gc_mark (struct bfd_link_info *info,
     {
       struct elf_reloc_cookie cookie;
 
-      if (!init_reloc_cookie_for_section (&cookie, info, sec))
+      if (!init_reloc_cookie_for_section (&cookie, info, sec, false))
 	ret = false;
       else
 	{
@@ -13957,7 +13959,14 @@ _bfd_elf_gc_mark (struct bfd_link_info *info,
     {
       struct elf_reloc_cookie cookie;
 
-      if (!init_reloc_cookie_for_section (&cookie, info, eh_frame))
+      /* NB: When --no-keep-memory is used, the symbol table and
+	 relocation info for eh_frame are freed after they are retrieved
+	 for each text section in the input object.  If an input object
+	 has many text sections, the same data is retrieved and freed
+	 many times which can take a very long time.  Always keep the
+	 symbol table and relocation info for eh_frame to avoid it.  */
+      if (!init_reloc_cookie_for_section (&cookie, info, eh_frame,
+					  true))
 	ret = false;
       else
 	{
@@ -14397,13 +14406,14 @@ bfd_elf_parse_eh_frame_entries (bfd *abfd ATTRIBUTE_UNUSED,
       if (sec == NULL || sec->sec_info_type == SEC_INFO_TYPE_JUST_SYMS)
 	continue;
 
-      if (!init_reloc_cookie (&cookie, info, ibfd))
+      if (!init_reloc_cookie (&cookie, info, ibfd, false))
 	return false;
 
       for (sec = ibfd->sections; sec; sec = sec->next)
 	{
 	  if (startswith (bfd_section_name (sec), ".eh_frame_entry")
-	      && init_reloc_cookie_rels (&cookie, info, ibfd, sec))
+	      && init_reloc_cookie_rels (&cookie, info, ibfd, sec,
+					 false))
 	    {
 	      _bfd_elf_parse_eh_frame_entry (info, sec, &cookie);
 	      fini_reloc_cookie_rels (&cookie, sec);
@@ -14448,7 +14458,8 @@ bfd_elf_gc_sections (bfd *abfd, struct bfd_link_info *info)
       if (sec == NULL || sec->sec_info_type == SEC_INFO_TYPE_JUST_SYMS)
 	continue;
       sec = bfd_get_section_by_name (sub, ".eh_frame");
-      while (sec && init_reloc_cookie_for_section (&cookie, info, sec))
+      while (sec && init_reloc_cookie_for_section (&cookie, info, sec,
+						   false))
 	{
 	  _bfd_elf_parse_eh_frame (sub, info, sec, &cookie);
 	  if (elf_section_data (sec)->sec_info
@@ -14960,7 +14971,7 @@ bfd_elf_discard_info (bfd *output_bfd, struct bfd_link_info *info)
 	  if (bfd_get_flavour (abfd) != bfd_target_elf_flavour)
 	    continue;
 
-	  if (!init_reloc_cookie_for_section (&cookie, info, i))
+	  if (!init_reloc_cookie_for_section (&cookie, info, i, false))
 	    return -1;
 
 	  if (_bfd_discard_section_stabs (abfd, i,
@@ -14991,7 +15002,7 @@ bfd_elf_discard_info (bfd *output_bfd, struct bfd_link_info *info)
 	  if (bfd_get_flavour (abfd) != bfd_target_elf_flavour)
 	    continue;
 
-	  if (!init_reloc_cookie_for_section (&cookie, info, i))
+	  if (!init_reloc_cookie_for_section (&cookie, info, i, false))
 	    return -1;
 
 	  _bfd_elf_parse_eh_frame (abfd, info, i, &cookie);
@@ -15056,7 +15067,7 @@ bfd_elf_discard_info (bfd *output_bfd, struct bfd_link_info *info)
 	  if (bfd_get_flavour (abfd) != bfd_target_elf_flavour)
 	    continue;
 
-	  if (!init_reloc_cookie_for_section (&cookie, info, i))
+	  if (!init_reloc_cookie_for_section (&cookie, info, i, false))
 	    return -1;
 
 	  if (_bfd_elf_parse_sframe (abfd, info, i, &cookie))
@@ -15092,7 +15103,7 @@ bfd_elf_discard_info (bfd *output_bfd, struct bfd_link_info *info)
 
       if (bed->elf_backend_discard_info != NULL)
 	{
-	  if (!init_reloc_cookie (&cookie, info, abfd))
+	  if (!init_reloc_cookie (&cookie, info, abfd, false))
 	    return -1;
 
 	  if ((*bed->elf_backend_discard_info) (abfd, &cookie, info))
-- 
2.44.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v12 6/6] elf: Add _bfd_elf_link_m[un]map_section_contents
  2024-03-17 12:19 [PATCH v12 0/6] elf: Use mmap to map in section contents H.J. Lu
                   ` (4 preceding siblings ...)
  2024-03-17 12:19 ` [PATCH v12 5/6] elf: Always keep symbol table and relocation info for eh_frame H.J. Lu
@ 2024-03-17 12:19 ` H.J. Lu
  2024-03-28 13:29 ` PING: [PATCH v12 0/6] elf: Use mmap to map in section contents H.J. Lu
  2024-04-04 13:12 ` Luis Machado
  7 siblings, 0 replies; 22+ messages in thread
From: H.J. Lu @ 2024-03-17 12:19 UTC (permalink / raw)
  To: binutils; +Cc: goldstein.w.n, sam, amodra

To copy input section contents, add _bfd_elf_link_mmap_section_contents
and _bfd_elf_link_munmap_section_contents to mmap in the input sections.

	* elf-bfd.h (_bfd_elf_link_mmap_section_contents): New.
	(_bfd_elf_link_munmap_section_contents): Likewise.
	* elf.c (elf_mmap_section_contents): New.
	(_bfd_elf_mmap_section_contents): Use it.
	(_bfd_elf_link_mmap_section_contents): New.
	(_bfd_elf_link_munmap_section_contents): Likewise.
	* elflink.c (elf_link_input_bfd): Call
	_bfd_elf_link_mmap_section_contents instead of
	bfd_get_full_section_contents.  Call
	_bfd_elf_link_munmap_section_contents to munmap the section
	contents.
	(bfd_elf_final_link): When mmap is used, initialize
	max_contents_size to _bfd_minimum_mmap_size and increase it
	for compressed or linker created sections or sections whose
	rawsize != size.
---
 bfd/elf-bfd.h |  4 ++++
 bfd/elf.c     | 57 +++++++++++++++++++++++++++++++++++++++++++++++----
 bfd/elflink.c | 33 +++++++++++++++++++++--------
 3 files changed, 82 insertions(+), 12 deletions(-)

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index e7c2cd19bed..0fcef04af36 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -3143,6 +3143,10 @@ extern bool _bfd_elf_mmap_section_contents
   (bfd *abfd, asection *section, bfd_byte **buf);
 extern void _bfd_elf_munmap_section_contents
   (asection *, void *);
+extern bool _bfd_elf_link_mmap_section_contents
+  (bfd *abfd, asection *section, bfd_byte **buf);
+extern void _bfd_elf_link_munmap_section_contents
+  (asection *);
 
 /* Large common section.  */
 extern asection _bfd_elf_large_com_section;
diff --git a/bfd/elf.c b/bfd/elf.c
index cb47c499704..f66ac9289df 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -14388,10 +14388,12 @@ _bfd_elf_write_secondary_reloc_section (bfd *abfd, asection *sec)
   return result;
 }
 
-/* Mmap in section contents.  */
+/* Mmap in section contents.  If FINAL_LINK is false, set *BUF to NULL
+   before calling bfd_get_full_section_contents.  */
 
-bool
-_bfd_elf_mmap_section_contents (bfd *abfd, sec_ptr sec, bfd_byte **buf)
+static bool
+elf_mmap_section_contents (bfd *abfd, sec_ptr sec, bfd_byte **buf,
+			   bool final_link)
 {
 #ifdef USE_MMAP
   const struct elf_backend_data *bed = get_elf_backend_data (abfd);
@@ -14415,16 +14417,41 @@ _bfd_elf_mmap_section_contents (bfd *abfd, sec_ptr sec, bfd_byte **buf)
 	  if (sec->mmapped_p)
 	    abort ();
 	  sec->mmapped_p = 1;
+
+	  /* Never use the preallocated buffer if mmapp is used.  */
+	  *buf = NULL;
 	}
     }
 #endif
-  *buf = NULL;
+  /* NB: When this is called from elf_link_input_bfd, FINAL_LINK is
+     true.  If FINAL_LINK is false, *BUF is set to the preallocated
+     buffer if USE_MMAP is undefined and *BUF is set to NULL if
+     USE_MMAP is defined.  */
+  if (!final_link)
+    *buf = NULL;
   bool ret = bfd_get_full_section_contents (abfd, sec, buf);
   if (ret && sec->mmapped_p)
     *buf = sec->contents;
   return ret;
 }
 
+/* Mmap in section contents.  */
+
+bool
+_bfd_elf_mmap_section_contents (bfd *abfd, sec_ptr sec, bfd_byte **buf)
+{
+  return elf_mmap_section_contents (abfd, sec, buf, false);
+}
+
+/* Mmap in the full section contents for the final link.  */
+
+bool
+_bfd_elf_link_mmap_section_contents (bfd *abfd, sec_ptr sec,
+				     bfd_byte **buf)
+{
+  return elf_mmap_section_contents (abfd, sec, buf, true);
+}
+
 /* Munmap section contents.  */
 
 void
@@ -14464,3 +14491,25 @@ _bfd_elf_munmap_section_contents (asection *sec ATTRIBUTE_UNUSED,
 
   free (contents);
 }
+
+/* Munmap the full section contents for the final link.  */
+
+void
+_bfd_elf_link_munmap_section_contents (asection *sec ATTRIBUTE_UNUSED)
+{
+#ifdef USE_MMAP
+  if (sec->mmapped_p && elf_section_data (sec)->contents_addr != NULL)
+    {
+      /* When _bfd_elf_link_mmap_section_contents returns CONTENTS as
+	 malloced, CONTENTS_ADDR is set to NULL.  */
+      /* NB: CONTENTS_ADDR and CONTENTS_SIZE must be valid.  */
+      if (munmap (elf_section_data (sec)->contents_addr,
+		  elf_section_data (sec)->contents_size) != 0)
+	abort ();
+      sec->mmapped_p = 0;
+      sec->contents = NULL;
+      elf_section_data (sec)->contents_addr = NULL;
+      elf_section_data (sec)->contents_size = 0;
+    }
+#endif
+}
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 601d15e9cef..ab3fa308d44 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -11487,7 +11487,8 @@ elf_link_input_bfd (struct elf_final_link_info *flinfo, bfd *input_bfd)
       else
 	{
 	  contents = flinfo->contents;
-	  if (! bfd_get_full_section_contents (input_bfd, o, &contents))
+	  if (! _bfd_elf_link_mmap_section_contents (input_bfd, o,
+						     &contents))
 	    return false;
 	}
 
@@ -12045,6 +12046,9 @@ elf_link_input_bfd (struct elf_final_link_info *flinfo, bfd *input_bfd)
 	  }
 	  break;
 	}
+
+      /* Munmap the section contents for each input section.  */
+      _bfd_elf_link_munmap_section_contents (o);
     }
 
   return true;
@@ -12483,13 +12487,17 @@ bfd_elf_final_link (bfd *abfd, struct bfd_link_info *info)
   /* Count up the number of relocations we will output for each output
      section, so that we know the sizes of the reloc sections.  We
      also figure out some maximum sizes.  */
-  max_contents_size = 0;
 #ifdef USE_MMAP
   /* Mmap is used only if section size >= the minimum mmap section
-     size.  max_external_reloc_size covers all relocation sections
-     smaller than the minimum mmap section size.   */
+     size.  The initial max_contents_size value covers all sections
+     smaller than the minimum mmap section size.  It may be increased
+     for compressed or linker created sections or sections whose
+     rawsize != size.  max_external_reloc_size covers all relocation
+     sections smaller than the minimum mmap section size.  */
+  max_contents_size = _bfd_minimum_mmap_size;
   max_external_reloc_size = _bfd_minimum_mmap_size;
 #else
+  max_contents_size = 0;
   max_external_reloc_size = 0;
 #endif
   max_internal_reloc_count = 0;
@@ -12525,10 +12533,19 @@ bfd_elf_final_link (bfd *abfd, struct bfd_link_info *info)
 	      if (sec->flags & SEC_MERGE)
 		merged = true;
 
-	      if (sec->rawsize > max_contents_size)
-		max_contents_size = sec->rawsize;
-	      if (sec->size > max_contents_size)
-		max_contents_size = sec->size;
+#ifdef USE_MMAP
+	      /* Mmap is used only on non-compressed, non-linker created
+		 sections whose rawsize == size.  */
+	      if (sec->compress_status != COMPRESS_SECTION_NONE
+		 || (sec->flags & SEC_LINKER_CREATED) != 0
+		 || sec->rawsize != sec->size)
+#endif
+		{
+		  if (sec->rawsize > max_contents_size)
+		    max_contents_size = sec->rawsize;
+		  if (sec->size > max_contents_size)
+		    max_contents_size = sec->size;
+		}
 
 	      if (bfd_get_flavour (sec->owner) == bfd_target_elf_flavour
 		  && (sec->owner->flags & DYNAMIC) == 0)
-- 
2.44.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* PING: [PATCH v12 0/6] elf: Use mmap to map in section contents
  2024-03-17 12:19 [PATCH v12 0/6] elf: Use mmap to map in section contents H.J. Lu
                   ` (5 preceding siblings ...)
  2024-03-17 12:19 ` [PATCH v12 6/6] elf: Add _bfd_elf_link_m[un]map_section_contents H.J. Lu
@ 2024-03-28 13:29 ` H.J. Lu
  2024-04-03 16:03   ` Nick Clifton
  2024-04-04 13:12 ` Luis Machado
  7 siblings, 1 reply; 22+ messages in thread
From: H.J. Lu @ 2024-03-28 13:29 UTC (permalink / raw)
  To: Binutils; +Cc: Noah Goldstein, Sam James, Alan Modra

On Sun, Mar 17, 2024 at 5:19 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> Changes in v12:
>
> 1. Don't call _bfd_elf_link_keep_memory for x86 since it always returns
> false.
> 2. Make _bfd_elf_link_keep_memory static.
>
> Changes in v11:
>
> 1. Update _bfd_mmap_read_temporary to allocate buffer as needed.
> 2. Don't allocate buffer when _bfd_mmap_read_temporary is called.
>
> Changes in v10:
>
> 1. Malloc a 4 * page size buffer for external relocations for the final
> link to avoid mmap/munmap on small relocation sections.
> 2. Malloc a buffer for input section contents which are smaller than
> 4 * page size or can't be mapped for the final link.
>
> Changes in v9:
>
> 1. Use MAP_FAILED for mmap failure.
>
> Changes in v8:
>
> 1. Rebase against master branch.
> 2. Add _bfd_elf_link_mmap_section_contents and
> _bfd_elf_link_munmap_section_contents.
>
> Changes in v7:
>
> 1. Don't add the --keep-memory linker option.
>
> Changes in v6:
>
> 1. Add the --keep-memory linker option and always cache symbol and
> relocation tables for --keep-memory.
> 2. Always keep symbol table and relocation info for eh_frame to speedup
> the Rust binary build by ~300x:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=31466
>
> Changes in v5:
>
> 1. Drop 2 patches which have been merged onto master branch.
> 2. Rename _bfd_elf_mmap_section to _bfd_elf_mmap_section_contents.
> 3. Rename _bfd_mmap_readonly_tracked, _bfd_mmap_readonly_untracked,
> _bfd_munmap_readonly_untracked, _bfd_mmap_read_untracked to
> _bfd_mmap_readonly_persistent, _bfd_mmap_readonly_temporary,
> _bfd_munmap_readonly_temporary and _bfd_mmap_read_temporary.
> 4. Drop the setup_group change.
> 5. Fix a typo.
> 6. Update comments.
>
> Changes in v4:
>
> 1. Change don't cache symbol nor relocation tables with mmap to opt-in.
>
> Changes in v3:
>
> 1. Fix non-mmap build.
> 2. Change the argument name of bfd_mmap_local from flags to prot since
> its values are PROT_XXX.
>
> Changes in v2:
>
> 1. Don't hard-code BFD_JUMP_TABLE_COPY in bfd so that elf-bfd.h can be
> included in libbfd.c.
> 2. Change the --with-mmap default to true.
> 3. Check USE_MMAP instead of HAVE_MMAP.
> 4. Remove the asize parameter to _bfd_mmap_readonly_tracked.
> 5. Add contents_addr and contents_size to bfd_elf_section_data.
> 6. Rename _bfd_link_keep_memory to _bfd_elf_link_keep_memory.
>
> ---
> We can use mmap to map in ELF section contents, instead of copying them
> into memory by hand.  We don't need to cache symbol nor relocation tables
> if they are mapped in.  Data to link the 3.5GB clang executable in LLVM
> 17 debug build on Linux/x86-64 with 32GB RAM is:
>
>                 stdio           mmap            improvement
> user            86.73           87.02           -0.3%
> system          9.55            9.21            3.6%
> total           100.40          97.66           0.7%
> maximum set(GB) 17.34           13.14           24%
> page faults     4047667         3042877         25%
>
> and data to link the 275M cc1plus executable in GCC 14 stage 1 build is:
>
> user            5.41            5.44            -0.5%
> system          0.80            0.76            5%
> total           6.25            6.26            -0.2%
> maximum set(MB) 1323            968             27%
> page faults     323451          236371          27%
>
> Data shows that these won't improve the single copy linker performance.
> But they improve the overall system performance when linker is used by
> reducing linker memory usage and page faults.  They allow more parallel
> linker jobs on LLVM debug build.
>
> Here is a quote from Noah Goldstein: "on a large project they are an
> extremely large speedup".
>
> H.J. Lu (6):
>   elf: Use mmap to map in read-only sections
>   elf: Add _bfd_elf_m[un]map_section_contents
>   elf: Use mmap to map in symbol and relocation tables
>   elf: Don't cache symbol nor relocation tables with mmap
>   elf: Always keep symbol table and relocation info for eh_frame
>   elf: Add _bfd_elf_link_m[un]map_section_contents
>
>  bfd/bfd-in2.h      |  24 ++++-
>  bfd/bfd.c          |  17 +++
>  bfd/bfdwin.c       |   8 +-
>  bfd/cache.c        |   7 +-
>  bfd/compress.c     |   2 +-
>  bfd/elf-bfd.h      |  24 +++++
>  bfd/elf-eh-frame.c |   4 +-
>  bfd/elf-sframe.c   |   4 +-
>  bfd/elf.c          | 246 ++++++++++++++++++++++++++++++++++--------
>  bfd/elf32-i386.c   |   8 +-
>  bfd/elf64-x86-64.c |  12 +--
>  bfd/elfcode.h      |   7 +-
>  bfd/elflink.c      | 213 ++++++++++++++++++++++++------------
>  bfd/elfxx-target.h |   6 +-
>  bfd/elfxx-x86.c    |   8 +-
>  bfd/elfxx-x86.h    |   1 +
>  bfd/libbfd-in.h    |  33 +++++-
>  bfd/libbfd.c       | 262 ++++++++++++++++++++++++++++++++++++++++++++-
>  bfd/libbfd.h       |  33 +++++-
>  bfd/linker.c       |  35 ------
>  bfd/lynx-core.c    |   2 +-
>  bfd/opncls.c       |  21 ++++
>  bfd/section.c      |   9 +-
>  bfd/sysdep.h       |   4 +
>  24 files changed, 795 insertions(+), 195 deletions(-)
>
> --
> 2.44.0
>

PING.

-- 
H.J.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: PING: [PATCH v12 0/6] elf: Use mmap to map in section contents
  2024-03-28 13:29 ` PING: [PATCH v12 0/6] elf: Use mmap to map in section contents H.J. Lu
@ 2024-04-03 16:03   ` Nick Clifton
  0 siblings, 0 replies; 22+ messages in thread
From: Nick Clifton @ 2024-04-03 16:03 UTC (permalink / raw)
  To: H.J. Lu, Binutils; +Cc: Noah Goldstein, Sam James, Alan Modra

Hi H.J.

>> Changes in v12:
>>
>> 1. Don't call _bfd_elf_link_keep_memory for x86 since it always returns
>> false.
>> 2. Make _bfd_elf_link_keep_memory static.

> PING.

Sorry for the delay.

Patch series approved - please apply.

Cheers
   Nick



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v12 0/6] elf: Use mmap to map in section contents
  2024-03-17 12:19 [PATCH v12 0/6] elf: Use mmap to map in section contents H.J. Lu
                   ` (6 preceding siblings ...)
  2024-03-28 13:29 ` PING: [PATCH v12 0/6] elf: Use mmap to map in section contents H.J. Lu
@ 2024-04-04 13:12 ` Luis Machado
  2024-04-04 13:53   ` H.J. Lu
  2024-04-04 20:27   ` Joseph Myers
  7 siblings, 2 replies; 22+ messages in thread
From: Luis Machado @ 2024-04-04 13:12 UTC (permalink / raw)
  To: H.J. Lu, binutils; +Cc: goldstein.w.n, sam, amodra

Hi,

On 3/17/24 12:19, H.J. Lu wrote:
> Changes in v12:
> 
> 1. Don't call _bfd_elf_link_keep_memory for x86 since it always returns
> false.
> 2. Make _bfd_elf_link_keep_memory static.
> 
> Changes in v11:
> 
> 1. Update _bfd_mmap_read_temporary to allocate buffer as needed.
> 2. Don't allocate buffer when _bfd_mmap_read_temporary is called.
> 
> Changes in v10:
> 
> 1. Malloc a 4 * page size buffer for external relocations for the final
> link to avoid mmap/munmap on small relocation sections.
> 2. Malloc a buffer for input section contents which are smaller than
> 4 * page size or can't be mapped for the final link.
> 
> Changes in v9:
> 
> 1. Use MAP_FAILED for mmap failure.
> 
> Changes in v8:
> 
> 1. Rebase against master branch.
> 2. Add _bfd_elf_link_mmap_section_contents and
> _bfd_elf_link_munmap_section_contents.
> 
> Changes in v7:
> 
> 1. Don't add the --keep-memory linker option.
> 
> Changes in v6:
> 
> 1. Add the --keep-memory linker option and always cache symbol and
> relocation tables for --keep-memory.
> 2. Always keep symbol table and relocation info for eh_frame to speedup
> the Rust binary build by ~300x:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=31466
> 
> Changes in v5:
> 
> 1. Drop 2 patches which have been merged onto master branch.
> 2. Rename _bfd_elf_mmap_section to _bfd_elf_mmap_section_contents.
> 3. Rename _bfd_mmap_readonly_tracked, _bfd_mmap_readonly_untracked,
> _bfd_munmap_readonly_untracked, _bfd_mmap_read_untracked to
> _bfd_mmap_readonly_persistent, _bfd_mmap_readonly_temporary,
> _bfd_munmap_readonly_temporary and _bfd_mmap_read_temporary.
> 4. Drop the setup_group change.
> 5. Fix a typo.
> 6. Update comments.
> 
> Changes in v4:
> 
> 1. Change don't cache symbol nor relocation tables with mmap to opt-in.
> 
> Changes in v3:
> 
> 1. Fix non-mmap build.
> 2. Change the argument name of bfd_mmap_local from flags to prot since
> its values are PROT_XXX.
> 
> Changes in v2:
> 
> 1. Don't hard-code BFD_JUMP_TABLE_COPY in bfd so that elf-bfd.h can be
> included in libbfd.c.
> 2. Change the --with-mmap default to true.
> 3. Check USE_MMAP instead of HAVE_MMAP.
> 4. Remove the asize parameter to _bfd_mmap_readonly_tracked.
> 5. Add contents_addr and contents_size to bfd_elf_section_data.
> 6. Rename _bfd_link_keep_memory to _bfd_elf_link_keep_memory.
> 
> ---
> We can use mmap to map in ELF section contents, instead of copying them
> into memory by hand.  We don't need to cache symbol nor relocation tables
> if they are mapped in.  Data to link the 3.5GB clang executable in LLVM
> 17 debug build on Linux/x86-64 with 32GB RAM is:
> 
> 		stdio		mmap		improvement
> user		86.73		87.02		-0.3%
> system		9.55		9.21		3.6%
> total		100.40		97.66		0.7%
> maximum set(GB)	17.34		13.14		24%
> page faults	4047667		3042877		25%
> 
> and data to link the 275M cc1plus executable in GCC 14 stage 1 build is:
> 
> user		5.41		5.44		-0.5%
> system		0.80		0.76		5%
> total		6.25		6.26		-0.2%
> maximum set(MB)	1323		968		27%
> page faults	323451		236371		27%
> 
> Data shows that these won't improve the single copy linker performance.
> But they improve the overall system performance when linker is used by
> reducing linker memory usage and page faults.  They allow more parallel
> linker jobs on LLVM debug build.
> 
> Here is a quote from Noah Goldstein: "on a large project they are an
> extremely large speedup".
> 
> H.J. Lu (6):
>   elf: Use mmap to map in read-only sections
>   elf: Add _bfd_elf_m[un]map_section_contents
>   elf: Use mmap to map in symbol and relocation tables
>   elf: Don't cache symbol nor relocation tables with mmap
>   elf: Always keep symbol table and relocation info for eh_frame
>   elf: Add _bfd_elf_link_m[un]map_section_contents
> 
>  bfd/bfd-in2.h      |  24 ++++-
>  bfd/bfd.c          |  17 +++
>  bfd/bfdwin.c       |   8 +-
>  bfd/cache.c        |   7 +-
>  bfd/compress.c     |   2 +-
>  bfd/elf-bfd.h      |  24 +++++
>  bfd/elf-eh-frame.c |   4 +-
>  bfd/elf-sframe.c   |   4 +-
>  bfd/elf.c          | 246 ++++++++++++++++++++++++++++++++++--------
>  bfd/elf32-i386.c   |   8 +-
>  bfd/elf64-x86-64.c |  12 +--
>  bfd/elfcode.h      |   7 +-
>  bfd/elflink.c      | 213 ++++++++++++++++++++++++------------
>  bfd/elfxx-target.h |   6 +-
>  bfd/elfxx-x86.c    |   8 +-
>  bfd/elfxx-x86.h    |   1 +
>  bfd/libbfd-in.h    |  33 +++++-
>  bfd/libbfd.c       | 262 ++++++++++++++++++++++++++++++++++++++++++++-
>  bfd/libbfd.h       |  33 +++++-
>  bfd/linker.c       |  35 ------
>  bfd/lynx-core.c    |   2 +-
>  bfd/opncls.c       |  21 ++++
>  bfd/section.c      |   9 +-
>  bfd/sysdep.h       |   4 +
>  24 files changed, 795 insertions(+), 195 deletions(-)
> 

Probably known, judging from the e-mail reworking _bfd_mmap_read_temporary, but I thought I'd mention anyway.

Looks like this might be causing gdb crashes of the following nature:

Starting program: /home/builder/worker/gdb-fedora-arm64/gdb-build/gdb/testsuite/outputs/gdb.base/break-idempotent/break-idempotent-pie 
BFD: BFD (GNU Binutils) 2.42.50.20240404 internal error, aborting at ../../binutils-gdb/bfd/libbfd.c:1211 in _bfd_mmap_read_temporary
Please report this bug.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v12 0/6] elf: Use mmap to map in section contents
  2024-04-04 13:12 ` Luis Machado
@ 2024-04-04 13:53   ` H.J. Lu
  2024-04-04 20:27   ` Joseph Myers
  1 sibling, 0 replies; 22+ messages in thread
From: H.J. Lu @ 2024-04-04 13:53 UTC (permalink / raw)
  To: Luis Machado; +Cc: binutils, goldstein.w.n, sam, amodra

On Thu, Apr 4, 2024 at 6:12 AM Luis Machado <luis.machado@arm.com> wrote:
>
> Hi,
>
> On 3/17/24 12:19, H.J. Lu wrote:
> > Changes in v12:
> >
> > 1. Don't call _bfd_elf_link_keep_memory for x86 since it always returns
> > false.
> > 2. Make _bfd_elf_link_keep_memory static.
> >
> > Changes in v11:
> >
> > 1. Update _bfd_mmap_read_temporary to allocate buffer as needed.
> > 2. Don't allocate buffer when _bfd_mmap_read_temporary is called.
> >
> > Changes in v10:
> >
> > 1. Malloc a 4 * page size buffer for external relocations for the final
> > link to avoid mmap/munmap on small relocation sections.
> > 2. Malloc a buffer for input section contents which are smaller than
> > 4 * page size or can't be mapped for the final link.
> >
> > Changes in v9:
> >
> > 1. Use MAP_FAILED for mmap failure.
> >
> > Changes in v8:
> >
> > 1. Rebase against master branch.
> > 2. Add _bfd_elf_link_mmap_section_contents and
> > _bfd_elf_link_munmap_section_contents.
> >
> > Changes in v7:
> >
> > 1. Don't add the --keep-memory linker option.
> >
> > Changes in v6:
> >
> > 1. Add the --keep-memory linker option and always cache symbol and
> > relocation tables for --keep-memory.
> > 2. Always keep symbol table and relocation info for eh_frame to speedup
> > the Rust binary build by ~300x:
> >
> > https://sourceware.org/bugzilla/show_bug.cgi?id=31466
> >
> > Changes in v5:
> >
> > 1. Drop 2 patches which have been merged onto master branch.
> > 2. Rename _bfd_elf_mmap_section to _bfd_elf_mmap_section_contents.
> > 3. Rename _bfd_mmap_readonly_tracked, _bfd_mmap_readonly_untracked,
> > _bfd_munmap_readonly_untracked, _bfd_mmap_read_untracked to
> > _bfd_mmap_readonly_persistent, _bfd_mmap_readonly_temporary,
> > _bfd_munmap_readonly_temporary and _bfd_mmap_read_temporary.
> > 4. Drop the setup_group change.
> > 5. Fix a typo.
> > 6. Update comments.
> >
> > Changes in v4:
> >
> > 1. Change don't cache symbol nor relocation tables with mmap to opt-in.
> >
> > Changes in v3:
> >
> > 1. Fix non-mmap build.
> > 2. Change the argument name of bfd_mmap_local from flags to prot since
> > its values are PROT_XXX.
> >
> > Changes in v2:
> >
> > 1. Don't hard-code BFD_JUMP_TABLE_COPY in bfd so that elf-bfd.h can be
> > included in libbfd.c.
> > 2. Change the --with-mmap default to true.
> > 3. Check USE_MMAP instead of HAVE_MMAP.
> > 4. Remove the asize parameter to _bfd_mmap_readonly_tracked.
> > 5. Add contents_addr and contents_size to bfd_elf_section_data.
> > 6. Rename _bfd_link_keep_memory to _bfd_elf_link_keep_memory.
> >
> > ---
> > We can use mmap to map in ELF section contents, instead of copying them
> > into memory by hand.  We don't need to cache symbol nor relocation tables
> > if they are mapped in.  Data to link the 3.5GB clang executable in LLVM
> > 17 debug build on Linux/x86-64 with 32GB RAM is:
> >
> >               stdio           mmap            improvement
> > user          86.73           87.02           -0.3%
> > system                9.55            9.21            3.6%
> > total         100.40          97.66           0.7%
> > maximum set(GB)       17.34           13.14           24%
> > page faults   4047667         3042877         25%
> >
> > and data to link the 275M cc1plus executable in GCC 14 stage 1 build is:
> >
> > user          5.41            5.44            -0.5%
> > system                0.80            0.76            5%
> > total         6.25            6.26            -0.2%
> > maximum set(MB)       1323            968             27%
> > page faults   323451          236371          27%
> >
> > Data shows that these won't improve the single copy linker performance.
> > But they improve the overall system performance when linker is used by
> > reducing linker memory usage and page faults.  They allow more parallel
> > linker jobs on LLVM debug build.
> >
> > Here is a quote from Noah Goldstein: "on a large project they are an
> > extremely large speedup".
> >
> > H.J. Lu (6):
> >   elf: Use mmap to map in read-only sections
> >   elf: Add _bfd_elf_m[un]map_section_contents
> >   elf: Use mmap to map in symbol and relocation tables
> >   elf: Don't cache symbol nor relocation tables with mmap
> >   elf: Always keep symbol table and relocation info for eh_frame
> >   elf: Add _bfd_elf_link_m[un]map_section_contents
> >
> >  bfd/bfd-in2.h      |  24 ++++-
> >  bfd/bfd.c          |  17 +++
> >  bfd/bfdwin.c       |   8 +-
> >  bfd/cache.c        |   7 +-
> >  bfd/compress.c     |   2 +-
> >  bfd/elf-bfd.h      |  24 +++++
> >  bfd/elf-eh-frame.c |   4 +-
> >  bfd/elf-sframe.c   |   4 +-
> >  bfd/elf.c          | 246 ++++++++++++++++++++++++++++++++++--------
> >  bfd/elf32-i386.c   |   8 +-
> >  bfd/elf64-x86-64.c |  12 +--
> >  bfd/elfcode.h      |   7 +-
> >  bfd/elflink.c      | 213 ++++++++++++++++++++++++------------
> >  bfd/elfxx-target.h |   6 +-
> >  bfd/elfxx-x86.c    |   8 +-
> >  bfd/elfxx-x86.h    |   1 +
> >  bfd/libbfd-in.h    |  33 +++++-
> >  bfd/libbfd.c       | 262 ++++++++++++++++++++++++++++++++++++++++++++-
> >  bfd/libbfd.h       |  33 +++++-
> >  bfd/linker.c       |  35 ------
> >  bfd/lynx-core.c    |   2 +-
> >  bfd/opncls.c       |  21 ++++
> >  bfd/section.c      |   9 +-
> >  bfd/sysdep.h       |   4 +
> >  24 files changed, 795 insertions(+), 195 deletions(-)
> >
>
> Probably known, judging from the e-mail reworking _bfd_mmap_read_temporary, but I thought I'd mention anyway.
>
> Looks like this might be causing gdb crashes of the following nature:
>
> Starting program: /home/builder/worker/gdb-fedora-arm64/gdb-build/gdb/testsuite/outputs/gdb.base/break-idempotent/break-idempotent-pie
> BFD: BFD (GNU Binutils) 2.42.50.20240404 internal error, aborting at ../../binutils-gdb/bfd/libbfd.c:1211 in _bfd_mmap_read_temporary
> Please report this bug.

The patch is at

https://sourceware.org/pipermail/binutils/2024-April/133374.html

-- 
H.J.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v12 0/6] elf: Use mmap to map in section contents
  2024-04-04 13:12 ` Luis Machado
  2024-04-04 13:53   ` H.J. Lu
@ 2024-04-04 20:27   ` Joseph Myers
  2024-04-04 22:22     ` Alan Modra
  1 sibling, 1 reply; 22+ messages in thread
From: Joseph Myers @ 2024-04-04 20:27 UTC (permalink / raw)
  To: Luis Machado; +Cc: H.J. Lu, binutils, goldstein.w.n, sam, amodra

On Thu, 4 Apr 2024, Luis Machado wrote:

> Probably known, judging from the e-mail reworking _bfd_mmap_read_temporary, but I thought I'd mention anyway.
> 
> Looks like this might be causing gdb crashes of the following nature:

It's also causing segfaults linking libstdc++ for powerpc64-linux-gnu in 
build-many-glibcs.py, when using binutils and GCC mainline.

-- 
Joseph S. Myers
josmyers@redhat.com


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v12 0/6] elf: Use mmap to map in section contents
  2024-04-04 20:27   ` Joseph Myers
@ 2024-04-04 22:22     ` Alan Modra
  2024-04-04 22:43       ` Joseph Myers
  0 siblings, 1 reply; 22+ messages in thread
From: Alan Modra @ 2024-04-04 22:22 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Luis Machado, H.J. Lu, binutils, goldstein.w.n, sam

On Thu, Apr 04, 2024 at 08:27:50PM +0000, Joseph Myers wrote:
> On Thu, 4 Apr 2024, Luis Machado wrote:
> 
> > Probably known, judging from the e-mail reworking _bfd_mmap_read_temporary, but I thought I'd mention anyway.
> > 
> > Looks like this might be causing gdb crashes of the following nature:
> 
> It's also causing segfaults linking libstdc++ for powerpc64-linux-gnu in 
> build-many-glibcs.py, when using binutils and GCC mainline.

Likely fixed with b43b352837ea.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v12 0/6] elf: Use mmap to map in section contents
  2024-04-04 22:22     ` Alan Modra
@ 2024-04-04 22:43       ` Joseph Myers
  2024-04-04 22:46         ` H.J. Lu
  0 siblings, 1 reply; 22+ messages in thread
From: Joseph Myers @ 2024-04-04 22:43 UTC (permalink / raw)
  To: Alan Modra; +Cc: Luis Machado, H.J. Lu, binutils, goldstein.w.n, sam

On Fri, 5 Apr 2024, Alan Modra wrote:

> On Thu, Apr 04, 2024 at 08:27:50PM +0000, Joseph Myers wrote:
> > On Thu, 4 Apr 2024, Luis Machado wrote:
> > 
> > > Probably known, judging from the e-mail reworking _bfd_mmap_read_temporary, but I thought I'd mention anyway.
> > > 
> > > Looks like this might be causing gdb crashes of the following nature:
> > 
> > It's also causing segfaults linking libstdc++ for powerpc64-linux-gnu in 
> > build-many-glibcs.py, when using binutils and GCC mainline.
> 
> Likely fixed with b43b352837ea.

I confirmed it's still present as of 
16810e455feb26ef826a3ed876d6d7e6d24818b0, which is more recent.

-- 
Joseph S. Myers
josmyers@redhat.com


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v12 0/6] elf: Use mmap to map in section contents
  2024-04-04 22:43       ` Joseph Myers
@ 2024-04-04 22:46         ` H.J. Lu
  2024-04-04 23:20           ` H.J. Lu
  0 siblings, 1 reply; 22+ messages in thread
From: H.J. Lu @ 2024-04-04 22:46 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Alan Modra, Luis Machado, binutils, goldstein.w.n, sam

On Thu, Apr 4, 2024 at 3:44 PM Joseph Myers <josmyers@redhat.com> wrote:
>
> On Fri, 5 Apr 2024, Alan Modra wrote:
>
> > On Thu, Apr 04, 2024 at 08:27:50PM +0000, Joseph Myers wrote:
> > > On Thu, 4 Apr 2024, Luis Machado wrote:
> > >
> > > > Probably known, judging from the e-mail reworking _bfd_mmap_read_temporary, but I thought I'd mention anyway.
> > > >
> > > > Looks like this might be causing gdb crashes of the following nature:
> > >
> > > It's also causing segfaults linking libstdc++ for powerpc64-linux-gnu in
> > > build-many-glibcs.py, when using binutils and GCC mainline.
> >
> > Likely fixed with b43b352837ea.
>
> I confirmed it's still present as of
> 16810e455feb26ef826a3ed876d6d7e6d24818b0, which is more recent.
>

Try:

360d244b24e bfd: Handle bmmap failure in _bfd_mmap_read_temporary

-- 
H.J.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v12 0/6] elf: Use mmap to map in section contents
  2024-04-04 22:46         ` H.J. Lu
@ 2024-04-04 23:20           ` H.J. Lu
  0 siblings, 0 replies; 22+ messages in thread
From: H.J. Lu @ 2024-04-04 23:20 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Alan Modra, Luis Machado, binutils, goldstein.w.n, sam

On Thu, Apr 4, 2024 at 3:46 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Thu, Apr 4, 2024 at 3:44 PM Joseph Myers <josmyers@redhat.com> wrote:
> >
> > On Fri, 5 Apr 2024, Alan Modra wrote:
> >
> > > On Thu, Apr 04, 2024 at 08:27:50PM +0000, Joseph Myers wrote:
> > > > On Thu, 4 Apr 2024, Luis Machado wrote:
> > > >
> > > > > Probably known, judging from the e-mail reworking _bfd_mmap_read_temporary, but I thought I'd mention anyway.
> > > > >
> > > > > Looks like this might be causing gdb crashes of the following nature:
> > > >
> > > > It's also causing segfaults linking libstdc++ for powerpc64-linux-gnu in
> > > > build-many-glibcs.py, when using binutils and GCC mainline.
> > >
> > > Likely fixed with b43b352837ea.
> >
> > I confirmed it's still present as of
> > 16810e455feb26ef826a3ed876d6d7e6d24818b0, which is more recent.
> >
>
> Try:
>
> 360d244b24e bfd: Handle bmmap failure in _bfd_mmap_read_temporary
>
> --
> H.J.

I opened:

https://sourceware.org/bugzilla/show_bug.cgi?id=31608


-- 
H.J.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v12 1/6] elf: Use mmap to map in read-only sections
  2024-03-17 12:19 ` [PATCH v12 1/6] elf: Use mmap to map in read-only sections H.J. Lu
@ 2024-04-08  3:57   ` Simon Marchi
  2024-04-08 14:26     ` [PATCH] bfd: Define pagesize variables only for mmap H.J. Lu
  0 siblings, 1 reply; 22+ messages in thread
From: Simon Marchi @ 2024-04-08  3:57 UTC (permalink / raw)
  To: H.J. Lu, binutils; +Cc: goldstein.w.n, sam, amodra



On 2024-03-17 08:19, H.J. Lu wrote:
> There are many linker input files in LLVM debug build with huge string
> sections.  All these string sections can be treated as read-only.  But
> linker copies all of them into memory which consumes huge amount of
> memory and slows down linker significantly.
> 
> Add _bfd_mmap_readonly_persistent and _bfd_mmap_readonly_temporary to
> mmap in reado-only sections with size >= 4 * page size.
> 
> NB: All string sections in valid ELF inputs must be null terminated.
> There is no need to terminate it again and string sections are mmapped
> as read-only.
> 
> 	* bfd.c (bfd_mmapped_entry): New.
> 	(bfd_mmapped): Likewise.
> 	(bfd): Add mmapped.
> 	* bfdwin.c (bfd_get_file_window): Use _bfd_pagesize.
> 	* cache.c (cache_bmmap): Remove pagesize_m1 and use pagesize_m1
> 	instead.
> 	* elf.c (bfd_elf_get_str_section): Call
> 	_bfd_mmap_readonly_persistent instead of _bfd_alloc_and_read.
> 	Don't terminate the string section again.
> 	(get_hash_table_data): Call _bfd_mmap_readonly_temporary and
> 	_bfd_munmap_readonly_temporary instead of _bfd_malloc_and_read
> 	and free.
> 	(_bfd_elf_get_dynamic_symbols): Call _bfd_mmap_readonly_persistent
> 	instead of _bfd_alloc_and_read.  Don't terminate the string
> 	section again.  Call _bfd_mmap_readonly_temporary and
> 	_bfd_munmap_readonly_temporary instead of _bfd_malloc_and_read
> 	and free.
> 	(_bfd_elf_slurp_version_tables): Call _bfd_mmap_readonly_temporary
> 	and _bfd_munmap_readonly_temporary instead of _bfd_malloc_and_read
> 	and free.
> 	* elflink.c (bfd_elf_link_record_dynamic_symbol): Use bfd_malloc
> 	to get the unversioned symbol.
> 	* libbfd-in.h (_bfd_pagesize): New.
> 	(_bfd_pagesize_m1): Likewise.
> 	(_bfd_minimum_mmap_size): Likewise.
> 	(_bfd_mmap_readonly_persistent): Likewise.
> 	(_bfd_mmap_readonly_temporary): Likewise.
> 	(_bfd_munmap_readonly_temporary): Likewise.
> 	* libbfd.c
> 	(bfd_allocate_mmapped_page): New.
> 	(_bfd_mmap_readonly_temporary): Likewise.
> 	(_bfd_munmap_readonly_temporary): Likewise.
> 	(_bfd_mmap_readonly_persistent): Likewise.
> 	(_bfd_pagesize): Likewise.
> 	(_bfd_pagesize_m1): Likewise.
> 	(_bfd_minimum_mmap_size): Likewise.
> 	(bfd_init_pagesize): Likewise.
> 	* lynx-core.c (lynx_core_file_p): Use _bfd_pagesize.
> 	* opncls.c (_bfd_delete_bfd): Munmap tracked mmapped memories.
> 	* sysdep.h (MAP_ANONYMOUS): New. Define if undefined.
> 	* bfd-in2.h: Regenerated.
> 	* libbfd.h: Likewise.

Hi,

Since this commit, when building for --host=x86_64-w64-mingw32, I get:


make[4]: Entering directory '/home/simark/build/binutils-gdb-x86_64-w64-mingw32/bfd'
  CC       libbfd.lo
/home/simark/src/binutils-gdb/bfd/libbfd.c: In function ‘bfd_init_pagesize’:
/home/simark/src/binutils-gdb/bfd/libbfd.c:1583:19: error: implicit declaration of function ‘getpagesize’ [-Werror=implicit-function-declaration]
 1583 |   _bfd_pagesize = getpagesize ();
      |                   ^~~~~~~~~~~

Simon

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH] bfd: Define pagesize variables only for mmap
  2024-04-08  3:57   ` Simon Marchi
@ 2024-04-08 14:26     ` H.J. Lu
  2024-04-08 22:55       ` Alan Modra
  0 siblings, 1 reply; 22+ messages in thread
From: H.J. Lu @ 2024-04-08 14:26 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Binutils, Noah Goldstein, Sam James, Alan Modra

[-- Attachment #1: Type: text/plain, Size: 3487 bytes --]

On Sun, Apr 7, 2024 at 8:58 PM Simon Marchi <simon.marchi@polymtl.ca> wrote:
>
>
>
> On 2024-03-17 08:19, H.J. Lu wrote:
> > There are many linker input files in LLVM debug build with huge string
> > sections.  All these string sections can be treated as read-only.  But
> > linker copies all of them into memory which consumes huge amount of
> > memory and slows down linker significantly.
> >
> > Add _bfd_mmap_readonly_persistent and _bfd_mmap_readonly_temporary to
> > mmap in reado-only sections with size >= 4 * page size.
> >
> > NB: All string sections in valid ELF inputs must be null terminated.
> > There is no need to terminate it again and string sections are mmapped
> > as read-only.
> >
> >       * bfd.c (bfd_mmapped_entry): New.
> >       (bfd_mmapped): Likewise.
> >       (bfd): Add mmapped.
> >       * bfdwin.c (bfd_get_file_window): Use _bfd_pagesize.
> >       * cache.c (cache_bmmap): Remove pagesize_m1 and use pagesize_m1
> >       instead.
> >       * elf.c (bfd_elf_get_str_section): Call
> >       _bfd_mmap_readonly_persistent instead of _bfd_alloc_and_read.
> >       Don't terminate the string section again.
> >       (get_hash_table_data): Call _bfd_mmap_readonly_temporary and
> >       _bfd_munmap_readonly_temporary instead of _bfd_malloc_and_read
> >       and free.
> >       (_bfd_elf_get_dynamic_symbols): Call _bfd_mmap_readonly_persistent
> >       instead of _bfd_alloc_and_read.  Don't terminate the string
> >       section again.  Call _bfd_mmap_readonly_temporary and
> >       _bfd_munmap_readonly_temporary instead of _bfd_malloc_and_read
> >       and free.
> >       (_bfd_elf_slurp_version_tables): Call _bfd_mmap_readonly_temporary
> >       and _bfd_munmap_readonly_temporary instead of _bfd_malloc_and_read
> >       and free.
> >       * elflink.c (bfd_elf_link_record_dynamic_symbol): Use bfd_malloc
> >       to get the unversioned symbol.
> >       * libbfd-in.h (_bfd_pagesize): New.
> >       (_bfd_pagesize_m1): Likewise.
> >       (_bfd_minimum_mmap_size): Likewise.
> >       (_bfd_mmap_readonly_persistent): Likewise.
> >       (_bfd_mmap_readonly_temporary): Likewise.
> >       (_bfd_munmap_readonly_temporary): Likewise.
> >       * libbfd.c
> >       (bfd_allocate_mmapped_page): New.
> >       (_bfd_mmap_readonly_temporary): Likewise.
> >       (_bfd_munmap_readonly_temporary): Likewise.
> >       (_bfd_mmap_readonly_persistent): Likewise.
> >       (_bfd_pagesize): Likewise.
> >       (_bfd_pagesize_m1): Likewise.
> >       (_bfd_minimum_mmap_size): Likewise.
> >       (bfd_init_pagesize): Likewise.
> >       * lynx-core.c (lynx_core_file_p): Use _bfd_pagesize.
> >       * opncls.c (_bfd_delete_bfd): Munmap tracked mmapped memories.
> >       * sysdep.h (MAP_ANONYMOUS): New. Define if undefined.
> >       * bfd-in2.h: Regenerated.
> >       * libbfd.h: Likewise.
>
> Hi,
>
> Since this commit, when building for --host=x86_64-w64-mingw32, I get:
>
>
> make[4]: Entering directory '/home/simark/build/binutils-gdb-x86_64-w64-mingw32/bfd'
>   CC       libbfd.lo
> /home/simark/src/binutils-gdb/bfd/libbfd.c: In function ‘bfd_init_pagesize’:
> /home/simark/src/binutils-gdb/bfd/libbfd.c:1583:19: error: implicit declaration of function ‘getpagesize’ [-Werror=implicit-function-declaration]
>  1583 |   _bfd_pagesize = getpagesize ();
>       |                   ^~~~~~~~~~~
>
> Simon

Please try this patch.

-- 
H.J.

[-- Attachment #2: 0001-bfd-Define-pagesize-variables-only-for-mmap.patch --]
[-- Type: application/x-patch, Size: 2337 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] bfd: Define pagesize variables only for mmap
  2024-04-08 14:26     ` [PATCH] bfd: Define pagesize variables only for mmap H.J. Lu
@ 2024-04-08 22:55       ` Alan Modra
  2024-04-09  2:49         ` H.J. Lu
  0 siblings, 1 reply; 22+ messages in thread
From: Alan Modra @ 2024-04-08 22:55 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Simon Marchi, Binutils, Noah Goldstein, Sam James

On Mon, Apr 08, 2024 at 07:26:37AM -0700, H.J. Lu wrote:
> On Sun, Apr 7, 2024 at 8:58 PM Simon Marchi <simon.marchi@polymtl.ca> wrote:
> > Since this commit, when building for --host=x86_64-w64-mingw32, I get:
> >
> >
> > make[4]: Entering directory '/home/simark/build/binutils-gdb-x86_64-w64-mingw32/bfd'
> >   CC       libbfd.lo
> > /home/simark/src/binutils-gdb/bfd/libbfd.c: In function ‘bfd_init_pagesize’:
> > /home/simark/src/binutils-gdb/bfd/libbfd.c:1583:19: error: implicit declaration of function ‘getpagesize’ [-Werror=implicit-function-declaration]
> >  1583 |   _bfd_pagesize = getpagesize ();
> >       |                   ^~~~~~~~~~~
> >
> > Simon
> 
> Please try this patch.

You need to revert the change to lynx-core.c as well.

Somewhat related, why does binutils/elfedit.c depend on HAVE_MMAP?
I run into failures with asan builds (ie. -fsanitize=address,undefined
in CFLAGS and CXXFLAGS passed to configure).
elfedit: unrecognized option '--enable-x86-feature'
Apparently HAVE_MMAP configury fusses too much, see this comment:
gnulib/import/m4/mmap-anon.m4:  # Check for mmap(). Don't use AC_FUNC_MMAP, because it checks too much: it

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] bfd: Define pagesize variables only for mmap
  2024-04-08 22:55       ` Alan Modra
@ 2024-04-09  2:49         ` H.J. Lu
  2024-04-09  5:47           ` Alan Modra
  0 siblings, 1 reply; 22+ messages in thread
From: H.J. Lu @ 2024-04-09  2:49 UTC (permalink / raw)
  To: Alan Modra; +Cc: Simon Marchi, Binutils, Noah Goldstein, Sam James

On Mon, Apr 8, 2024 at 3:55 PM Alan Modra <amodra@gmail.com> wrote:
>
> On Mon, Apr 08, 2024 at 07:26:37AM -0700, H.J. Lu wrote:
> > On Sun, Apr 7, 2024 at 8:58 PM Simon Marchi <simon.marchi@polymtl.ca> wrote:
> > > Since this commit, when building for --host=x86_64-w64-mingw32, I get:
> > >
> > >
> > > make[4]: Entering directory '/home/simark/build/binutils-gdb-x86_64-w64-mingw32/bfd'
> > >   CC       libbfd.lo
> > > /home/simark/src/binutils-gdb/bfd/libbfd.c: In function ‘bfd_init_pagesize’:
> > > /home/simark/src/binutils-gdb/bfd/libbfd.c:1583:19: error: implicit declaration of function ‘getpagesize’ [-Werror=implicit-function-declaration]
> > >  1583 |   _bfd_pagesize = getpagesize ();
> > >       |                   ^~~~~~~~~~~
> > >
> > > Simon
> >
> > Please try this patch.
>
> You need to revert the change to lynx-core.c as well.

The v2 patch is at

https://sourceware.org/pipermail/binutils/2024-April/133466.html

> Somewhat related, why does binutils/elfedit.c depend on HAVE_MMAP?

It uses mmap to update the ELF program header directly.

> I run into failures with asan builds (ie. -fsanitize=address,undefined
> in CFLAGS and CXXFLAGS passed to configure).
> elfedit: unrecognized option '--enable-x86-feature'

I built binutils with GCC 13 using

CC="gcc -fsanitize=address,undefined" CXX="g++
-fsanitize=address,undefined"
/export/gnu/import/git/gitlab/x86-binutils/configure \
--disable-werror --disable-gprofng \
\
--enable-plugins --disable-gdb --disable-gdbserver
--disable-libbacktrace --disable-libdecnumber --disable-readline
--disable-sim --enable-mark-plt --with-sysroot=/ --with-system-zlib \
--prefix=/usr/local \
--with-local-prefix=/usr/local

elfedit works:

[hjl@gnu-cfl-3 binutils]$ ./elfedit --enable-x86-feature ibt xx
[hjl@gnu-cfl-3 binutils]$

> Apparently HAVE_MMAP configury fusses too much, see this comment:
> gnulib/import/m4/mmap-anon.m4:  # Check for mmap(). Don't use AC_FUNC_MMAP, because it checks too much: it

It checks MAP_ANONYMOUS, not mmap:

  # Check for mmap(). Don't use AC_FUNC_MMAP, because it checks too much: it
  # fails on HP-UX 11, because MAP_FIXED mappings do not work. But this is
  # irrelevant for anonymous mappings.


-- 
H.J.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] bfd: Define pagesize variables only for mmap
  2024-04-09  2:49         ` H.J. Lu
@ 2024-04-09  5:47           ` Alan Modra
  2024-04-09 14:25             ` H.J. Lu
  0 siblings, 1 reply; 22+ messages in thread
From: Alan Modra @ 2024-04-09  5:47 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Simon Marchi, Binutils, Noah Goldstein, Sam James

On Mon, Apr 08, 2024 at 07:49:26PM -0700, H.J. Lu wrote:
> On Mon, Apr 8, 2024 at 3:55 PM Alan Modra <amodra@gmail.com> wrote:
> >
> > On Mon, Apr 08, 2024 at 07:26:37AM -0700, H.J. Lu wrote:
> > > On Sun, Apr 7, 2024 at 8:58 PM Simon Marchi <simon.marchi@polymtl.ca> wrote:
> > > > Since this commit, when building for --host=x86_64-w64-mingw32, I get:
> > > >
> > > >
> > > > make[4]: Entering directory '/home/simark/build/binutils-gdb-x86_64-w64-mingw32/bfd'
> > > >   CC       libbfd.lo
> > > > /home/simark/src/binutils-gdb/bfd/libbfd.c: In function ‘bfd_init_pagesize’:
> > > > /home/simark/src/binutils-gdb/bfd/libbfd.c:1583:19: error: implicit declaration of function ‘getpagesize’ [-Werror=implicit-function-declaration]
> > > >  1583 |   _bfd_pagesize = getpagesize ();
> > > >       |                   ^~~~~~~~~~~
> > > >
> > > > Simon
> > >
> > > Please try this patch.
> >
> > You need to revert the change to lynx-core.c as well.
> 
> The v2 patch is at
> 
> https://sourceware.org/pipermail/binutils/2024-April/133466.html
> 
> > Somewhat related, why does binutils/elfedit.c depend on HAVE_MMAP?
> 
> It uses mmap to update the ELF program header directly.
> 
> > I run into failures with asan builds (ie. -fsanitize=address,undefined
> > in CFLAGS and CXXFLAGS passed to configure).
> > elfedit: unrecognized option '--enable-x86-feature'
> 
> I built binutils with GCC 13 using
> 
> CC="gcc -fsanitize=address,undefined" CXX="g++
> -fsanitize=address,undefined"
> /export/gnu/import/git/gitlab/x86-binutils/configure \
> --disable-werror --disable-gprofng \
> \
> --enable-plugins --disable-gdb --disable-gdbserver
> --disable-libbacktrace --disable-libdecnumber --disable-readline
> --disable-sim --enable-mark-plt --with-sysroot=/ --with-system-zlib \
> --prefix=/usr/local \
> --with-local-prefix=/usr/local
> 
> elfedit works:
> 
> [hjl@gnu-cfl-3 binutils]$ ./elfedit --enable-x86-feature ibt xx
> [hjl@gnu-cfl-3 binutils]$
> 
> > Apparently HAVE_MMAP configury fusses too much, see this comment:
> > gnulib/import/m4/mmap-anon.m4:  # Check for mmap(). Don't use AC_FUNC_MMAP, because it checks too much: it
> 
> It checks MAP_ANONYMOUS, not mmap:
> 
>   # Check for mmap(). Don't use AC_FUNC_MMAP, because it checks too much: it
>   # fails on HP-UX 11, because MAP_FIXED mappings do not work. But this is
>   # irrelevant for anonymous mappings.

Yes, it isn't that.  I fail to get HAVE_MMAP for the rather more
mundane reason that the config test fails with:
=================================================================
==231796==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 4096 byte(s) in 1 object(s) allocated from:
    #0 0x7cdd3d0defdf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x5750c7f6d72b in main /home/alan/build/gas-san/all/bfd/conftest.c:239

Direct leak of 4096 byte(s) in 1 object(s) allocated from:
    #0 0x7cdd3d0defdf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x5750c7f6d2e1 in main /home/alan/build/gas-san/all/bfd/conftest.c:190

SUMMARY: AddressSanitizer: 8192 byte(s) leaked in 2 allocation(s).

This can be avoided by
export ASAN_OPTIONS=detect_leaks=0

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] bfd: Define pagesize variables only for mmap
  2024-04-09  5:47           ` Alan Modra
@ 2024-04-09 14:25             ` H.J. Lu
  0 siblings, 0 replies; 22+ messages in thread
From: H.J. Lu @ 2024-04-09 14:25 UTC (permalink / raw)
  To: Alan Modra; +Cc: Simon Marchi, Binutils, Noah Goldstein, Sam James

On Mon, Apr 8, 2024 at 10:47 PM Alan Modra <amodra@gmail.com> wrote:
>
> On Mon, Apr 08, 2024 at 07:49:26PM -0700, H.J. Lu wrote:
> > On Mon, Apr 8, 2024 at 3:55 PM Alan Modra <amodra@gmail.com> wrote:
> > >
> > > On Mon, Apr 08, 2024 at 07:26:37AM -0700, H.J. Lu wrote:
> > > > On Sun, Apr 7, 2024 at 8:58 PM Simon Marchi <simon.marchi@polymtl.ca> wrote:
> > > > > Since this commit, when building for --host=x86_64-w64-mingw32, I get:
> > > > >
> > > > >
> > > > > make[4]: Entering directory '/home/simark/build/binutils-gdb-x86_64-w64-mingw32/bfd'
> > > > >   CC       libbfd.lo
> > > > > /home/simark/src/binutils-gdb/bfd/libbfd.c: In function ‘bfd_init_pagesize’:
> > > > > /home/simark/src/binutils-gdb/bfd/libbfd.c:1583:19: error: implicit declaration of function ‘getpagesize’ [-Werror=implicit-function-declaration]
> > > > >  1583 |   _bfd_pagesize = getpagesize ();
> > > > >       |                   ^~~~~~~~~~~
> > > > >
> > > > > Simon
> > > >
> > > > Please try this patch.
> > >
> > > You need to revert the change to lynx-core.c as well.
> >
> > The v2 patch is at
> >
> > https://sourceware.org/pipermail/binutils/2024-April/133466.html
> >
> > > Somewhat related, why does binutils/elfedit.c depend on HAVE_MMAP?
> >
> > It uses mmap to update the ELF program header directly.
> >
> > > I run into failures with asan builds (ie. -fsanitize=address,undefined
> > > in CFLAGS and CXXFLAGS passed to configure).
> > > elfedit: unrecognized option '--enable-x86-feature'
> >
> > I built binutils with GCC 13 using
> >
> > CC="gcc -fsanitize=address,undefined" CXX="g++
> > -fsanitize=address,undefined"
> > /export/gnu/import/git/gitlab/x86-binutils/configure \
> > --disable-werror --disable-gprofng \
> > \
> > --enable-plugins --disable-gdb --disable-gdbserver
> > --disable-libbacktrace --disable-libdecnumber --disable-readline
> > --disable-sim --enable-mark-plt --with-sysroot=/ --with-system-zlib \
> > --prefix=/usr/local \
> > --with-local-prefix=/usr/local
> >
> > elfedit works:
> >
> > [hjl@gnu-cfl-3 binutils]$ ./elfedit --enable-x86-feature ibt xx
> > [hjl@gnu-cfl-3 binutils]$
> >
> > > Apparently HAVE_MMAP configury fusses too much, see this comment:
> > > gnulib/import/m4/mmap-anon.m4:  # Check for mmap(). Don't use AC_FUNC_MMAP, because it checks too much: it
> >
> > It checks MAP_ANONYMOUS, not mmap:
> >
> >   # Check for mmap(). Don't use AC_FUNC_MMAP, because it checks too much: it
> >   # fails on HP-UX 11, because MAP_FIXED mappings do not work. But this is
> >   # irrelevant for anonymous mappings.
>
> Yes, it isn't that.  I fail to get HAVE_MMAP for the rather more
> mundane reason that the config test fails with:
> =================================================================
> ==231796==ERROR: LeakSanitizer: detected memory leaks
>
> Direct leak of 4096 byte(s) in 1 object(s) allocated from:
>     #0 0x7cdd3d0defdf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
>     #1 0x5750c7f6d72b in main /home/alan/build/gas-san/all/bfd/conftest.c:239
>
> Direct leak of 4096 byte(s) in 1 object(s) allocated from:
>     #0 0x7cdd3d0defdf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
>     #1 0x5750c7f6d2e1 in main /home/alan/build/gas-san/all/bfd/conftest.c:190
>
> SUMMARY: AddressSanitizer: 8192 byte(s) leaked in 2 allocation(s).
>
> This can be avoided by
> export ASAN_OPTIONS=detect_leaks=0
>

A patch set is posted at

https://sourceware.org/pipermail/binutils/2024-April/133484.html

--
H.J.

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2024-04-09 14:26 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-17 12:19 [PATCH v12 0/6] elf: Use mmap to map in section contents H.J. Lu
2024-03-17 12:19 ` [PATCH v12 1/6] elf: Use mmap to map in read-only sections H.J. Lu
2024-04-08  3:57   ` Simon Marchi
2024-04-08 14:26     ` [PATCH] bfd: Define pagesize variables only for mmap H.J. Lu
2024-04-08 22:55       ` Alan Modra
2024-04-09  2:49         ` H.J. Lu
2024-04-09  5:47           ` Alan Modra
2024-04-09 14:25             ` H.J. Lu
2024-03-17 12:19 ` [PATCH v12 2/6] elf: Add _bfd_elf_m[un]map_section_contents H.J. Lu
2024-03-17 12:19 ` [PATCH v12 3/6] elf: Use mmap to map in symbol and relocation tables H.J. Lu
2024-03-17 12:19 ` [PATCH v12 4/6] elf: Don't cache symbol nor relocation tables with mmap H.J. Lu
2024-03-17 12:19 ` [PATCH v12 5/6] elf: Always keep symbol table and relocation info for eh_frame H.J. Lu
2024-03-17 12:19 ` [PATCH v12 6/6] elf: Add _bfd_elf_link_m[un]map_section_contents H.J. Lu
2024-03-28 13:29 ` PING: [PATCH v12 0/6] elf: Use mmap to map in section contents H.J. Lu
2024-04-03 16:03   ` Nick Clifton
2024-04-04 13:12 ` Luis Machado
2024-04-04 13:53   ` H.J. Lu
2024-04-04 20:27   ` Joseph Myers
2024-04-04 22:22     ` Alan Modra
2024-04-04 22:43       ` Joseph Myers
2024-04-04 22:46         ` H.J. Lu
2024-04-04 23:20           ` H.J. Lu

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