PR25993, read of freed memory

ldmain.c:add_archive_element copies file name pointers from the bfd to
a lang_input_statement_type.
  input->filename = abfd->filename;
  input->local_sym_name = abfd->filename;
This results in stale pointers when twiddling the bfd filename in
places like the pe ld after_open.  So don't free the bfd filename,
and make copies using bfd_alloc memory that won't result in small
memory leaks that annoy memory checkers.

	PR 25993
bfd/
	* archive.c (_bfd_get_elt_at_filepos): Don't strdup filename,
	use bfd_set_filename.
	* elfcode.h (_bfd_elf_bfd_from_remote_memory): Likewise.
	* mach-o.c (bfd_mach_o_fat_member_init): Likewise.
	* opncls.c (bfd_fopen, bfd_openstreamr, bfd_openr_iovec, bfd_openw),
	(bfd_create): Likewise.
	(_bfd_delete_bfd): Don't free filename.
	(bfd_set_filename): Copy filename param to bfd_alloc'd memory,
	return pointer to the copy or NULL on alloc fail.
	* vms-lib.c (_bfd_vms_lib_get_module): Free newname and test
	result of bfd_set_filename.
	* bfd-in2.h: Regenerate.
gdb/
	* solib-darwin.c (darwin_bfd_open): Don't strdup pathname for
	bfd_set_filename.
	* solib-aix.c (solib_aix_bfd_open): Use std::string for name
	passed to bfd_set_filename.
	* symfile-mem.c (add_vsyscall_page): Likewise for string
	passed to symbol_file_add_from_memory.
	(symbol_file_add_from_memory): Make name param a const char* and
	don't strdup.
ld/
	* emultempl/pe.em (gld_${EMULATION_NAME}_after_open): Don't copy
	other_bfd_filename for bfd_set_filename, and test result of
	bfd_set_filename call.  Don't create a new is->filename, simply
	copy from bfd filename.  Free new_name after bfd_set_filename.
	* emultempl/pep.em (gld_${EMULATION_NAME}_after_open): Likewise.

4 files changed, 24 insertions, 12 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9799e1e..85a016b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,17 @@
 2020-05-20  Alan Modra  <amodra@gmail.com>
 
+	PR 25993
+	* solib-darwin.c (darwin_bfd_open): Don't strdup pathname for
+	bfd_set_filename.
+	* solib-aix.c (solib_aix_bfd_open): Use std::string for name
+	passed to bfd_set_filename.
+	* symfile-mem.c (add_vsyscall_page): Likewise for string
+	passed to symbol_file_add_from_memory.
+	(symbol_file_add_from_memory): Make name param a const char* and
+	don't strdup.
+
+2020-05-20  Alan Modra  <amodra@gmail.com>
+
 	* coff-pe-read.c (read_pe_exported_syms): Use bfd_get_filename
 	rather than accessing bfd->filename directly.
 	* dtrace-probe.c (dtrace_static_probe_ops::get_probes): Likewise,
diff --git a/gdb/solib-aix.c b/gdb/solib-aix.c
index 5da1214..344c1f5 100644
--- a/gdb/solib-aix.c
+++ b/gdb/solib-aix.c
@@ -637,10 +637,10 @@ solib_aix_bfd_open (const char *pathname)
      along with appended parenthesized member name in order to allow commands
      listing all shared libraries to display.  Otherwise, we would only be
      displaying the name of the archive member object.  */
-  bfd_set_filename (object_bfd.get (),
-		    xstrprintf ("%s%s",
-				bfd_get_filename (archive_bfd.get ()),
-				sep));
+  std::string fname = string_printf ("%s%s",
+				     bfd_get_filename (archive_bfd.get ()),
+				     sep);
+  bfd_set_filename (object_bfd.get (), fname.c_str ());
 
   return object_bfd;
 }
diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
index e740a41..ee0483d 100644
--- a/gdb/solib-darwin.c
+++ b/gdb/solib-darwin.c
@@ -662,7 +662,7 @@ darwin_bfd_open (const char *pathname)
   /* The current filename for fat-binary BFDs is a name generated
      by BFD, usually a string containing the name of the architecture.
      Reset its value to the actual filename.  */
-  bfd_set_filename (res.get (), xstrdup (pathname));
+  bfd_set_filename (res.get (), pathname);
 
   return res;
 }
diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c
index e2d2e43..78096fc 100644
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -78,11 +78,10 @@ target_read_memory_bfd (bfd_vma memaddr, bfd_byte *myaddr, bfd_size_type len)
    and read its in-core symbols out of inferior memory.  SIZE, if
    non-zero, is the known size of the object.  TEMPL is a bfd
    representing the target's format.  NAME is the name to use for this
-   symbol file in messages; it can be NULL or a malloc-allocated string
-   which will be attached to the BFD.  */
+   symbol file in messages; it can be NULL.  */
 static struct objfile *
 symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr,
-			     size_t size, char *name, int from_tty)
+			     size_t size, const char *name, int from_tty)
 {
   struct objfile *objf;
   struct bfd *nbfd;
@@ -102,7 +101,7 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr,
   gdb_bfd_ref_ptr nbfd_holder = gdb_bfd_ref_ptr::new_reference (nbfd);
 
   if (name == NULL)
-    name = xstrdup ("shared object read from target memory");
+    name = "shared object read from target memory";
   bfd_set_filename (nbfd, name);
 
   if (!bfd_check_format (nbfd, bfd_object))
@@ -183,8 +182,9 @@ add_vsyscall_page (struct target_ops *target, int from_tty)
 	  return;
 	}
 
-      char *name = xstrprintf ("system-supplied DSO at %s",
-			       paddress (target_gdbarch (), vsyscall_range.start));
+      std::string name = string_printf ("system-supplied DSO at %s",
+					paddress (target_gdbarch (),
+						  vsyscall_range.start));
       try
 	{
 	  /* Pass zero for FROM_TTY, because the action of loading the
@@ -193,7 +193,7 @@ add_vsyscall_page (struct target_ops *target, int from_tty)
 	  symbol_file_add_from_memory (bfd,
 				       vsyscall_range.start,
 				       vsyscall_range.length,
-				       name,
+				       name.c_str (),
 				       0 /* from_tty */);
 	}
       catch (const gdb_exception &ex)