[src/trunk]: src/sys Minor changes to the process coredump code.

[dsl <dsl%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/d8c329f1fd28
branches:  trunk
changeset: 325693:d8c329f1fd28
user:      dsl <dsl%NetBSD.org@localhost>
date:      Fri Jan 03 15:15:02 2014 +0000

description:
Minor changes to the process coredump code.
- Add some extra comments.
- Add some XXX comments because the process state might not be stable,
- Add uvm_coredump_count_segs() to simplify the calling code.
- uvm code now only returns non-empty sections/segments.
- Put the 'iocookie' into the 'cookie' block passed to uvm_coredump_walkmap()
  instead of passing it through as an additional parameter.
amd64 can still generate core dumps that gdb can read.

diffstat:

 sys/kern/core_elf32.c  |  66 ++++++++++++++++---------------------------------
 sys/kern/core_netbsd.c |  40 ++++++++---------------------
 sys/uvm/uvm_coredump.c |  38 ++++++++++++++++++++++------
 sys/uvm/uvm_extern.h   |   9 +++---
 4 files changed, 67 insertions(+), 86 deletions(-)

diffs (truncated from 374 to 300 lines):

diff -r ae3dc0357896 -r d8c329f1fd28 sys/kern/core_elf32.c
--- a/sys/kern/core_elf32.c     Fri Jan 03 14:41:57 2014 +0000
+++ b/sys/kern/core_elf32.c     Fri Jan 03 15:15:02 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: core_elf32.c,v 1.37 2014/01/01 18:57:16 dsl Exp $      */
+/*     $NetBSD: core_elf32.c,v 1.38 2014/01/03 15:15:02 dsl Exp $      */
 
 /*
  * Copyright (c) 2001 Wasabi Systems, Inc.
@@ -40,7 +40,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(1, "$NetBSD: core_elf32.c,v 1.37 2014/01/01 18:57:16 dsl Exp $");
+__KERNEL_RCSID(1, "$NetBSD: core_elf32.c,v 1.38 2014/01/03 15:15:02 dsl Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_coredump.h"
@@ -66,20 +66,13 @@
 
 #ifdef COREDUMP
 
-struct countsegs_state {
-       int     npsections;
-};
-
-static int     ELFNAMEEND(coredump_countsegs)(struct proc *,
-                   struct coredump_iostate *, struct uvm_coredump_state *);
-
 struct writesegs_state {
        Elf_Phdr *psections;
        off_t   secoff;
 };
 
-static int     ELFNAMEEND(coredump_writeseghdrs)(struct proc *,
-                   struct coredump_iostate *, struct uvm_coredump_state *);
+static int     ELFNAMEEND(coredump_getseghdrs)(struct proc *,
+                   struct uvm_coredump_state *);
 
 static int     ELFNAMEEND(coredump_notes)(struct proc *, struct lwp *,
                    struct coredump_iostate *, size_t *);
@@ -105,7 +98,7 @@
        Elf_Ehdr ehdr;
        Elf_Phdr phdr, *psections;
        size_t psectionssize;
-       struct countsegs_state cs;
+       int npsections;
        struct writesegs_state ws;
        off_t notestart, secstart, offset;
        size_t notesize;
@@ -117,7 +110,7 @@
         * We have to make a total of 3 passes across the map:
         *
         *      1. Count the number of map entries (the number of
-        *         PT_LOAD sections).
+        *         PT_LOAD sections in the dump).
         *
         *      2. Write the P-section headers.
         *
@@ -125,16 +118,11 @@
         */
 
        /* Pass 1: count the entries. */
-       cs.npsections = 0;
-       error = uvm_coredump_walkmap(p, NULL,
-           ELFNAMEEND(coredump_countsegs), &cs);
-       if (error)
-               goto out;
+       npsections = uvm_coredump_count_segs(p);
+       /* Count the PT_NOTE section. */
+       npsections++;
 
-       /* Count the PT_NOTE section. */
-       cs.npsections++;
-
-       /* Get the size of the notes. */
+       /* Get the size of the notes (mostly all the registers). */
        error = ELFNAMEEND(coredump_notes)(p, l, NULL, &notesize);
        if (error)
                goto out;
@@ -162,7 +150,7 @@
        ehdr.e_flags = 0;
        ehdr.e_ehsize = sizeof(ehdr);
        ehdr.e_phentsize = sizeof(Elf_Phdr);
-       ehdr.e_phnum = cs.npsections;
+       ehdr.e_phnum = npsections;
        ehdr.e_shentsize = 0;
        ehdr.e_shnum = 0;
        ehdr.e_shstrndx = 0;
@@ -178,21 +166,20 @@
 
        offset = sizeof(ehdr);
 
-       notestart = offset + sizeof(phdr) * cs.npsections;
+       notestart = offset + sizeof(phdr) * npsections;
        secstart = notestart + notesize;
 
-       psectionssize = cs.npsections * sizeof(Elf_Phdr);
+       psectionssize = npsections * sizeof(Elf_Phdr);
        psections = kmem_zalloc(psectionssize, KM_SLEEP);
 
-       /* Pass 2: now write the P-section headers. */
+       /* Pass 2: now find the P-section headers. */
        ws.secoff = secstart;
        ws.psections = psections;
-       error = uvm_coredump_walkmap(p, cookie,
-           ELFNAMEEND(coredump_writeseghdrs), &ws);
+       error = uvm_coredump_walkmap(p, ELFNAMEEND(coredump_getseghdrs), &ws);
        if (error)
                goto out;
 
-       /* Write out the PT_NOTE header. */
+       /* Add the PT_NOTE header after the P-section headers. */
        ws.psections->p_type = PT_NOTE;
        ws.psections->p_offset = notestart;
        ws.psections->p_vaddr = 0;
@@ -202,13 +189,14 @@
        ws.psections->p_flags = PF_R;
        ws.psections->p_align = ELFROUNDSIZE;
 
+       /* Write the P-section headers followed by the PT_NOTR header */
        error = coredump_write(cookie, UIO_SYSSPACE, psections,
-           cs.npsections * sizeof(Elf_Phdr));
+           npsections * sizeof(Elf_Phdr));
        if (error)
                goto out;
 
 #ifdef DIAGNOSTIC
