Re: [PATCH v13 1/2] gdb: mips: Apply coding guidelines

["Maciej W. Rozycki" <macro@orcam.me.uk>]

On Wed, 4 Jun 2025, Jovan Dmitrovic wrote:

> From: Milica Matic <milica.matic@htecgroup.com>
> 
> Format mips-tdep.c code as described on links:
> https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards
> https://www.gnu.org/prep/standards/standards.html#Comments
> 
> Signed-off-by: Milica Matic <milica.matic@htecgroup.com>
> Signed-off-by: Jovan Dmitrović <jovan.dmitrovic@htecgroup.com>
> ---

 The GDB project does not currently accept submissions under the DCO.  
Please resubmit under a copyright assignment with FSF, or arrange to have 
one in place if you don't have one.

 Here's a rebase, with additional small tweaks, I've made on top of:
<https://patchwork.sourceware.org/project/gdb/list/?series=65135> 
(<https://inbox.sourceware.org/gdb-patches/alpine.DEB.2.21.2608161728490.14132@angie.orcam.me.uk/>).  
I consider it ready to commit, pending the resolution of your copyright 
assignment with FSF.  Please base your further work on top of this.

  Maciej
---
 gdb/mips-tdep.c |  251 +++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 148 insertions(+), 103 deletions(-)

Index: binutils-gdb/gdb/mips-tdep.c
===================================================================
--- binutils-gdb.orig/gdb/mips-tdep.c
+++ binutils-gdb/gdb/mips-tdep.c
@@ -78,8 +78,8 @@ static int mips16_insn_at_pc_has_delay_s
 static void mips_print_float_info (struct gdbarch *, struct ui_file *,
 				   const frame_info_ptr &, const char *);
 
-/* A useful bit in the CP0 status register (MIPS_PS_REGNUM).  */
-/* This bit is set if we are emulating 32-bit FPRs on a 64-bit chip.  */
+/* A useful bit in the CP0 status register (MIPS_PS_REGNUM).
+   This bit is set if we are emulating 32-bit FPRs on a 64-bit chip.  */
 #define ST0_FR (1 << 26)
 
 /* The sizes of floating point registers.  */
@@ -1073,21 +1073,29 @@ mips_register_type (struct gdbarch *gdba
       else if (gdbarch_osabi (gdbarch) != GDB_OSABI_LINUX
 	       && rawnum >= MIPS_FIRST_EMBED_REGNUM
 	       && rawnum <= MIPS_LAST_EMBED_REGNUM)
-	/* The pseudo/cooked view of the embedded registers is always
-	   32-bit.  The raw view is handled below.  */
-	return builtin_type (gdbarch)->builtin_int32;
+	{
+	  /* The pseudo/cooked view of the embedded registers is always
+	     32-bit.  The raw view is handled below.  */
+	  return builtin_type (gdbarch)->builtin_int32;
+	}
       else if (tdep->mips64_transfers_32bit_regs_p)
-	/* The target, while possibly using a 64-bit register buffer,
-	   is only transferring 32-bits of each integer register.
-	   Reflect this in the cooked/pseudo (ABI) register value.  */
-	return builtin_type (gdbarch)->builtin_int32;
+	{
+	  /* The target, while possibly using a 64-bit register buffer,
+	     is only transferring 32-bits of each integer register.
+	     Reflect this in the cooked/pseudo (ABI) register value.  */
+	  return builtin_type (gdbarch)->builtin_int32;
+	}
       else if (mips_abi_regsize (gdbarch) == 4)
-	/* The ABI is restricted to 32-bit registers (the ISA could be
-	   32- or 64-bit).  */
-	return builtin_type (gdbarch)->builtin_int32;
+	{
+	  /* The ABI is restricted to 32-bit registers (the ISA could be
+	     32- or 64-bit).  */
+	  return builtin_type (gdbarch)->builtin_int32;
+	}
       else
-	/* 64-bit ABI.  */
-	return builtin_type (gdbarch)->builtin_int64;
+	{
+	  /* 64-bit ABI.  */
+	  return builtin_type (gdbarch)->builtin_int64;
+	}
     }
 }
 
