* [PATCH 3/4] BFD: Fix reading Linux core PRSTATUS note for MIPS n32
@ 2017-10-06 11:03 Djordje Todorovic
2017-10-12 22:35 ` Maciej W. Rozycki
0 siblings, 1 reply; 3+ messages in thread
From: Djordje Todorovic @ 2017-10-06 11:03 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: binutils, gdb-patches, nemanja.popov, Nikola Prica,
petar.jovanovic, Ananthakrishna Sowda (asowda)
The kernel struct elf_prstatus which GDB MIPS n32 uses is defined as following:
(top-gdb-mipsN32) ptype struct elf_prstatus
type = struct elf_prstatus {
struct elf_siginfo pr_info;
short pr_cursig;
unsigned long long pr_sigpend;
unsigned long long pr_sighold;
__pid_t pr_pid;
__pid_t pr_ppid;
__pid_t pr_pgrp;
__pid_t pr_sid;
struct timeval pr_utime;
struct timeval pr_stime;
struct timeval pr_cutime;
struct timeval pr_cstime;
elf_gregset_t pr_reg;
int pr_fpvalid;
}
and the size of the structure is not right in the current source code, because:
(top-gdb-mipsN32) p sizeof(struct elf_prstatus)
$1 = 448
Also, offset of the pr_pid and pr_reg have to be corrected:
(top-gdb-mipsN32) print /d &((struct elf_prstatus *)0)->pr_reg
$2 = 80
(top-gdb-mipsN32) print /d &((struct elf_prstatus *)0)->pr_pid
$3 = 32
Also, it is detected that on MIPS n32 platform, GDB has never called functions for reading Linux core PRPSINFO and PRSTATUS note defined in bfd/elfn32-mips.c, but GDB MIPS n32
currently uses functions from bfd/elf32-mips.c. I am not sure if it is expected, but 'elf32_mips_grok_psinfo' from bfd/elfn32-mips.c is exactly the same as one from
bfd/elf32-mips.c, because GDB MIPS n32 uses exactly the same struct elf_prpsinfo and there is no problem for end users. But, when GDB MIPS n32 comes into 'elf32_mips_grok_prstatus'
from bfd/elf32-mips.c, it would never go into 'case 256' of the 'switch' because the size of struct elf_prstatus is different on MIPS n32.
So, I have also noticed when GDB MIPS n32 generates core file it calls proper functions for it (from bfd/elfn32-mips.c) because target vector points to the proper architecture:
(gdb) gcore
Breakpoint 1, elf32_mips_write_core_note (abfd=0x10b329e8, buf=0x10b32d88 "", bufsiz=0x7fff5fec,
note_type=1) at ../../binutils-gdb/bfd/elfn32-mips.c:3590
3590 switch (note_type)
(top-gdb-mipsN32) p abfd->xvec
$4 = (const struct bfd_target *) 0x10869010 <mips_elf32_ntrad_be_vec>
but when reads the core file it looks as following:
...
(top-gdb-mipsN32) c
Continuing.
A program is being debugged already. Kill it? (y or n) y
Breakpoint 2, elf32_mips_grok_prstatus (abfd=0x10ac9a58, note=0x7fff5d08)
at ../../binutils-gdb/bfd/elf32-mips.c:2323
2323 switch (note->descsz)
(top-gdb-mipsN32) p abfd->xvec
$5 = (const struct bfd_target *) 0x1085a318 <mips_elf32_trad_be_vec>
Even GDB MIPS n32 does not use the function by current design, at least on my MIPS board, the patch looks as following:
From 918226ecebb699916e7e3f3e0f5befa2602b8708 Mon Sep 17 00:00:00 2001
From: Djordje Todorovic <djordje.todorovic@rt-rk.com>
Date: Wed, 4 Oct 2017 15:01:00 +0200
Subject: [PATCH 3/4] BFD: Fix reading Linux core PRSTATUS note for MIPS n32
bfd/ChangeLog:
* bfd/elfn32-mips (elf32_mips_grok_prstatus): Fix pr_pid and
pr_reg offsets and size of struct elf_prstatus.
---
bfd/elfn32-mips.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/bfd/elfn32-mips.c b/bfd/elfn32-mips.c
index 5287da3..07793b6 100644
--- a/bfd/elfn32-mips.c
+++ b/bfd/elfn32-mips.c
@@ -3530,15 +3530,15 @@ elf32_mips_grok_prstatus (bfd *abfd, Elf_Internal_Note *note)
default:
return FALSE;
- case 440: /* Linux/MIPS N32 */
+ case 448: /* Linux/MIPS N32 */
/* pr_cursig */
elf_tdata (abfd)->core->signal = bfd_get_16 (abfd, note->descdata + 12);
/* pr_pid */
- elf_tdata (abfd)->core->lwpid = bfd_get_32 (abfd, note->descdata + 24);
+ elf_tdata (abfd)->core->lwpid = bfd_get_32 (abfd, note->descdata + 32);
/* pr_reg */
- offset = 72;
+ offset = 80;
size = 360;
break;
--
2.7.4
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 3/4] BFD: Fix reading Linux core PRSTATUS note for MIPS n32
2017-10-06 11:03 [PATCH 3/4] BFD: Fix reading Linux core PRSTATUS note for MIPS n32 Djordje Todorovic
@ 2017-10-12 22:35 ` Maciej W. Rozycki
2017-10-17 13:47 ` Djordje Todorovic
0 siblings, 1 reply; 3+ messages in thread
From: Maciej W. Rozycki @ 2017-10-12 22:35 UTC (permalink / raw)
To: Djordje Todorovic
Cc: binutils, gdb-patches, nemanja.popov, Nikola Prica,
petar.jovanovic, Ananthakrishna Sowda (asowda)
On Fri, 6 Oct 2017, Djordje Todorovic wrote:
> The kernel struct elf_prstatus which GDB MIPS n32 uses is defined as
> following:
>
> (top-gdb-mipsN32) ptype struct elf_prstatus
> type = struct elf_prstatus {
> struct elf_siginfo pr_info;
> short pr_cursig;
> unsigned long long pr_sigpend;
> unsigned long long pr_sighold;
> __pid_t pr_pid;
> __pid_t pr_ppid;
> __pid_t pr_pgrp;
> __pid_t pr_sid;
> struct timeval pr_utime;
> struct timeval pr_stime;
> struct timeval pr_cutime;
> struct timeval pr_cstime;
> elf_gregset_t pr_reg;
> int pr_fpvalid;
> }
>
> and the size of the structure is not right in the current source code,
> because:
>
> (top-gdb-mipsN32) p sizeof(struct elf_prstatus)
> $1 = 448
Well, `struct elf_prstatus' is for n64 only and n32 and o32 both use
`struct elf_prstatus32', although a different one each, and 440 is the
correct n32 size AFAICT:
(gdb) list arch/mips/kernel/binfmt_elfn32.c:40,62
40 #include <linux/module.h>
41 #include <linux/elfcore.h>
42 #include <linux/compat.h>
43 #include <linux/math64.h>
44
45 #define elf_prstatus elf_prstatus32
46 struct elf_prstatus32
47 {
48 struct elf_siginfo pr_info; /* Info associated with signal */
49 short pr_cursig; /* Current signal */
50 unsigned int pr_sigpend; /* Set of pending signals */
51 unsigned int pr_sighold; /* Set of held signals */
52 pid_t pr_pid;
53 pid_t pr_ppid;
54 pid_t pr_pgrp;
55 pid_t pr_sid;
56 struct compat_timeval pr_utime; /* User time */
57 struct compat_timeval pr_stime; /* System time */
58 struct compat_timeval pr_cutime;/* Cumulative user time */
59 struct compat_timeval pr_cstime;/* Cumulative system time */
60 elf_gregset_t pr_reg; /* GP registers */
61 int pr_fpvalid; /* True if math co-processor being used. */
62 };
(gdb) ptype struct elf_prstatus32
type = struct elf_prstatus32 {
struct elf_siginfo pr_info;
short pr_cursig;
unsigned int pr_sigpend;
unsigned int pr_sighold;
pid_t pr_pid;
pid_t pr_ppid;
pid_t pr_pgrp;
pid_t pr_sid;
struct compat_timeval pr_utime;
struct compat_timeval pr_stime;
struct compat_timeval pr_cutime;
struct compat_timeval pr_cstime;
elf_gregset_t pr_reg;
int pr_fpvalid;
}
(gdb) print sizeof(struct elf_prstatus32)
$1 = 440
(gdb) print sizeof((struct elf_prstatus32 *)0)->pr_reg
$2 = 360
(gdb) print /d &((struct elf_prstatus32 *)0)->pr_fpvalid
$3 = 432
(gdb)
> Also, offset of the pr_pid and pr_reg have to be corrected:
>
> (top-gdb-mipsN32) print /d &((struct elf_prstatus *)0)->pr_reg
> $2 = 80
> (top-gdb-mipsN32) print /d &((struct elf_prstatus *)0)->pr_pid
> $3 = 32
Nope:
(gdb) print /d &((struct elf_prstatus32 *)0)->pr_reg
$4 = 72
(gdb) print /d &((struct elf_prstatus32 *)0)->pr_pid
$5 = 24
(gdb)
Have I missed anything?
> Also, it is detected that on MIPS n32 platform, GDB has never called functions
> for reading Linux core PRPSINFO and PRSTATUS note defined in
> bfd/elfn32-mips.c, but GDB MIPS n32 currently uses functions from
> bfd/elf32-mips.c. I am not sure if it is expected, but
> 'elf32_mips_grok_psinfo' from bfd/elfn32-mips.c is exactly the same as one
> from bfd/elf32-mips.c, because GDB MIPS n32 uses exactly the same struct
> elf_prpsinfo and there is no problem for end users. But, when GDB MIPS n32
> comes into 'elf32_mips_grok_prstatus' from bfd/elf32-mips.c, it would never go
> into 'case 256' of the 'switch' because the size of struct elf_prstatus is
> different on MIPS n32.
Well, GDB indeed does not use this stuff, however BFD does, to create
artificial sections, as you can see in the very functions you've looked
at. So if you run say `objdump -h' over a core file, then you'll see
sections which aren't really there, as easily verified with `readelf -S'.
It would be good to have this feature verified in a target-dependent
manner in the binutils test suite, however that's a separate matter.
> diff --git a/bfd/elfn32-mips.c b/bfd/elfn32-mips.c
> index 5287da3..07793b6 100644
> --- a/bfd/elfn32-mips.c
> +++ b/bfd/elfn32-mips.c
> @@ -3530,15 +3530,15 @@ elf32_mips_grok_prstatus (bfd *abfd, Elf_Internal_Note
> *note)
> default:
> return FALSE;
>
> - case 440: /* Linux/MIPS N32 */
> + case 448: /* Linux/MIPS N32 */
> /* pr_cursig */
> elf_tdata (abfd)->core->signal = bfd_get_16 (abfd, note->descdata +
> 12);
>
> /* pr_pid */
> - elf_tdata (abfd)->core->lwpid = bfd_get_32 (abfd, note->descdata +
> 24);
> + elf_tdata (abfd)->core->lwpid = bfd_get_32 (abfd, note->descdata +
> 32);
>
> /* pr_reg */
> - offset = 72;
> + offset = 80;
> size = 360;
>
> break;
So as I noted above unless you can prove me wrong, your proposed change
looks invalid to me.
Maciej
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-10-17 13:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-06 11:03 [PATCH 3/4] BFD: Fix reading Linux core PRSTATUS note for MIPS n32 Djordje Todorovic
2017-10-12 22:35 ` Maciej W. Rozycki
2017-10-17 13:47 ` Djordje Todorovic
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).