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

Roland McGrath
2003-10-09T02:44:53Z

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