[PATCH 1/2] Nios2, libbfd: Support new coredump .reg section

[PATCH 1/2] Nios2, libbfd: Support new coredump .reg section

[Moritz Strübe]

The coredumps of the current kernel are larger.
---
 bfd/elf32-nios2.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/bfd/elf32-nios2.c b/bfd/elf32-nios2.c
index 806ec314c82..3d0fa6fbf27 100644
--- a/bfd/elf32-nios2.c
+++ b/bfd/elf32-nios2.c
@@ -1921,7 +1921,18 @@ nios2_grok_prstatus (bfd *abfd, Elf_Internal_Note *note)
     {
     default:
       return false;
+    case 272:  /* Linux/Nios II */
+      /* pr_cursig */
+      elf_tdata (abfd)->core->signal = bfd_get_16 (abfd, note->descdata + 12);
+
+      /* pr_pid */
+      elf_tdata (abfd)->core->pid = bfd_get_32 (abfd, note->descdata + 24);
 
+      /* pr_reg */
+      offset = 72;
+      size = 196;
+
+      break;
     case 212:	      /* Linux/Nios II */
       /* pr_cursig */
       elf_tdata (abfd)->core->signal = bfd_get_16 (abfd, note->descdata + 12);
-- 
2.30.2

Re: [PATCH 1/2] Nios2, libbfd: Support new coredump .reg section

[Tom Tromey]

>>>>> "Moritz" == Moritz Strübe <moritz.struebe@siemens-energy.com> writes:

Moritz> The coredumps of the current kernel are larger.

Hi.  BFD patches should go to the binutils list.

FWIW I'm a little surprised to see this given that nios2 was just
deprecated in GCC.

Tom

Re: [PATCH 1/2] Nios2, libbfd: Support new coredump .reg section

[Luis Machado]

Hi,

I just spotted Sandra's message [1] about nios2 deprecation.

Do we have plans for deprecating such support in binutils/gdb as well, for the
sake of reducing future maintenance burden?


On 4/19/24 11:39, Moritz Strübe wrote:
> The coredumps of the current kernel are larger.
> ---
>  bfd/elf32-nios2.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/bfd/elf32-nios2.c b/bfd/elf32-nios2.c
> index 806ec314c82..3d0fa6fbf27 100644
> --- a/bfd/elf32-nios2.c
> +++ b/bfd/elf32-nios2.c
> @@ -1921,7 +1921,18 @@ nios2_grok_prstatus (bfd *abfd, Elf_Internal_Note *note)
>      {
>      default:
>        return false;
> +    case 272:  /* Linux/Nios II */
> +      /* pr_cursig */
> +      elf_tdata (abfd)->core->signal = bfd_get_16 (abfd, note->descdata + 12);
> +
> +      /* pr_pid */
> +      elf_tdata (abfd)->core->pid = bfd_get_32 (abfd, note->descdata + 24);
>  
> +      /* pr_reg */
> +      offset = 72;
> +      size = 196;
> +
> +      break;
>      case 212:	      /* Linux/Nios II */
>        /* pr_cursig */
>        elf_tdata (abfd)->core->signal = bfd_get_16 (abfd, note->descdata + 12);

[1] https://gcc.gnu.org/pipermail/gcc/2024-April/243749.html

Re: [PATCH 1/2] Nios2, libbfd: Support new coredump .reg section

[Sandra Loosemore]

On 4/23/24 07:28, Luis Machado wrote:
> Hi,
> 
> I just spotted Sandra's message [1] about nios2 deprecation.
> 
> Do we have plans for deprecating such support in binutils/gdb as well, for the
> sake of reducing future maintenance burden?

Yes, I plan to take care of producing patches to remove the nios2 port 
entirely from binutils/gdb, as well as from gcc, a little bit farther 
down the line.  I'm not sure of the best timing with respect to upcoming 
releases, and at the moment I am swamped with other tasks anyway, but 
it's definitely on my list of things to do -- I don't want to leave this 
as a pointless maintenance burden for others.

-Sandra

Re: [PATCH 1/2] Nios2, libbfd: Support new coredump .reg section

[Simon Marchi]

On 2024-04-23 10:51, Sandra Loosemore wrote:
> On 4/23/24 07:28, Luis Machado wrote:
>> Hi,
>>
>> I just spotted Sandra's message [1] about nios2 deprecation.
>>
>> Do we have plans for deprecating such support in binutils/gdb as well, for the
>> sake of reducing future maintenance burden?
> 
> Yes, I plan to take care of producing patches to remove the nios2 port entirely from binutils/gdb, as well as from gcc, a little bit farther down the line.  I'm not sure of the best timing with respect to upcoming releases, and at the moment I am swamped with other tasks anyway, but it's definitely on my list of things to do -- I don't want to leave this as a pointless maintenance burden for others.
> 
> -Sandra

Thanks a lot for being proactive with this.

Simon

Re: [PATCH 1/2] Nios2, libbfd: Support new coredump .reg section

[Luis Machado]

On 4/23/24 15:51, Sandra Loosemore wrote:
> On 4/23/24 07:28, Luis Machado wrote:
>> Hi,
>>
>> I just spotted Sandra's message [1] about nios2 deprecation.
>>
>> Do we have plans for deprecating such support in binutils/gdb as well, for the
>> sake of reducing future maintenance burden?
> 
> Yes, I plan to take care of producing patches to remove the nios2 port entirely from binutils/gdb, as well as from gcc, a little bit farther down the line.  I'm not sure of the best timing with respect to upcoming releases, and at the moment I am swamped with other tasks anyway, but it's definitely on my list of things to do -- I don't want to leave this as a pointless maintenance burden for others.
> 
> -Sandra

Great. Thanks for the update Sandra.

[PATCH 2/2] Nios2, gdb: Adjust to new coredump format

[Moritz Strübe]

Current kernels create their coredumps using ptrace. Adjust to this new
format.
---
 gdb/nios2-linux-tdep.c | 68 ++++++++++++++++++++----------------------
 1 file changed, 32 insertions(+), 36 deletions(-)

diff --git a/gdb/nios2-linux-tdep.c b/gdb/nios2-linux-tdep.c
index fb9e88326fb..16581508688 100644
--- a/gdb/nios2-linux-tdep.c
+++ b/gdb/nios2-linux-tdep.c
@@ -32,46 +32,40 @@
 #include "gdbarch.h"
 
 /* Core file and register set support.  */
+/* See arch/nios2/kernel/ptrace.c */
 
-/* Map from the normal register enumeration order to the order that
-   registers appear in core files, which corresponds to the order
-   of the register slots in the kernel's struct pt_regs.  */
+#define NIOS2_GREGS_SIZE (4 * NIOS2_NUM_REGS)
 
-static const int reg_offsets[NIOS2_NUM_REGS] =
-{
-  -1,  8,  9, 10, 11, 12, 13, 14,	/* r0 - r7 */
-  0,  1,  2,  3,  4,  5,  6,  7,	/* r8 - r15 */
-  23, 24, 25, 26, 27, 28, 29, 30,	/* r16 - r23 */
-  -1, -1, 19, 18, 17, 21, -1, 16,	/* et bt gp sp fp ea sstatus ra */
-  21,					/* pc */
-  -1, 20, -1, -1, -1, -1, -1, -1,	/* status estatus ...  */
-  -1, -1, -1, -1, -1, -1, -1, -1
-};
-
-/* General register set size.  Should match sizeof (struct pt_regs) +
-   sizeof (struct switch_stack) from the NIOS2 Linux kernel patch.  */
-
-#define NIOS2_GREGS_SIZE (4 * 34)
+/* Registers not set by dump */
+static std::set<int> notsetregs = {0, 24, 25, 30};
 
 /* Implement the supply_regset hook for core files.  */
-
 static void
 nios2_supply_gregset (const struct regset *regset,
 		      struct regcache *regcache,
 		      int regnum, const void *gregs_buf, size_t len)
 {
   const gdb_byte *gregs = (const gdb_byte *) gregs_buf;
-  int regno;
-  static const gdb_byte zero_buf[4] = {0, 0, 0, 0};
-
-  for (regno = NIOS2_Z_REGNUM; regno <= NIOS2_MPUACC_REGNUM; regno++)
-    if (regnum == -1 || regnum == regno)
-      {
-	if (reg_offsets[regno] != -1)
-	  regcache->raw_supply (regno, gregs + 4 * reg_offsets[regno]);
-	else
-	  regcache->raw_supply (regno, zero_buf);
+  const uint32_t *regvals = (const uint32_t *) gregs_buf;
+
+  // If regno is 0 dump all registers
+  if(regnum > 0 ){
+    regcache->raw_supply (regnum, gregs + 4 * regnum);
+  } else {
+    for (auto regno = NIOS2_Z_REGNUM; regno < NIOS2_NUM_REGS; regno++){
+      // Only CPU registers are set by dump. Thus ignore everything above NIOS2_PC_REGNUM
+      if(notsetregs.find(regno) == notsetregs.end() && regno <= NIOS2_PC_REGNUM ) {
+        regcache->raw_supply(regno, gregs + 4 * regno);
+      } else {
+        // Print a warning in case the register is suddenly set in the future.
+        if(regvals[regno] != 0){
+          warning (_("'.reg': expected regno %d to be 0, but was 0x%08x."), regno, regvals[regno]);
+        }
+        // Registers not available should be set passing NULL.
+        regcache->raw_supply(regno, NULL);
       }
+    }
+  }
 }
 
 /* Implement the collect_regset hook for core files.  */
@@ -82,14 +76,16 @@ nios2_collect_gregset (const struct regset *regset,
 		       int regnum, void *gregs_buf, size_t len)
 {
   gdb_byte *gregs = (gdb_byte *) gregs_buf;
-  int regno;
-
-  for (regno = NIOS2_Z_REGNUM; regno <= NIOS2_MPUACC_REGNUM; regno++)
-    if (regnum == -1 || regnum == regno)
-      {
-	if (reg_offsets[regno] != -1)
-	  regcache->raw_collect (regno, gregs + 4 * reg_offsets[regno]);
+  // If regno is 0 dump all registers
+  if(regnum > 0 ){
+    regcache->raw_collect (regnum, gregs + 4 * regnum);
+  } else {
+    for (auto regno = NIOS2_Z_REGNUM; regno <= NIOS2_NUM_REGS; regno++){
+      if(notsetregs.find(regno) == notsetregs.end() && regno <= NIOS2_PC_REGNUM ) {
+        regcache->raw_collect(regno, gregs + 4 * regno);
       }
+    }
+  }
 }
 
 static const struct regset nios2_core_regset =
-- 
2.30.2

Re: [PATCH 2/2] Nios2, gdb: Adjust to new coredump format

[Tom Tromey]

>>>>> "Moritz" == Moritz Strübe <moritz.struebe@siemens-energy.com> writes:

Thank you for the patch.

Moritz> Current kernels create their coredumps using ptrace. Adjust to this new
Moritz> format.

Is there any need to support old coredumps as well; and if not, why not?

Moritz> +/* Registers not set by dump */
Moritz> +static std::set<int> notsetregs = {0, 24, 25, 30};

Should be const.

Moritz>  /* Implement the supply_regset hook for core files.  */
Moritz> -
Moritz>  static void

Spurious change.

Moritz> +  // If regno is 0 dump all registers
Moritz> +  if(regnum > 0 ){
Moritz> +    regcache->raw_supply (regnum, gregs + 4 * regnum);
Moritz> +  } else {

gdb doesn't use '//' comments and also uses a different formatting
style.

thanks,
Tom