[PATCH 3/4] BFD: Fix reading Linux core PRSTATUS note for MIPS n32

[PATCH 3/4] BFD: Fix reading Linux core PRSTATUS note for MIPS n32

[Djordje Todorovic]

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

Re: [PATCH 3/4] BFD: Fix reading Linux core PRSTATUS note for MIPS n32

[Maciej W. Rozycki]

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

Re: [PATCH 3/4] BFD: Fix reading Linux core PRSTATUS note for MIPS n32

[Djordje Todorovic]

Hi Maciej,

I agree with you.

Thanks,
Djordje