10:38 a.m.
Hello all,
When analysing elf format core, if "help -D" is specified, crash will
show dumpfile contents/statistics. But the output is hardly to read.
So the patches is to make it human readable.
When analysing kdump format core, if "help -D" is specified, crash won't
show the note information. The patches add note information to the output.
When analysing elf format core, if "help -D" is specified,
the original output like this:
:
:
Elf64_Nhdr:
n_namesz: 5 ("CORE")
n_descsz: 336
n_type: 1 (NT_PRSTATUS)
0000000000000000 0000000000000000
0000000000000000 0000000000000000
000000000000141d 0000000000000000
0000000000000000 0000000000000000
0000000000000000 0000000000000000
0000000000000000 0000000000000000
0000000000000000 0000000000000000
0000000000000004 0000000000000286
ffffffff81b01a40 0000000000000000
ffff88003d86be18 0000000000000063
0000000000000000 0000000000000001
ffffffff81645da0 0000000000000000
0000000000000010 0000000000000000
0000000000000000 0000000000000000
0000000000000063 ffffffffffffffff
ffffffff8134b6c6 0000000000000010
0000000000010096 ffff88003d86be18
0000000000000018 00007fc165c0b700
0000000000000000 0000000000000000
0000000000000000 0000000000000000
0000000000000000 0000000000000000
:
:
It is hardly to get some useful information. The patch is to make it
human readable. And the output changes to:
:
:
Elf64_Nhdr:
n_namesz: 5 ("CORE")
n_descsz: 336
n_type: 1 (NT_PRSTATUS)
si.signo: 0, si.code: 0, si.errno: 0, cursig:0
sigpend: 0
sighold: 0
pid: 5149, ppid: 0, pgrp: 0, sid:0
utime: 0.000000, stime: 0.000000, cutime: 0.000000, cstime: 0.000000
ORIG_RAX: -1, fpvalid: 0
R15 : 0x0000000000000004 R14 : 0x0000000000000286
R13 : 0xffffffff81b01a40 R12 : 0x0000000000000000
RBP : 0xffff88003d86be18 RBX : 0x0000000000000063
R11 : 0x0000000000000000 R10 : 0x0000000000000001
R9 : 0xffffffff81645da0 R8 : 0x0000000000000000
RAX : 0x0000000000000010 RCX : 0x0000000000000000
RDX : 0x0000000000000000 RSI : 0x0000000000000000
RDI : 0x0000000000000063 RIP : 0xffffffff8134b6c6
RFLAGS : 0x0000000000010096 RSP : 0xffff88003d86be18
FS_BASE: 0x00007fc165c0b700 GS_BASE : 0x0000000000000000
CS: 0x0010 SS: 0x0018 DS: 0x0000 ES: 0x0000 FS: 0x0000 GS: 0x0000
0000000000000000 0000000000000000
0000000000000000 0000000000000000
000000000000141d 0000000000000000
0000000000000000 0000000000000000
0000000000000000 0000000000000000
0000000000000000 0000000000000000
0000000000000000 0000000000000000
0000000000000004 0000000000000286
ffffffff81b01a40 0000000000000000
ffff88003d86be18 0000000000000063
0000000000000000 0000000000000001
ffffffff81645da0 0000000000000000
0000000000000010 0000000000000000
0000000000000000 0000000000000000
0000000000000063 ffffffffffffffff
ffffffff8134b6c6 0000000000000010
0000000000010096 ffff88003d86be18
0000000000000018 00007fc165c0b700
0000000000000000 0000000000000000
0000000000000000 0000000000000000
0000000000000000 0000000000000000
:
:
The patches support elf64, elf32, qemu64, qemu32 core format.
The kdump format didn't show the note information originally when help -D.
The patches add it.
the original output:
:
:
sub_header_kdump: 222aff0
phys_base: 0
dump_level: 0 (0x0)
split: 0
start_pfn: (unused)
end_pfn: (unused)
offset_vmcoreinfo: 4556 (0x11cc)
size_vmcoreinfo: 1440 (0x5a0)
OSRELEASE=2.6.32-431.el6.x86_64
PAGESIZE=4096
SYMBOL(init_uts_ns)=ffffffff81a8e940
SYMBOL(node_online_map)=ffffffff81c08f40
:
:
LENGTH(node_data)=512
CRASHTIME=1417488164
offset_note: 4176 (0x1050)
size_note: 1820 (0x71c)
num_prstatus_notes: 1
notes_buf: 224c020
notes[0]: 224c020
NT_PRSTATUS_offset: 1050
offset_eraseinfo: 0 (0x0)
size_eraseinfo: 0 (0x0)
:
:
the changed output:
:
:
sub_header_kdump: 2c75ff0
phys_base: 0
dump_level: 0 (0x0)
split: 0
start_pfn: (unused)
end_pfn: (unused)
offset_vmcoreinfo: 4556 (0x11cc)
size_vmcoreinfo: 1440 (0x5a0)
OSRELEASE=2.6.32-431.el6.x86_64
PAGESIZE=4096
SYMBOL(init_uts_ns)=ffffffff81a8e940
SYMBOL(node_online_map)=ffffffff81c08f40
:
:
LENGTH(node_data)=512
CRASHTIME=1417488164
offset_note: 4176 (0x1050)
size_note: 1820 (0x71c)
num_prstatus_notes: 1
notes_buf: 2c97020
notes[0]: 2c97020
si.signo: 0, si.code: 0, si.errno: 0, cursig:0
sigpend: 0
sighold: 0
pid: 5149, ppid: 0, pgrp: 0, sid:0
utime: 0.000000, stime: 0.000000, cutime: 0.000000, cstime: 0.000000
ORIG_RAX: -1, fpvalid: 0
R15 : 0x0000000000000004 R14 : 0x0000000000000286
R13 : 0xffffffff81b01a40 R12 : 0x0000000000000000
RBP : 0xffff88003d86be18 RBX : 0x0000000000000063
R11 : 0x0000000000000000 R10 : 0x0000000000000001
R9 : 0xffffffff81645da0 R8 : 0x0000000000000000
RAX : 0x0000000000000010 RCX : 0x0000000000000000
RDX : 0x0000000000000000 RSI : 0x0000000000000000
RDI : 0x0000000000000063 RIP : 0xffffffff8134b6c6
RFLAGS : 0x0000000000010096 RSP : 0xffff88003d86be18
FS_BASE: 0x00007fc165c0b700 GS_BASE : 0x0000000000000000
CS: 0x0010 SS: 0x0018 DS: 0x0000 ES: 0x0000 FS: 0x0000 GS: 0x0000
NT_PRSTATUS_offset: 1050
offset_eraseinfo: 0 (0x0)
size_eraseinfo: 0 (0x0)
:
:
--
Thanks
Zhou Wenjian
6:27 a.m.
----- Original Message -----
Hello all,
When analysing elf format core, if "help -D" is specified, crash will
show dumpfile contents/statistics. But the output is hardly to read.
So the patches is to make it human readable.
When analysing kdump format core, if "help -D" is specified, crash won't
show the note information. The patches add note information to the output.
Hello Zhou,
I haven't had the chance to fully test this patch, but I do
have a few prelinaries to get out of the way.
First, please address all of these warnings:
$ make warn
... [ cut ] ...
cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_7_6 netdump.c -Wall -O2 -Wstrict-prototypes
-Wmissing-prototypes -fstack-protector -Wformat-security
netdump.c: In function 'dump_Elf32_Nhdr':
netdump.c:1987:4: warning: format not a string literal and no format arguments
[-Wformat-security]
netdump.c: In function 'dump_Elf64_Nhdr':
netdump.c:2272:4: warning: format not a string literal and no format arguments
[-Wformat-security]
netdump.c:2303:4: warning: format not a string literal and no format arguments
[-Wformat-security]
netdump.c: At top level:
netdump.c:2593:1: warning: no previous prototype for 'display_prstatus_elf64'
[-Wmissing-prototypes]
netdump.c: In function 'display_prstatus_elf64':
netdump.c:2643:3: warning: format '%d' expects argument of type 'int', but
argument 7 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%d' expects argument of type 'int', but
argument 8 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%d' expects argument of type 'int', but
argument 13 has type '__time_t' [-Wformat]
netdump.c:2643:3: warning: format '%d' expects argument of type 'int', but
argument 14 has type '__suseconds_t' [-Wformat]
netdump.c:2643:3: warning: format '%d' expects argument of type 'int', but
argument 15 has type '__time_t' [-Wformat]
netdump.c:2643:3: warning: format '%d' expects argument of type 'int', but
argument 16 has type '__suseconds_t' [-Wformat]
netdump.c:2643:3: warning: format '%d' expects argument of type 'int', but
argument 17 has type '__time_t' [-Wformat]
netdump.c:2643:3: warning: format '%d' expects argument of type 'int', but
argument 18 has type '__suseconds_t' [-Wformat]
netdump.c:2643:3: warning: format '%d' expects argument of type 'int', but
argument 19 has type '__time_t' [-Wformat]
netdump.c:2643:3: warning: format '%d' expects argument of type 'int', but
argument 20 has type '__suseconds_t' [-Wformat]
netdump.c:2643:3: warning: format '%d' expects argument of type 'int', but
argument 21 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 23 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 24 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 25 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 26 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 27 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 28 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 29 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 30 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 31 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 32 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 33 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 34 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 35 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 36 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 37 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 38 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 39 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 40 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 41 has type 'long unsigned int' [-Wformat]
netdump.c:2643:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 42 has type 'long unsigned int' [-Wformat]
netdump.c: At top level:
netdump.c:2673:1: warning: no previous prototype for 'display_prstatus_elf32'
[-Wmissing-prototypes]
netdump.c: In function 'display_prstatus_elf32':
netdump.c:2713:3: warning: format '%d' expects argument of type 'int', but
argument 7 has type 'long unsigned int' [-Wformat]
netdump.c:2713:3: warning: format '%d' expects argument of type 'int', but
argument 8 has type 'long unsigned int' [-Wformat]
netdump.c:2713:3: warning: format '%d' expects argument of type 'int', but
argument 13 has type '__time_t' [-Wformat]
netdump.c:2713:3: warning: format '%d' expects argument of type 'int', but
argument 14 has type '__suseconds_t' [-Wformat]
netdump.c:2713:3: warning: format '%d' expects argument of type 'int', but
argument 15 has type '__time_t' [-Wformat]
netdump.c:2713:3: warning: format '%d' expects argument of type 'int', but
argument 16 has type '__suseconds_t' [-Wformat]
netdump.c:2713:3: warning: format '%d' expects argument of type 'int', but
argument 17 has type '__time_t' [-Wformat]
netdump.c:2713:3: warning: format '%d' expects argument of type 'int', but
argument 18 has type '__suseconds_t' [-Wformat]
netdump.c:2713:3: warning: format '%d' expects argument of type 'int', but
argument 19 has type '__time_t' [-Wformat]
netdump.c:2713:3: warning: format '%d' expects argument of type 'int', but
argument 20 has type '__suseconds_t' [-Wformat]
netdump.c:2713:3: warning: format '%d' expects argument of type 'int', but
argument 21 has type 'long unsigned int' [-Wformat]
netdump.c: At top level:
netdump.c:2717:1: warning: no previous prototype for 'display_qemu_elf'
[-Wmissing-prototypes]
netdump.c: In function 'display_qemu_elf':
netdump.c:2737:4: warning: format '%lx' expects argument of type 'long
unsigned int', but argument 3 has type 'uint32_t' [-Wformat]
netdump.c:2737:4: warning: format '%lx' expects argument of type 'long
unsigned int', but argument 4 has type 'uint32_t' [-Wformat]
netdump.c:2737:4: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 5 has type 'uint64_t' [-Wformat]
netdump.c:2737:4: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 6 has type 'uint64_t' [-Wformat]
netdump.c:2737:4: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 7 has type 'uint64_t' [-Wformat]
netdump.c:2737:4: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 8 has type 'uint64_t' [-Wformat]
netdump.c:2737:4: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 9 has type 'uint64_t' [-Wformat]
netdump.c:2737:4: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 10 has type 'uint64_t' [-Wformat]
netdump.c:2737:4: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 11 has type 'uint64_t' [-Wformat]
netdump.c:2737:4: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 12 has type 'uint64_t' [-Wformat]
netdump.c:2749:5: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 3 has type 'uint64_t' [-Wformat]
netdump.c:2749:5: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 4 has type 'uint64_t' [-Wformat]
netdump.c:2749:5: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 5 has type 'uint64_t' [-Wformat]
netdump.c:2749:5: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 6 has type 'uint64_t' [-Wformat]
netdump.c:2749:5: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 7 has type 'uint64_t' [-Wformat]
netdump.c:2749:5: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 8 has type 'uint64_t' [-Wformat]
netdump.c:2749:5: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 9 has type 'uint64_t' [-Wformat]
netdump.c:2749:5: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 10 has type 'uint64_t' [-Wformat]
netdump.c:2761:5: warning: format '%lx' expects argument of type 'long
unsigned int', but argument 4 has type 'uint32_t' [-Wformat]
netdump.c:2761:5: warning: format '%lx' expects argument of type 'long
unsigned int', but argument 5 has type 'uint32_t' [-Wformat]
netdump.c:2761:5: warning: format '%lx' expects argument of type 'long
unsigned int', but argument 6 has type 'uint32_t' [-Wformat]
netdump.c:2761:5: warning: format '%lx' expects argument of type 'long
unsigned int', but argument 7 has type 'uint32_t' [-Wformat]
netdump.c:2761:5: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 8 has type 'uint64_t' [-Wformat]
netdump.c:2771:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 3 has type 'uint64_t' [-Wformat]
netdump.c:2771:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 4 has type 'uint64_t' [-Wformat]
netdump.c:2771:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 5 has type 'uint64_t' [-Wformat]
netdump.c:2771:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 6 has type 'uint64_t' [-Wformat]
netdump.c:2771:3: warning: format '%llx' expects argument of type 'long long
unsigned int', but argument 7 has type 'uint64_t' [-Wformat]
netdump.c: At top level:
netdump.c:2775:1: warning: no previous prototype for 'display_qemu_elf64'
[-Wmissing-prototypes]
netdump.c:2788:1: warning: no previous prototype for 'display_qemu_elf32'
[-Wmissing-prototypes]
netdump.c:2801:1: warning: no previous prototype for 'display_note'
[-Wmissing-prototypes]
cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_7_6 diskdump.c -Wall -O2 -Wstrict-prototypes
-Wmissing-prototypes -fstack-protector -Wformat-security
diskdump.c: In function '__diskdump_memory_dump':
diskdump.c:1749:41: warning: implicit declaration of function 'display_note_elf32'
[-Wimplicit-function-declaration]
diskdump.c:1751:41: warning: implicit declaration of function 'display_note_elf64'
[-Wimplicit-function-declaration]
diskdump.c:1752:33: warning: format not a string literal and no format arguments
[-Wformat-security]
...
Secondly, for compressed kdumps in diskdump.c, you have this construct:
if (dd->machine_type == EM_386)
display_note_elf32(dd->nt_prstatus_percpu[i],l_buf);
else if (dd->machine_type == EM_X86_64)
display_note_elf64(dd->nt_prstatus_percpu[i],l_buf);
But for ELF kdumps in netdump.c, display_note_elf32() are display_note_elf64()
look to be called unconditionally. What about the other architectures?
Also, are there any dumping facilities that actually utilize any of
the nt_prstatus fields besides the registers and maybe the pid?:
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;
};
For example, kdump just does this:
memset(&prstatus, 0, sizeof(prstatus));
prstatus.pr_pid = current->pid;
elf_core_copy_kernel_regs(&prstatus.pr_reg, regs);
So I guess I wonder what is the point of displaying of all the other fields?
Also, can you explain to me the difference between the register contents
in the QEMUCPUState structure vs. the register contents in the NT_PRSTATUS
note?
And related to that question, why doesn't your dump_qemu_elf() function
display the rip and rflags registers?
Dave
2:56 p.m.
On 12/11/2014 06:27 AM, Dave Anderson wrote:
First, please address all of these warnings:
$ make warn
... [ cut ] ...
cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_7_6 netdump.c -Wall -O2 -Wstrict-prototypes
-Wmissing-prototypes -fstack-protector -Wformat-security
netdump.c: In function 'dump_Elf32_Nhdr':
netdump.c:1987:4: warning: format not a string literal and no format arguments
[-Wformat-security]
netdump.c: In function 'dump_Elf64_Nhdr':
netdump.c:2272:4: warning: format not a string literal and no format arguments
[-Wformat-security]
netdump.c:2303:4: warning: format not a string literal and no format arguments
[-Wformat-security]
...
Hello Dave,
These warnings have been addressed.
Secondly, for compressed kdumps in diskdump.c, you have this
construct:
if (dd->machine_type == EM_386)
display_note_elf32(dd->nt_prstatus_percpu[i],l_buf);
else if (dd->machine_type == EM_X86_64)
display_note_elf64(dd->nt_prstatus_percpu[i],l_buf);
But for ELF kdumps in netdump.c, display_note_elf32() are display_note_elf64()
look to be called unconditionally. What about the other architectures?
I distinguish the architectures by the struct size in display_note(). I wonder
if it is needed to add other conditions like machine_type.
display_note(void *note_ptr, char *buf, int descsz)
{
if (descsz == (2 * sizeof(struct x86_64_prstatus)))
display_prstatus_elf64(note_ptr, buf);
else if (descsz == sizeof(struct x86_prstatus))
display_prstatus_elf32(note_ptr, buf);
else if (descsz == (2 * sizeof(QEMUCPUState)))
display_qemu_elf64(note_ptr, buf);
else if (descsz == sizeof(QEMUCPUState))
display_qemu_elf32(note_ptr, buf);
}
Also, are there any dumping facilities that actually utilize any of
the nt_prstatus fields besides the registers and maybe the pid?:
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;
};
For example, kdump just does this:
memset(&prstatus, 0, sizeof(prstatus));
prstatus.pr_pid = current->pid;
elf_core_copy_kernel_regs(&prstatus.pr_reg, regs);
So I guess I wonder what is the point of displaying of all the other fields?
These patches are just used to make the original information readable.
I'm not sure whether it is a good idea to display only the registers contents here,
for help -r can show them well.
Also, can you explain to me the difference between the register
contents
in the QEMUCPUState structure vs. the register contents in the NT_PRSTATUS
note?
They are almost the same except that there are more information in QEMUCPUState.
And related to that question, why doesn't your dump_qemu_elf()
function
display the rip and rflags registers?
It's my fault that I left them. And they have been added.
Dave
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
--
Thanks
Zhou Wenjian
1:06 a.m.
----- Original Message -----
On 12/11/2014 06:27 AM, Dave Anderson wrote:
> First, please address all of these warnings:
>
> $ make warn
> ... [ cut ] ...
> cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_7_6 netdump.c -Wall -O2
> -Wstrict-prototypes -Wmissing-prototypes -fstack-protector
> -Wformat-security
> netdump.c: In function 'dump_Elf32_Nhdr':
> netdump.c:1987:4: warning: format not a string literal and no format
> arguments [-Wformat-security]
> netdump.c: In function 'dump_Elf64_Nhdr':
> netdump.c:2272:4: warning: format not a string literal and no format
> arguments [-Wformat-security]
> netdump.c:2303:4: warning: format not a string literal and no format
> arguments [-Wformat-security]
> ...
>
Hello Dave,
These warnings have been addressed.
> Secondly, for compressed kdumps in diskdump.c, you have this construct:
>
> if (dd->machine_type == EM_386)
> display_note_elf32(dd->nt_prstatus_percpu[i],l_buf);
> else if (dd->machine_type == EM_X86_64)
> display_note_elf64(dd->nt_prstatus_percpu[i],l_buf);
>
> But for ELF kdumps in netdump.c, display_note_elf32() are display_note_elf64()
> look to be called unconditionally. What about the other architectures?
>
I distinguish the architectures by the struct size in display_note(). I wonder
if it is needed to add other conditions like machine_type.
display_note(void *note_ptr, char *buf, int descsz)
{
if (descsz == (2 * sizeof(struct x86_64_prstatus)))
display_prstatus_elf64(note_ptr, buf);
else if (descsz == sizeof(struct x86_prstatus))
display_prstatus_elf32(note_ptr, buf);
else if (descsz == (2 * sizeof(QEMUCPUState)))
display_qemu_elf64(note_ptr, buf);
else if (descsz == sizeof(QEMUCPUState))
display_qemu_elf32(note_ptr, buf);
}
Yes, it should definitely take the machine type into account, given
that it's not guaranteed to work with any other architectures without
knowing exactly what the size of their notes would be.
In fact this whole patchset needs to be made more machine-type specific.
The myriad of new functions you've added to netdump.c are generally confusing
to me because several of their names imply that they are generic to ELF,
and not specific to x86 and x86_64. Yes, the data being displayed is contained
within an ELF note, but more importantly, the data itself is machine-specific.
So can we please simplify/clarify things a bit?
Here's what I suggest. Given the fact that display_prstatus_el64()
and display_prstatus_el32 are coming from two different dumpfile locations
(and perhaps more in the future?), why not add machine type and note type
arguments? To reduce the number of functions, create a general purpose
"display_ELF_note()" function that can distribute the work appropriately?
So diskdump.c can be simplified from:
if (dd->machine_type == EM_386)
display_note_elf32(dd->nt_prstatus_percpu[i],l_buf);
else if (dd->machine_type == EM_X86_64)
display_note_elf64(dd->nt_prstatus_percpu[i],l_buf);
to just:
display_ELF_note(dd->machine_type, PRSTATUS, dd->nt_prstatus_percpu[i],
l_buf);
And the 3 netdump.c invocations would be changed from:
display_note_elf32(note, l_buf);
display_note_elf32(note, l_buf);
display_note_el64((note, l_buf);
to:
display_ELF_note(EM_386, qemuinfo ? QEMU : PRSTATUS, note, l_buf);
display_ELF_note(EM_386, PRSTATUS, note, l_buf);
display_ELF_note(EM_X86_64, qemuinfo ? QEMU : PRSTATUS, note, l_buf);
Then the general-purpose display_ELF_note() function can directly call
the appropriate function. And those architecture-specific functions
should be should be renamed to append "_x86" or "_x86_64" (as you
have
done with "x86_prstatus" and "x86_64_prstatus" structures).
So the display_ELF_note() function could look something like:
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;
}
}
And in the future, functions can simply be added for other architectures.
Now, I understand that your display_qemu_elf32() and display_qemu_elf64()
functions call into your single display_qemu_elf() function because there
is common code. That could be handled in few different manners.
You could consolidate my suggested display_qemu_x86() and display_qemu_x86_64()
functions into a "common" function that is named display_qemu_x86_32_64(), or
display_qemu_intel(), or whatever (?).
Alternatively, you could have display_ELF_note() call an architecture-neutral
display_qemu() function, which can set up things based upon the machine-type,
and then call the architecture-specific function -- which in this case would
be the common function used by both x86 and x86_64.
Or your display_qemu_elf() function could be broken into two separate
functions, which would contain some common/duplicate code. That's also
fine with me.
However you do it, all I ask is that if a function only applies to a specific
architecture, then please follow the structure-naming convention so that there
is no question.
Thanks,
Dave
12:02 p.m.
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.
On 12/12/2014 01:06 AM, Dave Anderson wrote:
----- Original Message -----
> On 12/11/2014 06:27 AM, Dave Anderson wrote:
>
>> First, please address all of these warnings:
>>
>> $ make warn
>> ... [ cut ] ...
>> cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_7_6 netdump.c -Wall -O2
>> -Wstrict-prototypes -Wmissing-prototypes -fstack-protector
>> -Wformat-security
>> netdump.c: In function 'dump_Elf32_Nhdr':
>> netdump.c:1987:4: warning: format not a string literal and no format
>> arguments [-Wformat-security]
>> netdump.c: In function 'dump_Elf64_Nhdr':
>> netdump.c:2272:4: warning: format not a string literal and no format
>> arguments [-Wformat-security]
>> netdump.c:2303:4: warning: format not a string literal and no format
>> arguments [-Wformat-security]
>> ...
>>
>
> Hello Dave,
>
> These warnings have been addressed.
>
>> Secondly, for compressed kdumps in diskdump.c, you have this construct:
>>
>> if (dd->machine_type == EM_386)
>> display_note_elf32(dd->nt_prstatus_percpu[i],l_buf);
>> else if (dd->machine_type == EM_X86_64)
>> display_note_elf64(dd->nt_prstatus_percpu[i],l_buf);
>>
>> But for ELF kdumps in netdump.c, display_note_elf32() are display_note_elf64()
>> look to be called unconditionally. What about the other architectures?
>>
>
> I distinguish the architectures by the struct size in display_note(). I wonder
> if it is needed to add other conditions like machine_type.
>
>
> display_note(void *note_ptr, char *buf, int descsz)
> {
> if (descsz == (2 * sizeof(struct x86_64_prstatus)))
> display_prstatus_elf64(note_ptr, buf);
> else if (descsz == sizeof(struct x86_prstatus))
> display_prstatus_elf32(note_ptr, buf);
> else if (descsz == (2 * sizeof(QEMUCPUState)))
> display_qemu_elf64(note_ptr, buf);
> else if (descsz == sizeof(QEMUCPUState))
> display_qemu_elf32(note_ptr, buf);
> }
Yes, it should definitely take the machine type into account, given
that it's not guaranteed to work with any other architectures without
knowing exactly what the size of their notes would be.
In fact this whole patchset needs to be made more machine-type specific.
The myriad of new functions you've added to netdump.c are generally confusing
to me because several of their names imply that they are generic to ELF,
and not specific to x86 and x86_64. Yes, the data being displayed is contained
within an ELF note, but more importantly, the data itself is machine-specific.
So can we please simplify/clarify things a bit?
Here's what I suggest. Given the fact that display_prstatus_el64()
and display_prstatus_el32 are coming from two different dumpfile locations
(and perhaps more in the future?), why not add machine type and note type
arguments? To reduce the number of functions, create a general purpose
"display_ELF_note()" function that can distribute the work appropriately?
So diskdump.c can be simplified from:
if (dd->machine_type == EM_386)
display_note_elf32(dd->nt_prstatus_percpu[i],l_buf);
else if (dd->machine_type == EM_X86_64)
display_note_elf64(dd->nt_prstatus_percpu[i],l_buf);
to just:
display_ELF_note(dd->machine_type, PRSTATUS, dd->nt_prstatus_percpu[i],
l_buf);
And the 3 netdump.c invocations would be changed from:
display_note_elf32(note, l_buf);
display_note_elf32(note, l_buf);
display_note_el64((note, l_buf);
to:
display_ELF_note(EM_386, qemuinfo ? QEMU : PRSTATUS, note, l_buf);
display_ELF_note(EM_386, PRSTATUS, note, l_buf);
display_ELF_note(EM_X86_64, qemuinfo ? QEMU : PRSTATUS, note, l_buf);
Then the general-purpose display_ELF_note() function can directly call
the appropriate function. And those architecture-specific functions
should be should be renamed to append "_x86" or "_x86_64" (as you
have
done with "x86_prstatus" and "x86_64_prstatus" structures).
So the display_ELF_note() function could look something like:
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;
}
}
And in the future, functions can simply be added for other architectures.
Now, I understand that your display_qemu_elf32() and display_qemu_elf64()
functions call into your single display_qemu_elf() function because there
is common code. That could be handled in few different manners.
You could consolidate my suggested display_qemu_x86() and display_qemu_x86_64()
functions into a "common" function that is named display_qemu_x86_32_64(), or
display_qemu_intel(), or whatever (?).
Alternatively, you could have display_ELF_note() call an architecture-neutral
display_qemu() function, which can set up things based upon the machine-type,
and then call the architecture-specific function -- which in this case would
be the common function used by both x86 and x86_64.
Or your display_qemu_elf() function could be broken into two separate
functions, which would contain some common/duplicate code. That's also
fine with me.
However you do it, all I ask is that if a function only applies to a specific
architecture, then please follow the structure-naming convention so that there
is no question.
Thanks,
Dave
--
Thanks
Zhou Wenjian
5:50 a.m.
----- 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?
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
5:58 p.m.
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
5:40 a.m.
----- Original Message -----
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().
Hello Zhou,
Looking at this further, the usage of the static char buffers is
just plain ugly, so let's just get rid them entirely, and simplify
things even further.
I did a bit of tinkering with your patch-set:
(1) Instead of passing a char * to display_ELF_note(), the last argument
has been changed to be the relevant FILE * for printing. The 4 display
functions then simply fprintf to that FILE * instead of into a char buffer.
(2) I changed the 4 display functions to use the space(count) function,
and vary the count argument based upon where the call is coming
from. That way, the note data output "lines up" nicely with either
the ELF or compressed kdump displays that they are part of.
(3) And I tinkered with the output formats so as to more closely
be aligned with typical crash output, such as removing the "0x"
preceding hexadecimal values, etc...
Can you verify the attached "help-D.patch"? I don't have any 32-bit QEMU
dumps
to test it with. It was created against the current git tree.
Thanks,
Dave
10:13 a.m.
I test it, and it works well.
On 12/17/2014 05:40 AM, Dave Anderson wrote:
----- Original Message -----
> 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().
Hello Zhou,
Looking at this further, the usage of the static char buffers is
just plain ugly, so let's just get rid them entirely, and simplify
things even further.
I did a bit of tinkering with your patch-set:
(1) Instead of passing a char * to display_ELF_note(), the last argument
has been changed to be the relevant FILE * for printing. The 4 display
functions then simply fprintf to that FILE * instead of into a char buffer.
(2) I changed the 4 display functions to use the space(count) function,
and vary the count argument based upon where the call is coming
from. That way, the note data output "lines up" nicely with either
the ELF or compressed kdump displays that they are part of.
(3) And I tinkered with the output formats so as to more closely
be aligned with typical crash output, such as removing the "0x"
preceding hexadecimal values, etc...
Can you verify the attached "help-D.patch"? I don't have any 32-bit QEMU
dumps
to test it with. It was created against the current git tree.
Thanks,
Dave
--
Thanks
Zhou Wenjian
5:07 a.m.
----- Original Message -----
I test it, and it works well.
OK thanks -- although further testing showed that the patch needed a bit more work
in netdump.c to prevent calling display_ELF_note() if:
(1) the architecture is not x86 or x86-64
(2) the note type is not NT_PRSTATUS or QEMU, because it would also erroneously pass
pointers to other note types such as NT_PRPSINFO and NT_TASKSTRUCT.
Queued for crash-7.1.0:
https://github.com/crash-utility/crash/commit/e36a1eb28b30c12deb72ffc5157...
Thanks,
Dave
On 12/17/2014 05:40 AM, Dave Anderson wrote:
>
>
> ----- Original Message -----
>
>> 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().
>
> Hello Zhou,
>
> Looking at this further, the usage of the static char buffers is
> just plain ugly, so let's just get rid them entirely, and simplify
> things even further.
>
> I did a bit of tinkering with your patch-set:
>
> (1) Instead of passing a char * to display_ELF_note(), the last argument
> has been changed to be the relevant FILE * for printing. The 4
> display
> functions then simply fprintf to that FILE * instead of into a char
> buffer.
>
> (2) I changed the 4 display functions to use the space(count) function,
> and vary the count argument based upon where the call is coming
> from. That way, the note data output "lines up" nicely with either
> the ELF or compressed kdump displays that they are part of.
>
> (3) And I tinkered with the output formats so as to more closely
> be aligned with typical crash output, such as removing the "0x"
> preceding hexadecimal values, etc...
>
> Can you verify the attached "help-D.patch"? I don't have any 32-bit
QEMU
> dumps
> to test it with. It was created against the current git tree.
>
> Thanks,
> Dave
>
--
Thanks
Zhou Wenjian
11:59 a.m.
On 12/18/2014 05:07 AM, Dave Anderson wrote:
OK thanks -- although further testing showed that the patch needed a
bit more work
in netdump.c to prevent calling display_ELF_note() if:
(1) the architecture is not x86 or x86-64
(2) the note type is not NT_PRSTATUS or QEMU, because it would also erroneously pass
pointers to other note types such as NT_PRPSINFO and NT_TASKSTRUCT.
Queued for crash-7.1.0:
https://github.com/crash-utility/crash/commit/e36a1eb28b30c12deb72ffc5157...
Hello Dave,
I found something is missing in the committed patch. In dump_Elf64_Nhdr, qemu note of X86
is not available to call display_ELF_note. The process is like below:
if (BITS32() && type == NT_PRSTATUS)
display_ELF_note
else
if (machine_type("X86_64"))
display_ELF_note
And about the fix, please check the attachment.
--
Regards
Qiao Nuohan
12:47 a.m.
On 12/18/2014 05:07 AM, Dave Anderson wrote:
> OK thanks -- although further testing showed that the patch needed a bit
> more work
> in netdump.c to prevent calling display_ELF_note() if:
>
> (1) the architecture is not x86 or x86-64
> (2) the note type is not NT_PRSTATUS or QEMU, because it would also
> erroneously pass
> pointers to other note types such as NT_PRPSINFO and NT_TASKSTRUCT.
>
> Queued for crash-7.1.0:
>
>
https://github.com/crash-utility/crash/commit/e36a1eb28b30c12deb72ffc5157...
Hello Dave,
I found something is missing in the committed patch. In dump_Elf64_Nhdr,
qemu note of X86
is not available to call display_ELF_note. The process is like below:
if (BITS32() && type == NT_PRSTATUS)
display_ELF_note
else
if (machine_type("X86_64"))
display_ELF_note
And about the fix, please check the attachment.
--
Regards
Qiao Nuohan
Hi Qiao,
OK, thanks, I wasn't even able to test that scenario.
Anyway, I will be out of the office until Monday January 5th, and
won't be doing any crash maintenance until then.
Thanks,
Dave
3:09 a.m.
----- Original Message -----
Hello Dave,
I found something is missing in the committed patch. In dump_Elf64_Nhdr, qemu note of
X86
is not available to call display_ELF_note. The process is like below:
if (BITS32() && type == NT_PRSTATUS)
display_ELF_note
else
if (machine_type("X86_64"))
display_ELF_note
And about the fix, please check the attachment.
--
Regards
Qiao Nuohan
Thanks Qiao, queued for crash-7.1.0:
https://github.com/crash-utility/crash/commit/6dd0c32cbeebdf4e095410479da...
Dave
3574
days inactive
3600
days old
devel@lists.crash-utility.osci.io
12 comments
4 participants
participants (4)
-
"Zhou, Wenjian/周文剑" -
Dave Anderson -
Dave Anderson -
qiaonuohan