On 12/16/2014 05:50 AM, Dave Anderson wrote:
----- Original Message -----
Hello Dave,
Thank you for suggestions.
I think I have further understanding of "keep it simple".
This time, there are five functions in the patch.
They are:
display_ELF_note()
display_prstatus_x86_64()
display_prstatus_x86()
display_qemu_x86_64()
display_qemu_x86()
It looks much better.
Previously, I tried to avoid duplicate code though it can bring
more complexity. Now, I think simple in logic is more important.
I do appreciate your teaching me so much.
Hello Zhou,
Thank you for tightening this up -- it is looking much better. But I still
have a few more questions, suggestions, and further simplifications.
In diskdump.c, you have this declaration:
+void display_ELF_note(int, int, void *, char *);
+
Please move this declaration in defs.h with the other function
prototypes under "netdump.c". You can also put the new PRSTATUS
and QEMU #define's right underneath the function declaration
instead of putting them at the bottom of defs.h.
Also in diskdump.c:
@@ -1736,10 +1738,18 @@ __diskdump_memory_dump(FILE *fp)
dd->num_prstatus_notes);
fprintf(fp, " notes_buf: %lx\n",
(ulong)dd->notes_buf);
+
+ char *l_buf = (char *)malloc(2 * BUFSIZE);
for (i = 0; i< dd->num_prstatus_notes; i++) {
fprintf(fp, " notes[%d]: %lx\n",
i, (ulong)dd->nt_prstatus_percpu[i]);
+
+ display_ELF_note(dd->machine_type, PRSTATUS,
+ dd->nt_prstatus_percpu[i],l_buf);
+ fprintf(fp, "%s", l_buf);
+ memset(l_buf, 0, 2 * BUFSIZE);
}
+ free(l_buf);
dump_nt_prstatus_offset(fp);
}
if (dh->header_version>= 5) {
First, what's the reason for the memset() call after the data has
been displayed? Aren't they always going to be the exact same,
NULL-terminated, size?
Secondly, the use of malloc() is discouraged during the runtime of
a crash session, such as when "help -D" is entered. The reason
for that is because:
(1) a crash command may fail and call the error(FATAL) function
for any number of reasons, or
(2) CTRL-c may be entered in the middle of the command.
In both cases, RESTART() is called, which does a longjmp() back to main().
So if malloc() had been called prior to either of those two occurances,
the buffer would be leaked. That's the reason behind using the
GETBUF()/FREEBUF facility, because buffers that GETBUF() returns are
guaranteed to be freed prior to the next "crash>" prompt, regardless
whether FREEBUF() had been called.
However, since "crash -d1" will also call __diskdump_memory_dump() very
early on before main() calls buf_init() -- which sets up the GETBUF()/FREEBUF()
facility -- GETBUF()/FREEBUF() aren't available for use at that time.
So to avoid having to choose between malloc() or GETBUF(), I would
just use a char[BUFSIZE*2] buffer automatically declared on the stack
in __diskdump_memory_dump().
In netdump.c, in the 3 places you call display_ELF_note() you qualify
it by checking "if (nd->ofp)". I'm not sure why you do that? You
should be able to just print the whole buffer by entering:
netdump_print("%s", l_buf);
Correct?
All have been addressed except this. Originally, I used the function
netdump_print(), but BUFSIZE in netdump_print() is not enough. So I
changed it to fprintf().
And like diskdump.c, the ELF dumping functions can be called
immediately during initialization if "crash -d1" is invoked,
so GETBUF() cannot be used, and using malloc() during runtime
via "help -D" could cause a memory leak. SO again, please
just declare a char[] buffer on the function stack.
And lastly, you'll note that none of the crash command output
ever use tabs as you have done. Can you replace them with spaces,
or alternatively, use the space(count) function to return a pointer
to a string with "count" number of spaces?
Thanks,
Dave
--
Thanks
Zhou Wenjian
From 53dc39f8271c2a0489a4a7de689b7c4f59f2dfdf Mon Sep 17 00:00:00 2001
From: Zhou Wenjian <zhouwj-fnst@xxxxxxxxxxxxxx>
Date: Tue, 16 Dec 2014 09:37:29 +0800
Subject: [PATCH] Make note info human readable when help -D
---
defs.h | 7 ++
diskdump.c | 7 ++
netdump.c | 312 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 326 insertions(+), 0 deletions(-)
diff --git a/defs.h b/defs.h
index 2e52bc4..76b113a 100755
--- a/defs.h
+++ b/defs.h
@@ -5547,6 +5547,13 @@ int write_proc_kcore(int, void *, int, ulong, physaddr_t);
int kcore_memory_dump(FILE *);
void dump_registers_for_qemu_mem_dump(void);
void kdump_backup_region_init(void);
+void display_ELF_note(int, int, void *, char *);
+
+/*
+ * for display_ELF_note() type
+ */
+#define PRSTATUS 0x1
+#define QEMU 0X2
/*
* ramdump.c
diff --git a/diskdump.c b/diskdump.c
index 3d33fdc..b6fbfd9 100644
--- a/diskdump.c
+++ b/diskdump.c
@@ -1736,9 +1736,16 @@ __diskdump_memory_dump(FILE *fp)
dd->num_prstatus_notes);
fprintf(fp, " notes_buf: %lx\n",
(ulong)dd->notes_buf);
+
+ char l_buf[BUFSIZE * 2];
+ BZERO(l_buf, BUFSIZE *2);
for (i = 0; i < dd->num_prstatus_notes; i++) {
fprintf(fp, " notes[%d]: %lx\n",
i, (ulong)dd->nt_prstatus_percpu[i]);
+
+ display_ELF_note(dd->machine_type, PRSTATUS,
+ dd->nt_prstatus_percpu[i],l_buf);
+ fprintf(fp, "%s", l_buf);
}
dump_nt_prstatus_offset(fp);
}
diff --git a/netdump.c b/netdump.c
index 903faa0..02f1bba 100644
--- a/netdump.c
+++ b/netdump.c
@@ -1976,6 +1976,12 @@ dump_Elf32_Nhdr(Elf32_Off offset, int store)
}
lf = 0;
} else {
+ if (nd->ofp) {
+ char l_buf[BUFSIZE * 2];
+ BZERO(l_buf, BUFSIZE *2);
+ display_ELF_note(EM_386, qemuinfo ? QEMU : PRSTATUS, note,l_buf);
+ fprintf(fp, "%s", l_buf);
+ }
for (i = lf = 0; i < note->n_descsz/sizeof(ulong); i++) {
if (((i%4)==0)) {
netdump_print("%s ",
@@ -2255,6 +2261,13 @@ dump_Elf64_Nhdr(Elf64_Off offset, int store)
}
if (BITS32() && (xen_core || (note->n_type == NT_PRSTATUS))) {
+ if (!xen_core && nd->ofp) {
+ char l_buf[BUFSIZE * 2];
+ BZERO(l_buf, BUFSIZE *2);
+ display_ELF_note(EM_386, PRSTATUS, note, l_buf);
+ fprintf(fp, "%s", l_buf);
+ }
+
iptr = (int *)uptr;
for (i = lf = 0; i < note->n_descsz/sizeof(ulong); i++) {
if (((i%4)==0)) {
@@ -2279,6 +2292,12 @@ dump_Elf64_Nhdr(Elf64_Off offset, int store)
iptr = (int *)uptr;
netdump_print(" %08lx\n", *iptr);
} else {
+ if (nd->ofp) {
+ char l_buf[BUFSIZE * 2];
+ BZERO(l_buf, BUFSIZE *2);
+ display_ELF_note(EM_X86_64, qemuinfo ? QEMU : PRSTATUS, note, l_buf);
+ fprintf(fp, "%s", l_buf);
+ }
for (i = lf = 0; i < note->n_descsz/sizeof(ulonglong); i++) {
if (((i%2)==0)) {
netdump_print("%s ",
@@ -2543,6 +2562,299 @@ struct x86_64_user_regs_struct {
unsigned long ds,es,fs,gs;
};
+struct x86_64_prstatus {
+ int si_signo;
+ int si_code;
+ int si_errno;
+ short cursig;
+ unsigned long sigpend;
+ unsigned long sighold;
+ int pid;
+ int ppid;
+ int pgrp;
+ int sid;
+ struct timeval utime;
+ struct timeval stime;
+ struct timeval cutime;
+ struct timeval cstime;
+ struct x86_64_user_regs_struct regs;
+ int fpvalid;
+};
+
+static void
+display_prstatus_x86_64(void *note_ptr, char *buf)
+{
+ struct x86_64_prstatus *pr;
+ Elf64_Nhdr *note;
+
+ note = (Elf64_Nhdr *)note_ptr;
+ pr = (struct x86_64_prstatus *)(
+ (char *)note + sizeof(Elf64_Nhdr) + note->n_namesz);
+ pr = (struct x86_64_prstatus *)roundup((ulong)pr, 4);
+ sprintf(buf,
+ " si.signo: %d, si.code: %d, si.errno: %d, cursig:%d\n"
+ " sigpend: %lu\n"
+ " sighold: %lu\n"
+ " pid: %d, ppid: %d, pgrp: %d, sid:%d\n"
+ " utime: %01lld.%06d, stime: %01lld.%06d, cutime: %01lld.%06d, "
+ "cstime: %01lld.%06d\n"
+ " ORIG_RAX: %ld, fpvalid: %d\n"
+ " R15 : 0x%016lx R14 : 0x%016lx\n"
+ " R13 : 0x%016lx R12 : 0x%016lx\n"
+ " RBP : 0x%016lx RBX : 0x%016lx\n"
+ " R11 : 0x%016lx R10 : 0x%016lx\n"
+ " R9 : 0x%016lx R8 : 0x%016lx\n"
+ " RAX : 0x%016lx RCX : 0x%016lx\n"
+ " RDX : 0x%016lx RSI : 0x%016lx\n"
+ " RDI : 0x%016lx RIP : 0x%016lx\n"
+ " RFLAGS : 0x%016lx RSP : 0x%016lx\n"
+ " FS_BASE: 0x%016lx GS_BASE : 0x%016lx\n"
+ " CS: 0x%04lx SS: 0x%04lx DS: 0x%04lx ES: 0x%04lx "
+ "FS: 0x%04lx GS: 0x%04lx\n",
+ pr->si_signo, pr->si_code, pr->si_errno, pr->cursig,
+ pr->sigpend, pr->sighold,
+ pr->pid, pr->ppid, pr->pgrp, pr->sid,
+ (long long)pr->utime.tv_sec, (int)pr->utime.tv_usec,
+ (long long)pr->stime.tv_sec, (int)pr->stime.tv_usec,
+ (long long)pr->cutime.tv_sec, (int)pr->cutime.tv_usec,
+ (long long)pr->cstime.tv_sec, (int)pr->cstime.tv_usec,
+ pr->regs.orig_rax, pr->fpvalid,
+ pr->regs.r15, pr->regs.r14,
+ pr->regs.r13, pr->regs.r12,
+ pr->regs.rbp, pr->regs.rbx,
+ pr->regs.r11, pr->regs.r10,
+ pr->regs.r9, pr->regs.r8,
+ pr->regs.rax, pr->regs.rcx,
+ pr->regs.rdx, pr->regs.rsi,
+ pr->regs.rdi, pr->regs.rip,
+ pr->regs.eflags, pr->regs.rsp,
+ pr->regs.fs_base, pr->regs.gs_base,
+ pr->regs.cs, pr->regs.ss,
+ pr->regs.ds, pr->regs.es,
+ pr->regs.fs, pr->regs.gs
+ );
+}
+
+struct x86_user_regs_struct {
+ unsigned long ebx,ecx,edx,esi,edi,ebp,eax;
+ unsigned long ds,es,fs,gs,orig_eax;
+ unsigned long eip,cs,eflags;
+ unsigned long esp,ss;
+};
+
+struct x86_prstatus {
+ int si_signo;
+ int si_code;
+ int si_errno;
+ short cursig;
+ unsigned long sigpend;
+ unsigned long sighold;
+ int pid;
+ int ppid;
+ int pgrp;
+ int sid;
+ struct timeval utime;
+ struct timeval stime;
+ struct timeval cutime;
+ struct timeval cstime;
+ struct x86_user_regs_struct regs;
+ int fpvalid;
+};
+
+static void
+display_prstatus_x86(void *note_ptr, char *buf)
+{
+ struct x86_prstatus *pr;
+ Elf32_Nhdr *note;
+
+ note = (Elf32_Nhdr *)note_ptr;
+ pr = (struct x86_prstatus *)(
+ (char *)note + sizeof(Elf32_Nhdr) + note->n_namesz);
+ pr = (struct x86_prstatus *)roundup((ulong)pr, 4);
+
+ sprintf(buf,
+ " si.signo: %d si.code: %d si.errno: %d cursig: %d\n"
+ " sigpend : %lu\n"
+ " sighold : %lu\n"
+ " pid: %d ppid: %d pgrp: %d sid: %d\n"
+ " utime: %01lld.%06d, stime: %01lld.%06d, cutime: %01lld.%06d, "
+ "cstime: %01lld.%06d\n"
+ " orig_eax: %ld, fpvalid: %d\n"
+ " EBX : 0x%08lx ECX : 0x%08lx\n"
+ " EDX : 0x%08lx ESI : 0x%08lx\n"
+ " EDI : 0x%08lx EBP : 0x%08lx\n"
+ " EAX : 0x%08lx EIP : 0x%08lx\n"
+ " EFLAGS : 0x%08lx ESP : 0x%08lx\n"
+ " DS: 0x%04lx ES: 0x%04lx FS: 0x%04lx GS: 0x%04lx "
+ "CS: 0x%04lx SS: 0x%04lx\n",
+ pr->si_signo, pr->si_code, pr->si_errno, pr->cursig,
+ pr->sigpend, pr->sighold,
+ pr->pid, pr->ppid, pr->pgrp, pr->sid,
+ (long long)pr->utime.tv_sec, (int)pr->utime.tv_usec,
+ (long long)pr->stime.tv_sec, (int)pr->stime.tv_usec,
+ (long long)pr->cutime.tv_sec, (int)pr->cutime.tv_usec,
+ (long long)pr->cstime.tv_sec, (int)pr->cstime.tv_usec,
+ pr->regs.orig_eax, pr->fpvalid,
+ pr->regs.ebx, pr->regs.ecx,
+ pr->regs.edx, pr->regs.esi,
+ pr->regs.edi, pr->regs.ebp,
+ pr->regs.eax, pr->regs.eip,
+ pr->regs.eflags, pr->regs.esp,
+ pr->regs.ds, pr->regs.es, pr->regs.fs,
+ pr->regs.gs, pr->regs.cs, pr->regs.ss
+ );
+}
+
+static void
+display_qemu_x86_64(void *note_ptr, char *buf)
+{
+ int i, size;
+ Elf64_Nhdr *note;
+ QEMUCPUState *ptr;
+ QEMUCPUSegment *seg;
+ char *seg_names[] = {"CS", "DS", "ES", "FS", "GS", "SS", "LDT", "TR",
+ "GDT", "IDT"};
+
+ note = (Elf64_Nhdr *)note_ptr;
+ ptr = (QEMUCPUState *)(
+ (char *)note + sizeof(Elf64_Nhdr) + note->n_namesz);
+
+ ptr = (QEMUCPUState *)roundup((ulong)ptr, 4);
+ seg = &(ptr->cs);
+
+ size = sprintf(buf,
+ " version: 0x%08x size: 0x%08x\n"
+ " RAX : 0x%016llx RBX : 0x%016llx\n"
+ " RCX : 0x%016llx RDX : 0x%016llx\n"
+ " RSI : 0x%016llx RDI : 0x%016llx\n"
+ " RSP : 0x%016llx RBP : 0x%016llx\n"
+ " RIP : 0x%016llx RFLAGS: 0x%016llx\n"
+ " R8 : 0x%016llx R9 : 0x%016llx\n"
+ " R10 : 0x%016llx R11 : 0x%016llx\n"
+ " R12 : 0x%016llx R13 : 0x%016llx\n"
+ " R14 : 0x%016llx R15 : 0x%016llx\n",
+ ptr->version, ptr->size,
+ (ulonglong)ptr->rax, (ulonglong)ptr->rbx,
+ (ulonglong)ptr->rcx, (ulonglong)ptr->rdx,
+ (ulonglong)ptr->rsi, (ulonglong)ptr->rdi,
+ (ulonglong)ptr->rsp, (ulonglong)ptr->rbp,
+ (ulonglong)ptr->rip, (ulonglong)ptr->rflags,
+ (ulonglong)ptr->r8, (ulonglong)ptr->r9,
+ (ulonglong)ptr->r10, (ulonglong)ptr->r11,
+ (ulonglong)ptr->r12, (ulonglong)ptr->r13,
+ (ulonglong)ptr->r14, (ulonglong)ptr->r15
+ );
+ buf += size;
+
+ for(i = 0; i < sizeof(seg_names)/sizeof(seg_names[0]); i++) {
+ size = sprintf(buf,
+ " %s:\n"
+ " selector: 0x%08x limit: 0x%08x flags: 0x%08x\n"
+ " pad : 0x%08x base : 0x%016llx\n",
+ seg_names[i],
+ seg->selector, seg->limit, seg->flags,
+ seg->pad, (ulonglong)seg->base
+ );
+ buf += size;
+ seg++;
+ }
+
+ sprintf(buf,
+ " cr[0]: %016llx cr[1]: %016llx cr[2]: %016llx\n"
+ " cr[3]: %016llx cr[4]: %016llx\n",
+ (ulonglong)ptr->cr[0], (ulonglong)ptr->cr[1], (ulonglong)ptr->cr[2],
+ (ulonglong)ptr->cr[3], (ulonglong)ptr->cr[4]
+ );
+}
+
+static void
+display_qemu_x86(void *note_ptr, char *buf)
+{
+ int i, size, t=0;
+ Elf32_Nhdr *note;
+ QEMUCPUState *ptr;
+ QEMUCPUSegment *seg;
+ char *seg_names[] = {"CS", "DS", "ES", "FS", "GS", "SS", "LDT", "TR",
+ "GDT", "IDT"};
+
+
+ note = (Elf32_Nhdr *)note_ptr;
+ ptr = (QEMUCPUState *)(
+ (char *)note + sizeof(Elf32_Nhdr) + note->n_namesz);
+ ptr = (QEMUCPUState *)roundup((ulong)ptr, 4);
+ seg = &(ptr->cs);
+
+ size = sprintf(buf,
+ " version: 0x%08x\tsize: 0x%08x\n"
+ " EAX : 0x%016llx\tEBX : 0x%016llx\n"
+ " ECX : 0x%016llx\tEDX : 0x%016llx\n"
+ " ESI : 0x%016llx\tEDI : 0x%016llx\n"
+ " ESP : 0x%016llx\tEBP : 0x%016llx\n"
+ " EIP : 0x%016llx\tEFLAGS: 0x%016llx\n",
+ ptr->version, ptr->size,
+ (ulonglong)ptr->rax, (ulonglong)ptr->rbx, (ulonglong)ptr->rcx,
+ (ulonglong)ptr->rdx, (ulonglong)ptr->rsi, (ulonglong)ptr->rdi,
+ (ulonglong)ptr->rsp, (ulonglong)ptr->rbp,
+ (ulonglong)ptr->rip, (ulonglong)ptr->rflags
+ );
+ buf += size;
+ t+=size;
+
+ for(i = 0; i < sizeof(seg_names)/sizeof(seg_names[0]); i++) {
+ size = sprintf(buf,
+ " %s:\n"
+ " selector: 0x%08x limit: 0x%08x flags: 0x%08x\n"
+ " pad : 0x%08x base : 0x%016llx\n",
+ seg_names[i],
+ seg->selector, seg->limit, seg->flags,
+ seg->pad, (ulonglong)seg->base
+ );
+ buf += size;
+ seg++;
+ }
+
+ sprintf(buf,
+ " cr[0]: %016llx cr[1]: %016llx cr[2]: %016llx\n"
+ " cr[3]: %016llx cr[4]: %016llx\n",
+ (ulonglong)ptr->cr[0], (ulonglong)ptr->cr[1], (ulonglong)ptr->cr[2],
+ (ulonglong)ptr->cr[3], (ulonglong)ptr->cr[4]
+ );
+}
+
+void
+display_ELF_note(int machine, int type, void *note, char *buf)
+{
+ switch (machine)
+ {
+ case EM_386:
+ switch (type)
+ {
+ case PRSTATUS:
+ display_prstatus_x86(note, buf);
+ break;
+ case QEMU:
+ display_qemu_x86(note, buf);
+ break;
+ }
+ break;
+
+ case EM_X86_64:
+ switch (type)
+ {
+ case PRSTATUS:
+ display_prstatus_x86_64(note, buf);
+ break;
+ case QEMU:
+ display_qemu_x86_64(note, buf);
+ break;
+ }
+ break;
+
+ default:
+ return;
+ }
+}
+
void
get_netdump_regs_x86_64(struct bt_info *bt, ulong *ripp, ulong *rspp)
{
--
1.7.1
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
