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

Roland McGrath
2003-10-07T08:19:13Z

After making gcore dump NT_AUXV notes, I noticed that examining the
resultant dumps still had the problem that the vsyscall DSO page was not
included in the dump. I changed Linux 2.6 to show that region of memory in
/proc/PID/maps, which is the natural way to have it show up properly for
gdb's target_find_memory_regions. After testing my kernel changes I tried
gcore again and was surprised to find that it still didn't produce a dump
actually containing the magic page's data. I tracked this down to
gcore_create_callback, where it avoids writing any read-only sections into
the dump. It's obviously desireable to omit all the text segments from
executables and shared libraries so that core dumps are not always huge.
But you lose information by omitting read-only pages that didn't come from
files gdb knows to look in. The following patch to gcore.c makes it omit a
read-only section only if it's found in a known on-disk BFD.

With that change alone, gcore omits only the text segment of the executable
itself and not only (rightly) dumps the vsyscall DSO page but also
(wrongly) dumps all the normal shared libraries' text segments. I tracked
this down to to_sections getting the solib sections only when the right
argument is passed to SOLIB_ADD, and the infrun.c patch below changes that.
I don't understand why that wasn't done already, or what effects the change
might have; I have not noticed any problems, and gcore now omits all the
right things and none of the wrong ones.

I was motivated to discover this by wanting gcore to produce complete
results for the vsyscall page so unwinding works when examining the dump.
This is in part a generalized solution motivated by avoiding a weird
special case kludge for the vsyscall page. But I think it's also
desireable for other cases. Take for example this program:

#include <sys/mman.h>
#include <string.h>
#include <unistd.h>

int main ()
{
const size_t pagesz = sysconf (_SC_PAGE_SIZE);
void *page = mmap (0, pagesz, PROT_READ|PROT_WRITE,
MAP_ANON|MAP_PRIVATE, -1, 0);
memset (page, 0x17, pagesz);
mprotect (page, pagesz, PROT_READ);
printf ("pid %d address %p size %x\n", getpid (),
page, (unsigned int) pagesz);
abort ();
}

Without this change, a gcore made with the process stopped at the abort or
printf calls after mprotect will not include the `page' page--when
examining the dump, there will be no way to know it contains 0x17. This is
obviously a contrived and useless example. But such cases do exist in the
real world, e.g. garbage collectors that temporarily mprotect some pages to
read-only--in a core dump of such a process, knowing the contents of these
pages could be important. With my change, that page appears in the dump.

Note that the existing gcore behavior is consistent with what the Linux
kernel does in making dumps: it omits all read-only pages. However, I
think that's arguably a bug because of cases like the one I've just cited.
I am submitting patches to make Linux 2.6 omit only the read-only pages
that are backed by mapped files, and include all anonymous pages in the dump.

Note that this patch makes gcore dump more regions than the Linux kernel
does even with my change to its behavior. In particular, read-only mmap'd
portions of files are dumped by gcore but not by the kernel. This is a
real common issue in practice, as your average GNU/Linux process nowadays
has the large locale-archive file mapped in, and some processes may be
mapping huge files in read-only. An alternative change would be to change
the to_find_memory_regions callback interface to add a flag argument saying
whether the memory region came from a file. Then gcore_create_callback
could simply test !write && !anonymous and be wholly consistent with the
kernel core dumping (assuming my change to it), and the infrun.c change is
not required (though I'm still curious about why it's not that way already).

Comments?

Thanks,
Roland

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

* gcore.c (gcore_create_callback): Omit read-only sections only if
they correspond to a known disk file.
* infrun.c (handle_inferior_event): Pass &current_target to SOLIB_ADD.

Index: gcore.c
===================================================================
RCS file: /cvs/src/src/gdb/gcore.c,v
retrieving revision 1.12
diff -p -u -r1.12 gcore.c
--- gcore.c 21 Sep 2003 01:26:44 -0000 1.12
+++ gcore.c 7 Oct 2003 07:40:11 -0000
@@ -343,10 +343,38 @@ gcore_create_callback (CORE_ADDR vaddr,

if (write == 0)
{
+ /* See if this region of memory lies inside a known file on disk. */
+ struct section_table *secp;
+ for (secp = current_target.to_sections;
+ secp < current_target.to_sections_end;
+ secp++)
+ {
+ asection *const asec = secp->the_bfd_section;
+ bfd_vma align = (bfd_vma) 1 << bfd_get_section_alignment (secp->bfd,
+ asec);
+ bfd_vma start = secp->addr & -align;
+ bfd_vma end = (secp->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). */
+ if ((vaddr >= start && vaddr + size <= end)
+ || (start >= vaddr && end <= vaddr + size))
+ {
+ if (bfd_get_file_flags (secp->bfd) & BFD_IN_MEMORY)
+ /* This BFD was synthesized from reading target memory,
+ so we don't want to omit that. */
+ continue;
+ break;
+ }
+ }
+
flags |= SEC_READONLY;
- /* Mark readonly sections as zero-sized, such that we can avoid
- copying their contents. */
- size = 0;
+
+ if (secp < current_target.to_sections_end)
+ /* Mark readonly sections as zero-sized, such that we can avoid
+ copying their contents. */
+ size = 0;
}

if (exec)
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.115
diff -b -p -u -r1.115 infrun.c
--- infrun.c 2 Oct 2003 20:28:29 -0000 1.115
+++ infrun.c 7 Oct 2003 07:40:23 -0000
@@ -1366,7 +1366,7 @@ handle_inferior_event (struct execution_
terminal for any messages produced by
breakpoint_re_set. */
target_terminal_ours_for_output ();
- SOLIB_ADD (NULL, 0, NULL, auto_solib_add);
+ SOLIB_ADD (NULL, 0, &current_target, auto_solib_add);
target_terminal_inferior ();

/* Reinsert breakpoints and continue. */
@@ -2189,7 +2189,7 @@ process_event_stop_test:
terminal for any messages produced by
breakpoint_re_set. */
target_terminal_ours_for_output ();
- SOLIB_ADD (NULL, 0, NULL, auto_solib_add);
+ SOLIB_ADD (NULL, 0, &current_target, auto_solib_add);
target_terminal_inferior ();

/* Try to reenable shared library breakpoints, additional