-       offset += cs.npsections * sizeof(Elf_Phdr);
+       offset += npsections * sizeof(Elf_Phdr);
        if (offset != notestart)
                panic("coredump: offset %lld != notestart %lld",
                    (long long) offset, (long long) notestart);
@@ -227,7 +215,7 @@
 #endif
 
        /* Pass 3: finally, write the sections themselves. */
-       for (i = 0; i < cs.npsections - 1; i++) {
+       for (i = 0; i < npsections - 1; i++) {
                if (psections[i].p_filesz == 0)
                        continue;
 
@@ -256,18 +244,7 @@
 }
 
 static int
-ELFNAMEEND(coredump_countsegs)(struct proc *p,
-    struct coredump_iostate *iocookie, struct uvm_coredump_state *us)
-{
-       struct countsegs_state *cs = us->cookie;
-
-       cs->npsections++;
-       return (0);
-}
-
-static int
-ELFNAMEEND(coredump_writeseghdrs)(struct proc *p,
-    struct coredump_iostate *iocookie, struct uvm_coredump_state *us)
+ELFNAMEEND(coredump_getseghdrs)(struct proc *p, struct uvm_coredump_state *us)
 {
        struct writesegs_state *ws = us->cookie;
        Elf_Phdr phdr;
@@ -279,6 +256,7 @@
        realsize = us->realend - us->start;
        end = us->realend;
 
+       /* Don't bother writing out trailing zeros */
        while (realsize > 0) {
                long buf[1024 / sizeof(long)];
                size_t slen = realsize > sizeof(buf) ? sizeof(buf) : realsize;
diff -r ae3dc0357896 -r d8c329f1fd28 sys/kern/core_netbsd.c
--- a/sys/kern/core_netbsd.c    Fri Jan 03 14:41:57 2014 +0000
+++ b/sys/kern/core_netbsd.c    Fri Jan 03 15:15:02 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: core_netbsd.c,v 1.19 2014/01/01 18:57:16 dsl Exp $     */
+/*     $NetBSD: core_netbsd.c,v 1.20 2014/01/03 15:15:02 dsl Exp $     */
 
 /*
  * Copyright (c) 1997 Charles D. Cranor and Washington University.
@@ -45,7 +45,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: core_netbsd.c,v 1.19 2014/01/01 18:57:16 dsl Exp $");
+__KERNEL_RCSID(0, "$NetBSD: core_netbsd.c,v 1.20 2014/01/03 15:15:02 dsl Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_coredump.h"
@@ -70,13 +70,12 @@
 #ifdef COREDUMP
 
 struct coredump_state {
+       struct coredump_iostate *iocookie;
        struct CORENAME(core) core;
 };
 
-static int     CORENAME(coredump_countsegs_netbsd)(struct proc *,
-                   struct coredump_iostate *, struct uvm_coredump_state *);
 static int     CORENAME(coredump_writesegs_netbsd)(struct proc *,
-                   struct coredump_iostate *, struct uvm_coredump_state *);
+                   struct uvm_coredump_state *);
 
 int
 CORENAME(coredump_netbsd)(struct lwp *l, struct coredump_iostate *iocookie)
@@ -86,6 +85,7 @@
        struct vmspace *vm = p->p_vmspace;
        int error;
 
+       cs.iocookie = iocookie;
        cs.core.c_midmag = 0;
        strncpy(cs.core.c_name, p->p_comm, MAXCOMLEN);
        cs.core.c_nseg = 0;
@@ -99,10 +99,7 @@
        error = CORENAME(cpu_coredump)(l, NULL, &cs.core);
        if (error)
                return (error);
-       error = uvm_coredump_walkmap(p, NULL,
-           CORENAME(coredump_countsegs_netbsd), &cs);
-       if (error)
-               return (error);
+       cs.core.c_nseg = uvm_coredump_count_segs(p);
 
        /* First write out the core header. */
        error = coredump_write(iocookie, UIO_SYSSPACE, &cs.core,
@@ -116,33 +113,18 @@
                return (error);
 
        /* Finally, the address space dump */
-       return uvm_coredump_walkmap(p, iocookie,
-           CORENAME(coredump_writesegs_netbsd), &cs);
-}
-
-static int
-CORENAME(coredump_countsegs_netbsd)(struct proc *p,
-    struct coredump_iostate *iocookie, struct uvm_coredump_state *us)
-{
-       struct coredump_state *cs = us->cookie;
-
-       if (us->start != us->realend)
-               cs->core.c_nseg++;
-
-       return (0);
+       return uvm_coredump_walkmap(p, CORENAME(coredump_writesegs_netbsd),
+           &cs);
 }
 
 static int
 CORENAME(coredump_writesegs_netbsd)(struct proc *p,
-    struct coredump_iostate *iocookie, struct uvm_coredump_state *us)
+    struct uvm_coredump_state *us)
 {
        struct coredump_state *cs = us->cookie;
        struct CORENAME(coreseg) cseg;
        int flag, error;
 
-       if (us->start == us->realend)
-               return (0);
-
        if (us->flags & UVM_COREDUMP_STACK)
                flag = CORE_STACK;
        else
@@ -155,12 +137,12 @@
        cseg.c_addr = us->start;
        cseg.c_size = us->end - us->start;
 
-       error = coredump_write(iocookie, UIO_SYSSPACE,
+       error = coredump_write(cs->iocookie, UIO_SYSSPACE,
            &cseg, cs->core.c_seghdrsize);
        if (error)
                return (error);
 
-       return coredump_write(iocookie, UIO_USERSPACE,
+       return coredump_write(cs->iocookie, UIO_USERSPACE,
            (void *)(vaddr_t)us->start, cseg.c_size);
 }
 
diff -r ae3dc0357896 -r d8c329f1fd28 sys/uvm/uvm_coredump.c
--- a/sys/uvm/uvm_coredump.c    Fri Jan 03 14:41:57 2014 +0000
+++ b/sys/uvm/uvm_coredump.c    Fri Jan 03 15:15:02 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: uvm_coredump.c,v 1.3 2014/01/01 18:57:16 dsl Exp $     */
+/*     $NetBSD: uvm_coredump.c,v 1.4 2014/01/03 15:15:02 dsl Exp $     */
 
 /*
  * Copyright (c) 1997 Charles D. Cranor and Washington University.
@@ -62,7 +62,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uvm_coredump.c,v 1.3 2014/01/01 18:57:16 dsl Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uvm_coredump.c,v 1.4 2014/01/03 15:15:02 dsl Exp $");
 
 /*
  * uvm_coredump.c: glue functions for coredump
@@ -77,13 +77,15 @@
 /*
  * uvm_coredump_walkmap: walk a process's map for the purpose of dumping
  * a core file.
+ * XXX: I'm not entirely sure the locking is this function is in anyway
+ * correct.  If the process isn't actually stopped then the data passed