[PATCH, FT32] Correctly interpret function prologs

[PATCH, FT32] Correctly interpret function prologs

[James Bowman]

The stack unwinder did not understand the function prologs
generated by gcc with -Os. Add code to recognize and interpret the
prolog calls.

OK to apply?


2015-09-29  James Bowman  <james.bowman@ftdichip.com>

	* ft32-tdep.c (ft32_analyze_prologue): Add function prolog
	subroutine handling.

diff --git a/gdb/ft32-tdep.c b/gdb/ft32-tdep.c
index 00cf847..0b51af3 100644
--- a/gdb/ft32-tdep.c
+++ b/gdb/ft32-tdep.c
@@ -164,33 +164,66 @@ ft32_analyze_prologue (CORE_ADDR start_addr, CORE_ADDR end_addr,
   CORE_ADDR next_addr;
   ULONGEST inst, inst2;
   LONGEST offset;
-  int regnum;
+  int regnum, pushreg;
+  struct bound_minimal_symbol msymbol;
+  unsigned prologs[32];
 
   cache->saved_regs[FT32_PC_REGNUM] = 0;
   cache->framesize = 0;
 
+  for (regnum = 0; regnum < 32; regnum++)
+    {
+      char prolog_symbol[32];
+
+      snprintf (prolog_symbol, sizeof (prolog_symbol), "__prolog_$r%02d",
+		regnum);
+      msymbol = lookup_minimal_symbol (prolog_symbol, NULL, NULL);
+      if (msymbol.minsym)
+	prologs[regnum] = BMSYMBOL_VALUE_ADDRESS (msymbol);
+      else
+	prologs[regnum] = 0;
+    }
+
   if (start_addr >= end_addr)
-      return end_addr;
+    return end_addr;
 
   cache->established = 0;
-  for (next_addr = start_addr; next_addr < end_addr; )
+  for (next_addr = start_addr; next_addr < end_addr;)
     {
       inst = read_memory_unsigned_integer (next_addr, 4, byte_order);
 
       if (FT32_IS_PUSH (inst))
 	{
-	  regnum = FT32_R0_REGNUM + FT32_PUSH_REG (inst);
+	  pushreg = FT32_PUSH_REG (inst);
 	  cache->framesize += 4;
-	  cache->saved_regs[regnum] = cache->framesize;
+	  cache->saved_regs[FT32_R0_REGNUM + pushreg] = cache->framesize;
 	  next_addr += 4;
 	}
+      else if (FT32_IS_CALL (inst))
+	{
+	  for (regnum = 0; regnum < 32; regnum++)
+	    {
+	      if ((4 * (inst & 0x3ffff)) == prologs[regnum])
+		{
+		  for (pushreg = 13; pushreg <= regnum; pushreg++)
+		    {
+		      cache->framesize += 4;
+		      cache->saved_regs[FT32_R0_REGNUM + pushreg] =
+			cache->framesize;
+		    }
+		  next_addr += 4;
+		}
+	    }
+	  break;
+	}
       else
 	break;
     }
   for (regnum = FT32_R0_REGNUM; regnum < FT32_PC_REGNUM; regnum++)
     {
       if (cache->saved_regs[regnum] != REG_UNAVAIL)
-	cache->saved_regs[regnum] = cache->framesize - cache->saved_regs[regnum];
+	cache->saved_regs[regnum] =
+	  cache->framesize - cache->saved_regs[regnum];
     }
   cache->saved_regs[FT32_PC_REGNUM] = cache->framesize;
 

Re: [PATCH, FT32] Correctly interpret function prologs

[Kevin Buettner]

Hi James,

See my comments inline with your patch below.

On Tue, 29 Sep 2015 16:38:57 +0000
James Bowman <james.bowman@ftdichip.com> wrote:

> The stack unwinder did not understand the function prologs
> generated by gcc with -Os. Add code to recognize and interpret the
> prolog calls.
> 
> OK to apply?
> 
> 
> 2015-09-29  James Bowman  <james.bowman@ftdichip.com>
> 
> 	* ft32-tdep.c (ft32_analyze_prologue): Add function prolog
> 	subroutine handling.
> 
> diff --git a/gdb/ft32-tdep.c b/gdb/ft32-tdep.c
> index 00cf847..0b51af3 100644
> --- a/gdb/ft32-tdep.c
> +++ b/gdb/ft32-tdep.c
> @@ -164,33 +164,66 @@ ft32_analyze_prologue (CORE_ADDR start_addr, CORE_ADDR end_addr,
>    CORE_ADDR next_addr;
>    ULONGEST inst, inst2;
>    LONGEST offset;
> -  int regnum;
> +  int regnum, pushreg;
> +  struct bound_minimal_symbol msymbol;
> +  unsigned prologs[32];

I think that the type of prologs[] should be CORE_ADDR instead of `unsigned'.

>  
>    cache->saved_regs[FT32_PC_REGNUM] = 0;
>    cache->framesize = 0;
>  

I think it'd be useful to have a brief comment somewhere in here
describing what a call to __prolog_$rN does.  I'm guessing that
these functions push a number of registers.  It'd be useful to know,
for a given N, which registers are pushed and the order in which they're
pushed.

> +  for (regnum = 0; regnum < 32; regnum++)
> +    {
> +      char prolog_symbol[32];
> +
> +      snprintf (prolog_symbol, sizeof (prolog_symbol), "__prolog_$r%02d",
> +		regnum);
> +      msymbol = lookup_minimal_symbol (prolog_symbol, NULL, NULL);
> +      if (msymbol.minsym)
> +	prologs[regnum] = BMSYMBOL_VALUE_ADDRESS (msymbol);
> +      else
> +	prologs[regnum] = 0;
> +    }
> +
>    if (start_addr >= end_addr)
> -      return end_addr;
> +    return end_addr;
>  
>    cache->established = 0;
> -  for (next_addr = start_addr; next_addr < end_addr; )
> +  for (next_addr = start_addr; next_addr < end_addr;)
>      {
>        inst = read_memory_unsigned_integer (next_addr, 4, byte_order);
>  
>        if (FT32_IS_PUSH (inst))
>  	{
> -	  regnum = FT32_R0_REGNUM + FT32_PUSH_REG (inst);
> +	  pushreg = FT32_PUSH_REG (inst);
>  	  cache->framesize += 4;
> -	  cache->saved_regs[regnum] = cache->framesize;
> +	  cache->saved_regs[FT32_R0_REGNUM + pushreg] = cache->framesize;
>  	  next_addr += 4;
>  	}
> +      else if (FT32_IS_CALL (inst))
> +	{
> +	  for (regnum = 0; regnum < 32; regnum++)
> +	    {
> +	      if ((4 * (inst & 0x3ffff)) == prologs[regnum])
> +		{
> +		  for (pushreg = 13; pushreg <= regnum; pushreg++)

This looks strange to me.  The outer loop has regnum ranging from 0
thru 31.  But this inner loop won't be executed for regnum values
between 0 thru 12 due to pushreg starting at 13.

Is that really what you want?

If so, it seems to me that calls to __prolog_$r01 thru __prolog_$r12
don't contribute anything to the frame.  Please add a comment
about this if that's truly the case.

> +		    {
> +		      cache->framesize += 4;
> +		      cache->saved_regs[FT32_R0_REGNUM + pushreg] =
> +			cache->framesize;
> +		    }
> +		  next_addr += 4;
> +		}
> +	    }
> +	  break;
> +	}
>        else
>  	break;
>      }
>    for (regnum = FT32_R0_REGNUM; regnum < FT32_PC_REGNUM; regnum++)
>      {
>        if (cache->saved_regs[regnum] != REG_UNAVAIL)
> -	cache->saved_regs[regnum] = cache->framesize - cache->saved_regs[regnum];
> +	cache->saved_regs[regnum] =
> +	  cache->framesize - cache->saved_regs[regnum];
>      }
>    cache->saved_regs[FT32_PC_REGNUM] = cache->framesize;
>  

RE: [PATCH, FT32] Correctly interpret function prologs

[James Bowman]

Hi Kevin,

Thanks for the review, I have corrected as suggested, and added a comment
describing the action of the prolog subroutines.

OK to apply?


2015-10-02  James Bowman  <james.bowman@ftdichip.com>

	* ft32-tdep.c (ft32_analyze_prologue): Add function prolog
	subroutine handling.

diff --git a/gdb/ft32-tdep.c b/gdb/ft32-tdep.c
index 00cf847..e9da23e 100644
--- a/gdb/ft32-tdep.c
+++ b/gdb/ft32-tdep.c
@@ -164,33 +164,76 @@ ft32_analyze_prologue (CORE_ADDR start_addr, CORE_ADDR end_addr,
   CORE_ADDR next_addr;
   ULONGEST inst, inst2;
   LONGEST offset;
-  int regnum;
+  int regnum, pushreg;
+  struct bound_minimal_symbol msymbol;
+  const int first_saved_reg = 13;	/* The first saved register.  */
+  /* PROLOGS are addresses of the subroutine prologs, PROLOGS[n]
+     is the address of __prolog_$rN.
+     __prolog_$rN pushes registers from 13 through n inclusive.
+     So for example CALL __prolog_$r15 is equivalent to:
+       PUSH $r13 
+       PUSH $r14 
+       PUSH $r15 
+     Note that PROLOGS[0] through PROLOGS[12] are unused.  */
+  CORE_ADDR prologs[32];
 
   cache->saved_regs[FT32_PC_REGNUM] = 0;
   cache->framesize = 0;
 
+  for (regnum = first_saved_reg; regnum < 32; regnum++)
+    {
+      char prolog_symbol[32];
+
+      snprintf (prolog_symbol, sizeof (prolog_symbol), "__prolog_$r%02d",
+		regnum);
+      msymbol = lookup_minimal_symbol (prolog_symbol, NULL, NULL);
+      if (msymbol.minsym)
+	prologs[regnum] = BMSYMBOL_VALUE_ADDRESS (msymbol);
+      else
+	prologs[regnum] = 0;
+    }
+
   if (start_addr >= end_addr)
-      return end_addr;
+    return end_addr;
 
   cache->established = 0;
-  for (next_addr = start_addr; next_addr < end_addr; )
+  for (next_addr = start_addr; next_addr < end_addr;)
     {
       inst = read_memory_unsigned_integer (next_addr, 4, byte_order);
 
       if (FT32_IS_PUSH (inst))
 	{
-	  regnum = FT32_R0_REGNUM + FT32_PUSH_REG (inst);
+	  pushreg = FT32_PUSH_REG (inst);
 	  cache->framesize += 4;
-	  cache->saved_regs[regnum] = cache->framesize;
+	  cache->saved_regs[FT32_R0_REGNUM + pushreg] = cache->framesize;
 	  next_addr += 4;
 	}
+      else if (FT32_IS_CALL (inst))
+	{
+	  for (regnum = first_saved_reg; regnum < 32; regnum++)
+	    {
+	      if ((4 * (inst & 0x3ffff)) == prologs[regnum])
+		{
+		  for (pushreg = first_saved_reg; pushreg <= regnum;
+		       pushreg++)
+		    {
+		      cache->framesize += 4;
+		      cache->saved_regs[FT32_R0_REGNUM + pushreg] =
+			cache->framesize;
+		    }
+		  next_addr += 4;
+		}
+	    }
+	  break;
+	}
       else
 	break;
     }
   for (regnum = FT32_R0_REGNUM; regnum < FT32_PC_REGNUM; regnum++)
     {
       if (cache->saved_regs[regnum] != REG_UNAVAIL)
-	cache->saved_regs[regnum] = cache->framesize - cache->saved_regs[regnum];
+	cache->saved_regs[regnum] =
+	  cache->framesize - cache->saved_regs[regnum];
     }
   cache->saved_regs[FT32_PC_REGNUM] = cache->framesize;
 

Re: [PATCH, FT32] Correctly interpret function prologs

[Kevin Buettner]

On Fri, 2 Oct 2015 18:35:13 +0000
James Bowman <james.bowman@ftdichip.com> wrote:

> Hi Kevin,
> 
> Thanks for the review, I have corrected as suggested, and added a comment
> describing the action of the prolog subroutines.
> 
> OK to apply?

Yes, please apply.

Kevin