@@ -1597,8 +1605,10 @@ mips32_bc1_pc (struct gdbarch *gdbarch,
   int cond;
 
   if (fcsr == -1)
-    /* No way to handle; it'll most likely trap anyway.  */
-    return pc;
+    {
+      /* No way to handle; it'll most likely trap anyway.  */
+      return pc;
+    }
 
   fcs = regcache_raw_get_unsigned (regcache, fcsr);
   cond = ((fcs >> 24) & 0xfe) | ((fcs >> 23) & 0x01);
@@ -1631,10 +1641,10 @@ is_octeon_bbit_op (int op, struct gdbarc
 {
   if (!is_octeon (gdbarch))
     return 0;
-  /* BBIT0 is encoded as LWC2: 110 010.  */
-  /* BBIT032 is encoded as LDC2: 110 110.  */
-  /* BBIT1 is encoded as SWC2: 111 010.  */
-  /* BBIT132 is encoded as SDC2: 111 110.  */
+  /* BBIT0 is encoded as LWC2: 110 010.
+     BBIT032 is encoded as LDC2: 110 110.
+     BBIT1 is encoded as SWC2: 111 010.
+     BBIT132 is encoded as SDC2: 111 110.  */
   if (op == 50 || op == 54 || op == 58 || op == 62)
     return 1;
   return 0;
@@ -1653,12 +1663,13 @@ mips32_next_pc (struct regcache *regcach
   int op;
   inst = mips_fetch_instruction (gdbarch, ISA_MIPS, pc, NULL);
   op = itype_op (inst);
-  if ((inst & 0xe0000000) != 0)		/* Not a special, jump or branch
-					   instruction.  */
+  if ((inst & 0xe0000000) != 0)
     {
+      /* Not a special, jump or branch instruction.  */
+
       if (op >> 2 == 5)
-	/* BEQL, BNEL, BLEZL, BGTZL: bits 0101xx */
 	{
+	  /* BEQL, BNEL, BLEZL, BGTZL: bits 0101xx */
 	  switch (op & 0x03)
 	    {
 	    case 0:		/* BEQL */
@@ -1674,20 +1685,26 @@ mips32_next_pc (struct regcache *regcach
 	    }
 	}
       else if (op == 17 && itype_rs (inst) == 8)
-	/* BC1F, BC1FL, BC1T, BC1TL: 010001 01000 */
-	pc = mips32_bc1_pc (gdbarch, regcache, inst, pc + 4, 1);
+	{
+	  /* BC1F, BC1FL, BC1T, BC1TL: 010001 01000 */
+	  pc = mips32_bc1_pc (gdbarch, regcache, inst, pc + 4, 1);
+	}
       else if (op == 17 && itype_rs (inst) == 9
 	       && (itype_rt (inst) & 2) == 0)
-	/* BC1ANY2F, BC1ANY2T: 010001 01001 xxx0x */
-	pc = mips32_bc1_pc (gdbarch, regcache, inst, pc + 4, 2);
+	{
+	  /* BC1ANY2F, BC1ANY2T: 010001 01001 xxx0x */
+	  pc = mips32_bc1_pc (gdbarch, regcache, inst, pc + 4, 2);
+	}
       else if (op == 17 && itype_rs (inst) == 10
 	       && (itype_rt (inst) & 2) == 0)
-	/* BC1ANY4F, BC1ANY4T: 010001 01010 xxx0x */
-	pc = mips32_bc1_pc (gdbarch, regcache, inst, pc + 4, 4);
+	{
+	  /* BC1ANY4F, BC1ANY4T: 010001 01010 xxx0x */
+	  pc = mips32_bc1_pc (gdbarch, regcache, inst, pc + 4, 4);
+	}
       else if (op == 29)
-	/* JALX: 011101 */
-	/* The new PC will be alternate mode.  */
 	{
+	  /* JALX: 011101
+	     The new PC will be alternate mode.  */
 	  unsigned long reg;
 
 	  reg = jtype_target (inst) << 2;
@@ -1701,9 +1718,8 @@ mips32_next_pc (struct regcache *regcach
 	  branch_if = op == 58 || op == 62;
 	  bit = itype_rt (inst);
 
-	  /* Take into account the *32 instructions.  */
 	  if (op == 54 || op == 62)
-	    bit += 32;
+	    bit += 32;		/* Take into account the *32 instructions.  */
 
 	  pc_adj = mips32_relative_offset (inst);
 	  if (pc_adj
@@ -1718,7 +1734,8 @@ mips32_next_pc (struct regcache *regcach
 	pc += 4;		/* Not a branch, next instruction is easy.  */
     }
   else
-    {				/* This gets way messy.  */
+    {
+      /* This gets way messy.  */
 
       /* Further subdivide into SPECIAL, REGIMM and other.  */
       switch (op & 0x07)	/* Extract bits 28,27,26.  */
@@ -1788,8 +1805,10 @@ mips32_next_pc (struct regcache *regcach
 		    int dspctl = mips_regnum (gdbarch)->dspctl;
 
 		    if (dspctl == -1)
-		      /* No way to handle; it'll most likely trap anyway.  */
-		      break;
+		      {
+			/* No way to handle; it'll most likely trap anyway.  */
+			break;
+		      }
 
 		    pc_adj = mips32_relative_offset (inst);
 		    if (pc_adj
@@ -1919,8 +1938,10 @@ micromips_bc1_pc (struct gdbarch *gdbarc
   int cond;
 
   if (fcsr == -1)
-    /* No way to handle; it'll most likely trap anyway.  */
-    return pc;
+    {
+      /* No way to handle; it'll most likely trap anyway.  */
+      return pc;
+    }
 
   fcs = regcache_raw_get_unsigned (regcache, fcsr);
   cond = ((fcs >> 24) & 0xfe) | ((fcs >> 23) & 0x01);
@@ -2048,8 +2069,10 @@ micromips_next_pc (struct regcache *regc
 	    case 0x14: /* BC2F: bits 010000 10100 xxx00 */
 	    case 0x15: /* BC2T: bits 010000 10101 xxx00 */
 	      if (((insn >> 16) & 0x3) == 0x0)
-		/* BC2F, BC2T: don't know how to handle these.  */
-		break;
+		{
+		  /* BC2F, BC2T: don't know how to handle these.  */
+		  break;
+		}
 	      break;
 
 	    case 0x1a: /* BPOSGE64: bits 010000 11010 */
@@ -2059,8 +2082,10 @@ micromips_next_pc (struct regcache *regc
 		int dspctl = mips_regnum (gdbarch)->dspctl;
 
 		if (dspctl == -1)
-		  /* No way to handle; it'll most likely trap anyway.  */
-		  break;
+		  {
+		    /* No way to handle; it'll most likely trap anyway.  */
+		    break;
+		  }
 
 		pc_adj = micromips_relative_offset16 (insn);
 		if (pc_adj
@@ -2689,10 +2714,12 @@ mips16_scan_prologue (struct gdbarch *gd
 	  if (offset < 0)	/* Negative stack adjustment?  */
 	    frame_offset -= offset;
 	  else
-	    /* Exit loop if a positive stack adjustment is found, which
-	       usually means that the stack cleanup code in the function
-	       epilogue is reached.  */
-	    break;
+	    {
+	      /* Exit loop if a positive stack adjustment is found, which
+		 usually means that the stack cleanup code in the function
+		 epilogue is reached.  */
+	      break;
+	    }
 	}
       else if ((inst & 0xf800) == 0xd000)	/* sw reg,n($sp) */
 	{
@@ -3586,10 +3613,12 @@ mips32_scan_prologue (struct gdbarch *gd
 	  if (offset < 0)		/* Negative stack adjustment?  */
 	    frame_offset -= offset;
 	  else
-	    /* Exit loop if a positive stack adjustment is found, which
-	       usually means that the stack cleanup code in the function
-	       epilogue is reached.  */
-	    break;
+	    {
+	      /* Exit loop if a positive stack adjustment is found, which
+		 usually means that the stack cleanup code in the function
+		 epilogue is reached.  */
+	      break;
+	    }
 	  seen_sp_adjust = 1;
 	}
       else if (((high_word & 0xFFE0) == 0xafa0) /* sw reg,offset($sp) */
@@ -3990,22 +4019,24 @@ mips_addr_bits_remove (struct gdbarch *g
   mips_gdbarch_tdep *tdep = gdbarch_tdep<mips_gdbarch_tdep> (gdbarch);
 
   if (mips_mask_address_p (tdep) && (((ULONGEST) addr) >> 32 == 0xffffffffUL))
-    /* This hack is a work-around for existing boards using PMON, the
-       simulator, and any other 64-bit targets that doesn't have true
-       64-bit addressing.  On these targets, the upper 32 bits of
-       addresses are ignored by the hardware.  Thus, the PC or SP are
-       likely to have been sign extended to all 1s by instruction
-       sequences that load 32-bit addresses.  For example, a typical
-       piece of code that loads an address is this:
+    {
+      /* This hack is a work-around for existing boards using PMON, the
+	 simulator, and any other 64-bit targets that doesn't have true
+	 64-bit addressing.  On these targets, the upper 32 bits of
+	 addresses are ignored by the hardware.  Thus, the PC or SP are
+	 likely to have been sign extended to all 1s by instruction
+	 sequences that load 32-bit addresses.  For example, a typical
+	 piece of code that loads an address is this:
 
-       lui $r2, <upper 16 bits>
-       ori $r2, <lower 16 bits>
+	 lui $r2, <upper 16 bits>
+	 ori $r2, <lower 16 bits>
 
-       But the lui sign-extends the value such that the upper 32 bits
-       may be all 1s.  The workaround is simply to mask off these
-       bits.  In the future, gcc may be changed to support true 64-bit
-       addressing, and this masking will have to be disabled.  */
-    return addr &= 0xffffffffUL;
+	 But the lui sign-extends the value such that the upper 32 bits
+	 may be all 1s.  The workaround is simply to mask off these
+	 bits.  In the future, gcc may be changed to support true 64-bit
+	 addressing, and this masking will have to be disabled.  */
+      return addr &= 0xffffffffUL;
+    }
   else
     return addr;
 }
@@ -7519,36 +7550,42 @@ mips_adjust_breakpoint_address (struct g
 	    break;
 	  addr -= MIPS_INSN16_SIZE;
 	  if (i == 1 && insn_at_pc_has_delay_slot (gdbarch, addr, 0))
-	    /* Looks like a JR/JALR at [target-1], but it could be
-	       the second word of a previous JAL/JALX, so record it
-	       and check back one more.  */
-	    jmpaddr = addr;
+	    {
+	      /* Looks like a JR/JALR at [target-1], but it could be
+		 the second word of a previous JAL/JALX, so record it
+		 and check back one more.  */
+	      jmpaddr = addr;
+	    }
 	  else if (i > 1 && insn_at_pc_has_delay_slot (gdbarch, addr, 1))
 	    {
 	      if (i == 2)
-		/* Looks like a JAL/JALX at [target-2], but it could also
-		   be the second word of a previous JAL/JALX, record it,
-		   and check back one more.  */
-		jmpaddr = addr;
+		{
+		  /* Looks like a JAL/JALX at [target-2], but it could also
+		     be the second word of a previous JAL/JALX, record it,
+		     and check back one more.  */
+		  jmpaddr = addr;
+		}
 	      else
-		/* Looks like a JAL/JALX at [target-3], so any previously
-		   recorded JAL/JALX or JR/JALR must be wrong, because:
+		{
+		  /* Looks like a JAL/JALX at [target-3], so any previously
+		     recorded JAL/JALX or JR/JALR must be wrong, because:
 
-		   >-3: JAL
-		    -2: JAL-ext (can't be JAL/JALX)
-		    -1: bdslot (can't be JR/JALR)
-		     0: target insn
+		     >-3: JAL
+		      -2: JAL-ext (can't be JAL/JALX)
+		      -1: bdslot (can't be JR/JALR)
+		       0: target insn
 
-		   Of course it could be another JAL-ext which looks
-		   like a JAL, but in that case we'd have broken out
-		   of this loop at [target-2]:
+		     Of course it could be another JAL-ext which looks
+		     like a JAL, but in that case we'd have broken out
+		     of this loop at [target-2]:
 
-		    -4: JAL
-		   >-3: JAL-ext
-		    -2: bdslot (can't be jmp)
-		    -1: JR/JALR
-		     0: target insn  */
-		jmpaddr = 0;
+		      -4: JAL
+		     >-3: JAL-ext
+		      -2: bdslot (can't be jmp)
+		      -1: JR/JALR
+		       0: target insn  */
+		  jmpaddr = 0;
+		}
 	    }
 	  else
 	    {
@@ -7815,15 +7852,19 @@ mips_skip_mips16_trampoline_code (const
 	       && mips_is_stub_suffix (name + prefixlen + 3, 0))
 	{
 	  if (pc == start_addr)
-	    /* This is the 'call' part of a call stub.  The return
-	       address is in $2.  */
-	    return get_frame_register_signed
-		     (frame, gdbarch_num_regs (gdbarch) + MIPS_V0_REGNUM);
+	    {
+	      /* This is the 'call' part of a call stub.  The return
+		 address is in $2.  */
+	      return get_frame_register_signed
+		       (frame, gdbarch_num_regs (gdbarch) + MIPS_V0_REGNUM);
+	    }
 	  else
-	    /* This is the 'return' part of a call stub.  The return
-	       address is in $18.  */
-	    return get_frame_register_signed
-		     (frame, gdbarch_num_regs (gdbarch) + MIPS_S2_REGNUM);
+	    {
+	      /* This is the 'return' part of a call stub.  The return
+		 address is in $18.  */
+	      return get_frame_register_signed
+		       (frame, gdbarch_num_regs (gdbarch) + MIPS_S2_REGNUM);
+	    }
 	}
       else
 	return 0;		/* Not a stub.  */
@@ -7835,15 +7876,19 @@ mips_skip_mips16_trampoline_code (const
       || startswith (name, mips_str_call_stub))
     {
       if (pc == start_addr)
-	/* This is the 'call' part of a call stub.  Call this helper
-	   to scan through this code for interesting instructions
-	   and determine the final PC.  */
-	return mips_get_mips16_fn_stub_pc (frame, pc);
+	{
+	  /* This is the 'call' part of a call stub.  Call this helper
+	     to scan through this code for interesting instructions
+	     and determine the final PC.  */
+	  return mips_get_mips16_fn_stub_pc (frame, pc);
+	}
       else
-	/* This is the 'return' part of a call stub.  The return address
-	   is in $18.  */
-	return get_frame_register_signed
-		 (frame, gdbarch_num_regs (gdbarch) + MIPS_S2_REGNUM);
+	{
+	  /* This is the 'return' part of a call stub.  The return address
+	     is in $18.  */
+	  return get_frame_register_signed
+		   (frame, gdbarch_num_regs (gdbarch) + MIPS_S2_REGNUM);
+	}
     }
 
   return 0;			/* Not a stub.  */