Re: [PATCH] make gcore dump read-only sections not from files

[Roland McGrath <roland at redhat dot com>]

> I'll step up, since I wrote gcore.  I like what you're doing,
> but I'm uncertain about the SOLIB_ADD part.  Like you, I don't
> understand why it was the way it was, nor the implications of
> the change.  But I think this can be done fairly easily without
> that change.

Ok.  I would sure like to know why core files work differently this way.
It would be nice if there were any comments in the code, for example!
The comment above update_solib_list says it's used for core files and
attaching, which is true.  But it says nothing about why.  I don't
understand why anything about this part of the solib handling would be
different for core files than for running.

> Does this (rewriting your main loop using ALL_OBJSECTIONS)
> seem reasonable?

Sure does.  I didn't read enough code to understand exactly what
objfile_find_memory_regions was doing and misread it as doing less.

Along the way I noticed another difference between gcore-produced and
kernel-produced core dumps.  The omitted segments in real core dumps
have nonzero p_memsz but zero p_filesz, which in phdrs indicates that
the memory is occupied but the contents are not available.  gcore's
dumps zero the size, which gives a wrong indication of the address space.
I changed that as well, so gcore's dumps now look more like real dumps.

This works well enough.  However, I think that making the determination
based on the kernel-supplied indication of anonymous vs file-backed may
make more sense.  (Linux 2.6's behavior may be changing in this regard,
and using that as a determining factor rather than just permission
bits.)  That would require changing the to_find_memory_regions interface
as I described earlier.  Can you comment on that?


Thanks,
Roland


2003-10-08  Roland McGrath  <roland@redhat.com>

	* gcore.c (make_mem_sec): Function removed, folded into ...
	(gcore_create_callback): ... here.  To omit a section, clear its
	SEC_LOAD bit rather than zeroing its size.
	Omit read-only sections only if they correspond to a known disk file.
	(gcore_copy_callback): Ignore sections without SEC_LOAD flag set.

--- gcore.c.~1.12.~	2003-09-23 03:01:26.000000000 -0700
+++ gcore.c	2003-10-08 19:39:11.000000000 -0700
@@ -306,55 +306,73 @@ make_output_phdrs (bfd *obfd, asection *
   bfd_record_phdr (obfd, p_type, 1, p_flags, 0, 0, 0, 0, 1, &osec);
 }
 
-static asection *
-make_mem_sec (bfd *obfd, bfd_vma addr, bfd_size_type size,
-	      unsigned int flags, unsigned int alignment)
+static int
+gcore_create_callback (CORE_ADDR vaddr, unsigned long size,
+		       int read, int write, int exec, void *data)
 {
+  bfd *obfd = data;
   asection *osec;
+  flagword flags = SEC_ALLOC | SEC_HAS_CONTENTS | SEC_LOAD;
+
+  if (write == 0)
+    {
+      /* See if this region of memory lies inside a known file on disk.
+	 If so, we can avoid copying its contents by clearing SEC_LOAD.  */
+      struct objfile *objfile;
+      struct obj_section *objsec;
+
+      ALL_OBJSECTIONS (objfile, objsec)
+	{
+	  bfd *abfd = objfile->obfd;
+	  asection *asec = objsec->the_bfd_section;
+	  bfd_vma align = (bfd_vma) 1 << bfd_get_section_alignment (abfd,
+								    asec);
+	  bfd_vma start = objsec->addr & -align;
+	  bfd_vma end = (objsec->endaddr + align - 1) & -align;
+	  /* Match if either the entire memory region lies inside the
+	     section (i.e. a mapping covering some pages of a large
+	     segment) or the entire section lies inside the memory region
+	     (i.e. a mapping covering multiple small sections).
+
+	     This BFD was synthesized from reading target memory,
+	     we don't want to omit that.  */
+	  if (((vaddr >= start && vaddr + size <= end)
+	       || (start >= vaddr && end <= vaddr + size))
+	      && !(bfd_get_file_flags (abfd) & BFD_IN_MEMORY))
+	    {
+	      flags &= ~SEC_LOAD;
+	      goto keep;	/* break out of two nested for loops */
+	    }
+	}
+
+    keep:
+      flags |= SEC_READONLY;
+    }
+
+  if (exec)
+    flags |= SEC_CODE;
+  else
+    flags |= SEC_DATA;
 
   osec = bfd_make_section_anyway (obfd, "load");
   if (osec == NULL)
     {
       warning ("Couldn't make gcore segment: %s",
 	       bfd_errmsg (bfd_get_error ()));
-      return NULL;
+      return 1;
     }
 
   if (info_verbose)
     {
       fprintf_filtered (gdb_stdout, "Save segment, %lld bytes at 0x%s\n",
-			(long long) size, paddr_nz (addr));
+			(long long) size, paddr_nz (vaddr));
     }
 
   bfd_set_section_size (obfd, osec, size);
-  bfd_set_section_vma (obfd, osec, addr);
+  bfd_set_section_vma (obfd, osec, vaddr);
   bfd_section_lma (obfd, osec) = 0; /* ??? bfd_set_section_lma?  */
-  bfd_set_section_alignment (obfd, osec, alignment);
-  bfd_set_section_flags (obfd, osec,
-			 flags | SEC_LOAD | SEC_ALLOC | SEC_HAS_CONTENTS);
-  return osec;
-}
-
-static int
-gcore_create_callback (CORE_ADDR vaddr, unsigned long size,
-		       int read, int write, int exec, void *data)
-{
-  flagword flags = 0;
-
-  if (write == 0)
-    {
-      flags |= SEC_READONLY;
-      /* Mark readonly sections as zero-sized, such that we can avoid
-         copying their contents.  */
-      size = 0;
-    }
-
-  if (exec)
-    flags |= SEC_CODE;
-  else
-    flags |= SEC_DATA;
-
-  return ((make_mem_sec (data, vaddr, size, flags, 0)) == NULL);
+  bfd_set_section_flags (obfd, osec, flags);
+  return 0;
 }
 
 static int
@@ -416,9 +434,8 @@ gcore_copy_callback (bfd *obfd, asection
   struct cleanup *old_chain = NULL;
   void *memhunk;
 
-  /* Read-only sections are marked as zero-size.  We don't have to
-     copy their contents.  */
-  if (size == 0)
+  /* Read-only sections are marked; we don't have to copy their contents.  */
+  if ((bfd_get_section_flags (obfd, osec) & SEC_LOAD) == 0)
     return;
 
   /* Only interested in "load" sections.  */