----- "Per Fransson" <per.xx.fransson@xxxxxxxxxxxxxx> wrote:
> Hi Dave and Mika,
>
> Thanks for your input. Here's attempt number two. I have:
>
> - eliminated the leaks
> - removed 'crash_task_pid'
> - fixed the formatting
> - not used gmail, since it corrupts the patch
> - used malloc/free for panic_task_regs
Thanks Per, I'll give it a test run.
No need to repost, but in the future can you also compile it
with "make warn" to clean it up a bit?
# make warn
...
cc -c -g -DARM -m32 -D_FILE_OFFSET_BITS=64 arm.c -Wall -O2 -Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wp,-D_FORTIFY_SOURCE=2
arm.c: In function âarm_get_crash_notesâ:
arm.c:496: warning: assignment from incompatible pointer type
arm.c:515: warning: assignment from incompatible pointer type
arm.c:484: warning: unused variable âptrâ
...
Thanks,
Dave
>
> Regards,
> Per
>
>
> diff --git a/arm.c b/arm.c
> index 06b2f1c..b3841c0 100644
> --- a/arm.c
> +++ b/arm.c
> @@ -73,7 +73,7 @@ struct arm_cpu_context_save {
> /*
> * Holds registers during the crash.
> */
> -static struct arm_pt_regs panic_task_regs;
> +static struct arm_pt_regs *panic_task_regs;
>
> #define PGDIR_SIZE() (4 * PAGESIZE())
> #define PGDIR_OFFSET(X) (((ulong)(X)) & (PGDIR_SIZE() - 1))
> @@ -392,7 +392,6 @@ arm_dump_machdep_table(ulong arg)
> fprintf(fp, " kernel_text_end: %lx\n", ms->kernel_text_end);
> fprintf(fp, "exception_text_start: %lx\n",
> ms->exception_text_start);
> fprintf(fp, " exception_text_end: %lx\n", ms->exception_text_end);
> - fprintf(fp, " crash_task_pid: %ld\n", ms->crash_task_pid);
> fprintf(fp, " crash_task_regs: %lx\n",
> (ulong)ms->crash_task_regs);
> }
>
> @@ -484,71 +483,104 @@ arm_get_crash_notes(void)
> Elf32_Nhdr *note;
> ulong ptr, offset;
> char *buf, *p;
> + ulong *notes_ptrs;
> + ulong per_cpu_offsets_addr;
> + ulong *per_cpu_offsets;
> + ulong i;
>
> if (!symbol_exists("crash_notes"))
> return FALSE;
>
> crash_notes = symbol_value("crash_notes");
>
> - if (kt->cpus > 1)
> - error(WARNING, "only one CPU is currently supported\n");
> + notes_ptrs = GETBUF(kt->cpus*sizeof(notes_ptrs[0]));
>
> /*
> * Read crash_notes for the first CPU. crash_notes are in standard
> ELF
> * note format.
> */
> - if (!readmem(crash_notes, KVADDR, &ptr, sizeof(ptr), "crash_notes",
> + if (!readmem(crash_notes, KVADDR, ¬es_ptrs[kt->cpus-1],
> sizeof(notes_ptrs[kt->cpus-1]), "crash_notes",
> RETURN_ON_ERROR)) {
> error(WARNING, "cannot read crash_notes\n");
> + FREEBUF(notes_ptrs);
> return FALSE;
> }
> +
> +
> + if (symbol_exists("__per_cpu_offset")) {
> +
> + /* Get the __per_cpu_offset array */
> + per_cpu_offsets_addr = symbol_value("__per_cpu_offset");
> +
> + per_cpu_offsets = GETBUF(kt->cpus*sizeof(*per_cpu_offsets));
> +
> + if (!readmem(per_cpu_offsets_addr, KVADDR, per_cpu_offsets,
> kt->cpus*sizeof(*per_cpu_offsets), "per_cpu_offsets",
> + RETURN_ON_ERROR)) {
> + error(WARNING, "cannot read per_cpu_offsets\n");
> + FREEBUF(per_cpu_offsets);
> + return FALSE;
> + }
> +
> + /* Add __per_cpu_offset for each cpu to form the pointer to the
> notes */
> + for (i = 0; i<kt->cpus; i++) {
> + notes_ptrs[i] = notes_ptrs[kt->cpus-1] + per_cpu_offsets[i];
> + }
> + FREEBUF(per_cpu_offsets);
> + }
>
> buf = GETBUF(SIZE(note_buf));
> + panic_task_regs = malloc(kt->cpus*sizeof(*panic_task_regs));
> +
> + for (i=0;i<kt->cpus;i++) {
> +
> + if (!readmem(notes_ptrs[i], KVADDR, buf, SIZE(note_buf),
> "note_buf_t",
> + RETURN_ON_ERROR)) {
> + error(WARNING, "failed to read note_buf_t\n");
> + goto fail;
> + }
>
> - if (!readmem(ptr, KVADDR, buf, SIZE(note_buf), "note_buf_t",
> - RETURN_ON_ERROR)) {
> - error(WARNING, "failed to read note_buf_t\n");
> - goto fail;
> - }
> + /*
> + * Do some sanity checks for this note before reading registers
> from it.
> + */
> + note = (Elf32_Nhdr *)buf;
> + p = buf + sizeof(Elf32_Nhdr);
>
> - /*
> - * Do some sanity checks for this note before reading registers from
> it.
> - */
> - note = (Elf32_Nhdr *)buf;
> - p = buf + sizeof(Elf32_Nhdr);
> + if (note->n_type != NT_PRSTATUS) {
> + error(WARNING, "invalid note (n_type != NT_PRSTATUS)\n");
> + goto fail;
> + }
> + if (p[0] != 'C' || p[1] != 'O' || p[2] != 'R' || p[3] != 'E') {
> + error(WARNING, "invalid note (name != \"CORE\"\n");
> + goto fail;
> + }
>
> - if (note->n_type != NT_PRSTATUS) {
> - error(WARNING, "invalid note (n_type != NT_PRSTATUS)\n");
> - goto fail;
> - }
> - if (p[0] != 'C' || p[1] != 'O' || p[2] != 'R' || p[3] != 'E') {
> - error(WARNING, "invalid note (name != \"CORE\"\n");
> - goto fail;
> - }
> + /*
> + * Find correct location of note data. This contains elf_prstatus
> + * structure which has registers etc. for the crashed task.
> + */
> + offset = sizeof(Elf32_Nhdr);
> + offset = roundup(offset + note->n_namesz, 4);
> + p = buf + offset; /* start of elf_prstatus */
>
> - /*
> - * Find correct location of note data. This contains elf_prstatus
> - * structure which has registers etc. for the crashed task.
> - */
> - offset = sizeof(Elf32_Nhdr);
> - offset = roundup(offset + note->n_namesz, 4);
> - p = buf + offset; /* start of elf_prstatus */
> + BCOPY(p + OFFSET(elf_prstatus_pr_reg), &panic_task_regs[i],
> + sizeof(panic_task_regs[i]));
>
> - BCOPY(p + OFFSET(elf_prstatus_pr_reg), &panic_task_regs,
> - sizeof(panic_task_regs));
> + }
>
> /*
> - * And finally we have pid and registers for the crashed task. This
> is
> + * And finally we have the registers for the crashed task. This is
> * used later on when dumping backtrace.
> */
> - ms->crash_task_pid = *(ulong *)(p + OFFSET(elf_prstatus_pr_pid));
> - ms->crash_task_regs = &panic_task_regs;
> + ms->crash_task_regs = panic_task_regs;
>
> FREEBUF(buf);
> + FREEBUF(notes_ptrs);
> return TRUE;
>
> fail:
> FREEBUF(buf);
> + FREEBUF(notes_ptrs);
> + free(panic_task_regs);
> return FALSE;
> }
>
> @@ -996,20 +1028,20 @@ arm_get_dumpfile_stack_frame(struct bt_info
> *bt, ulong *nip, ulong *ksp)
> if (!ms->crash_task_regs)
> return FALSE;
>
> - if (tt->panic_task != bt->task || bt->tc->pid !=
> ms->crash_task_pid)
> + if (!is_task_active(bt->task))
> return FALSE;
> -
> +
> /*
> * We got registers for panic task from crash_notes. Just return
> them.
> */
> - *nip = ms->crash_task_regs->ARM_pc;
> - *ksp = ms->crash_task_regs->ARM_sp;
> + *nip = ms->crash_task_regs[bt->tc->processor].ARM_pc;
> + *ksp = ms->crash_task_regs[bt->tc->processor].ARM_sp;
>
> /*
> * Also store pointer to all registers in case unwinding code needs
> * to access LR.
> */
> - bt->machdep = ms->crash_task_regs;
> + bt->machdep = &(ms->crash_task_regs[bt->tc->processor]);
>
> return TRUE;
> }
> diff --git a/defs.h b/defs.h
> index d431d6e..6e0c8cc 100755
> --- a/defs.h
> +++ b/defs.h
> @@ -85,7 +85,7 @@
> #define NR_CPUS (64)
> #endif
> #ifdef ARM
> -#define NR_CPUS (1)
> +#define NR_CPUS (4)
> #endif
>
> #define BUFSIZE (1500)
> @@ -4062,7 +4062,6 @@ struct machine_specific {
> ulong kernel_text_end;
> ulong exception_text_start;
> ulong exception_text_end;
> - ulong crash_task_pid;
> struct arm_pt_regs *crash_task_regs;
> };
>
>
> --
> Crash-utility mailing list
> Crash-utility@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/crash-utility
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
