[RFA] Fix PR tdep/1291, SH prologue scanning bug

[RFA] Fix PR tdep/1291, SH prologue scanning bug

[Fred Fish]

This patch fixes the bug reported in PR 1291.  It is based on the suggested
patch included in the PR.  I believe it is small enough to not need a
copyright assignment, but recent events may have changed that.  :-(

-Fred

2004-02-19  Fred Fish  <fnf@redhat.com>

	Fix for PR tdep/1291 as suggested by inaba@src.ricoh.co.jp
	* sh-tdep.c (IS_MOV_R3): Rename to IS_MOV_IMM_R3 and fix pattern.
	(IS_ADD_R3SP): Rename to IS_ADD_R3_SP for consistency.
	(IS_MOVW_R1): New macro.
	(IS_MOVL_R1): New macro.
	(IS_SUB_R1_SP): New macro.
	(sh_analyze_prologue): Add r1_val local var and initialize to zero.
	Use IS_MOVW_R1, IS_MOVL_R1, and IS_SUB_R1_SP to recognize use of
	stack allocation via constant loaded into r1.
  
Index: sh-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sh-tdep.c,v
retrieving revision 1.165
diff -c -p -r1.165 sh-tdep.c
*** sh-tdep.c	20 Feb 2004 00:16:16 -0000	1.165
--- sh-tdep.c	20 Feb 2004 06:32:56 -0000
*************** sh_breakpoint_from_pc (CORE_ADDR *pcptr,
*** 330,341 ****
     r15+imm-->r15 */
  #define IS_ADD_IMM_SP(x) 	(((x) & 0xff00) == 0x7f00)
  
! #define IS_MOV_R3(x) 		(((x) & 0xff00) == 0x1a00)
  #define IS_SHLL_R3(x)		((x) == 0x4300)
  
  /* ADD r3,r15      0011111100111100
     r15+r3-->r15 */
! #define IS_ADD_R3SP(x)		((x) == 0x3f3c)
  
  /* FMOV.S FRm,@-Rn  Rn-4-->Rn, FRm-->(Rn)     1111nnnnmmmm1011
     FMOV DRm,@-Rn    Rn-8-->Rn, DRm-->(Rn)     1111nnnnmmm01011
--- 330,355 ----
     r15+imm-->r15 */
  #define IS_ADD_IMM_SP(x) 	(((x) & 0xff00) == 0x7f00)
  
! /* MOV #imm,r3     11100011iiiiiiii
!    imm-->r3 */
! #define IS_MOV_IMM_R3(x)	(((x) & 0xff00) == 0xe300)
  #define IS_SHLL_R3(x)		((x) == 0x4300)
  
  /* ADD r3,r15      0011111100111100
     r15+r3-->r15 */
! #define IS_ADD_R3_SP(x)		((x) == 0x3f3c)
! 
! /* MOV.W @(disp, pc), r1     10010001dddddddd
!    (disp * 2 + pc + 4)-->r1 */
! #define IS_MOVW_R1(x)		(((x) & 0xff00) == 0x9100)
! 
! /* MOV.L @(disp, pc), r1     11010001dddddddd
!    (disp * 4 + pc + 4)-->r1 */
! #define IS_MOVL_R1(x)		(((x) & 0xff00) == 0xd100)
! 
! /* SUB r1,r15		     00111111100011000
!    r15-r1-->r15 */
! #define IS_SUB_R1_SP(x)		((x) == 0x3f18)
  
  /* FMOV.S FRm,@-Rn  Rn-4-->Rn, FRm-->(Rn)     1111nnnnmmmm1011
     FMOV DRm,@-Rn    Rn-8-->Rn, DRm-->(Rn)     1111nnnnmmm01011
*************** sh_analyze_prologue (CORE_ADDR pc, CORE_
*** 394,399 ****
--- 408,414 ----
    CORE_ADDR opc;
    int offset;
    int sav_offset = 0;
+   int r1_val = 0;
    int r3_val = 0;
    int reg, sav_reg = -1;
  
*************** sh_analyze_prologue (CORE_ADDR pc, CORE_
*** 415,421 ****
  	  cache->saved_regs[PR_REGNUM] = cache->sp_offset;
  	  cache->sp_offset += 4;
  	}
!       else if (IS_MOV_R3 (inst))
  	{
  	  r3_val = ((inst & 0xff) ^ 0x80) - 0x80;
  	}
--- 430,436 ----
  	  cache->saved_regs[PR_REGNUM] = cache->sp_offset;
  	  cache->sp_offset += 4;
  	}
!       else if (IS_MOV_IMM_R3 (inst))
  	{
  	  r3_val = ((inst & 0xff) ^ 0x80) - 0x80;
  	}
*************** sh_analyze_prologue (CORE_ADDR pc, CORE_
*** 423,431 ****
  	{
  	  r3_val <<= 1;
  	}
!       else if (IS_ADD_R3SP (inst))
  	{
  	  cache->sp_offset += -r3_val;
  	}
        else if (IS_ADD_IMM_SP (inst))
  	{
--- 438,458 ----
  	{
  	  r3_val <<= 1;
  	}
!       else if (IS_ADD_R3_SP (inst))
  	{
  	  cache->sp_offset += -r3_val;
+ 	}
+       else if (IS_MOVW_R1 (inst))
+ 	{
+ 	  r1_val = read_memory_integer (pc + 4 + (inst & 0xff) * 2, 2);
+ 	}
+       else if (IS_MOVL_R1 (inst))
+ 	{
+ 	  r1_val = read_memory_integer (pc + 4 + (inst & 0xff) * 4, 4);
+ 	}
+       else if (IS_SUB_R1_SP (inst))
+ 	{
+ 	  cache->sp_offset += r1_val;
  	}
        else if (IS_ADD_IMM_SP (inst))
  	{

[RFA] sh-tdep.c: New patch solving gdb1291.exp (was Re: [RFA] Fix PR tdep/1291, SH prologue scanning bug)

[Corinna Vinschen]

On Feb 19 23:44, Fred Fish wrote:
> This patch fixes the bug reported in PR 1291.  It is based on the suggested
> patch included in the PR.  I believe it is small enough to not need a
> copyright assignment, but recent events may have changed that.  :-(
> 
> -Fred
> 
> 2004-02-19  Fred Fish  <fnf@redhat.com>
> 
> 	Fix for PR tdep/1291 as suggested by inaba@src.ricoh.co.jp
> 	* sh-tdep.c (IS_MOV_R3): Rename to IS_MOV_IMM_R3 and fix pattern.
> 	(IS_ADD_R3SP): Rename to IS_ADD_R3_SP for consistency.
> 	(IS_MOVW_R1): New macro.
> 	(IS_MOVL_R1): New macro.
> 	(IS_SUB_R1_SP): New macro.
> 	(sh_analyze_prologue): Add r1_val local var and initialize to zero.
> 	Use IS_MOVW_R1, IS_MOVL_R1, and IS_SUB_R1_SP to recognize use of
> 	stack allocation via constant loaded into r1.

I've looked into this one and found that there's a very simple patch
to solve that issue.  Basically the evaluation of the memory address
to access in PC relative addressing was misbehaving.  The below patch
evaluates the PC relative memory location now exactly according to the
descriptions of the PC relative addressing modes with 8 bit displacement
in the official SH documentation:

  FOO.w @(disp:8,PC):

    displacement = (instruction & 0xff) << 1;
    address = (PC + 4) + displacement;

  FOO.l @(disp:8,PC):

    displacement = (instruction & 0xff) << 2;
    address = ((PC & 0xfffffffc) + 4) + displacement;

I checked the entire testsuite against sh2, sh2e, sh3, sh4 and sh4-nofpu.
In all cases, the FAIL count has been reduced by exactly one, the FAIL
from gdb1291.exp.

Is that ok to checkin?


Corinna

	* sh-tdep.c (sh_analyze_prologue): Align PC relative addressing
	to official documentation.

Index: sh-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sh-tdep.c,v
retrieving revision 1.165
diff -u -p -r1.165 sh-tdep.c
--- sh-tdep.c	20 Feb 2004 00:16:16 -0000	1.165
+++ sh-tdep.c	25 Feb 2004 15:47:32 -0000
@@ -440,9 +440,9 @@ sh_analyze_prologue (CORE_ADDR pc, CORE_
 	      if (reg < 14)
 		{
 		  sav_reg = reg;
-		  offset = (((inst & 0xff) ^ 0x80) - 0x80) << 1;
+		  offset = (inst & 0xff) << 1;
 		  sav_offset =
-		    read_memory_integer (((pc + 4) & ~3) + offset, 2);
+		    read_memory_integer ((pc + 4) + offset, 2);
 		}
 	    }
 	}
@@ -450,13 +450,13 @@ sh_analyze_prologue (CORE_ADDR pc, CORE_
 	{
 	  if (sav_reg < 0)
 	    {
-	      reg = (inst & 0x0f00) >> 8;
+	      reg = GET_TARGET_REG (inst);
 	      if (reg < 14)
 		{
 		  sav_reg = reg;
-		  offset = (((inst & 0xff) ^ 0x80) - 0x80) << 1;
+		  offset = (inst & 0xff) << 2;
 		  sav_offset =
-		    read_memory_integer (((pc + 4) & ~3) + offset, 4);
+		    read_memory_integer (((pc & 0xfffffffc) + 4) + offset, 4);
 		}
 	    }
 	}

-- 
Corinna Vinschen
Cygwin Developer
Red Hat, Inc.

Re: [RFA] sh-tdep.c: New patch solving gdb1291.exp (was Re: [RFA] Fix PR tdep/1291, SH prologue scanning bug)

[Corinna Vinschen]

Ping?

Corinna

On Feb 25 17:33, Corinna Vinschen wrote:
> On Feb 19 23:44, Fred Fish wrote:
> > This patch fixes the bug reported in PR 1291.  It is based on the suggested
> > patch included in the PR.  I believe it is small enough to not need a
> > copyright assignment, but recent events may have changed that.  :-(
> > 
> > -Fred
> > 
> > 2004-02-19  Fred Fish  <fnf@redhat.com>
> > 
> > 	Fix for PR tdep/1291 as suggested by inaba@src.ricoh.co.jp
> > 	* sh-tdep.c (IS_MOV_R3): Rename to IS_MOV_IMM_R3 and fix pattern.
> > 	(IS_ADD_R3SP): Rename to IS_ADD_R3_SP for consistency.
> > 	(IS_MOVW_R1): New macro.
> > 	(IS_MOVL_R1): New macro.
> > 	(IS_SUB_R1_SP): New macro.
> > 	(sh_analyze_prologue): Add r1_val local var and initialize to zero.
> > 	Use IS_MOVW_R1, IS_MOVL_R1, and IS_SUB_R1_SP to recognize use of
> > 	stack allocation via constant loaded into r1.
> 
> I've looked into this one and found that there's a very simple patch
> to solve that issue.  Basically the evaluation of the memory address
> to access in PC relative addressing was misbehaving.  The below patch
> evaluates the PC relative memory location now exactly according to the
> descriptions of the PC relative addressing modes with 8 bit displacement
> in the official SH documentation:
> 
>   FOO.w @(disp:8,PC):
> 
>     displacement = (instruction & 0xff) << 1;
>     address = (PC + 4) + displacement;
> 
>   FOO.l @(disp:8,PC):
> 
>     displacement = (instruction & 0xff) << 2;
>     address = ((PC & 0xfffffffc) + 4) + displacement;
> 
> I checked the entire testsuite against sh2, sh2e, sh3, sh4 and sh4-nofpu.
> In all cases, the FAIL count has been reduced by exactly one, the FAIL
> from gdb1291.exp.
> 
> Is that ok to checkin?
> 
> 
> Corinna
> 
> 	* sh-tdep.c (sh_analyze_prologue): Align PC relative addressing
> 	to official documentation.
> 
> Index: sh-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/sh-tdep.c,v
> retrieving revision 1.165
> diff -u -p -r1.165 sh-tdep.c
> --- sh-tdep.c	20 Feb 2004 00:16:16 -0000	1.165
> +++ sh-tdep.c	25 Feb 2004 15:47:32 -0000
> @@ -440,9 +440,9 @@ sh_analyze_prologue (CORE_ADDR pc, CORE_
>  	      if (reg < 14)
>  		{
>  		  sav_reg = reg;
> -		  offset = (((inst & 0xff) ^ 0x80) - 0x80) << 1;
> +		  offset = (inst & 0xff) << 1;
>  		  sav_offset =
> -		    read_memory_integer (((pc + 4) & ~3) + offset, 2);
> +		    read_memory_integer ((pc + 4) + offset, 2);
>  		}
>  	    }
>  	}
> @@ -450,13 +450,13 @@ sh_analyze_prologue (CORE_ADDR pc, CORE_
>  	{
>  	  if (sav_reg < 0)
>  	    {
> -	      reg = (inst & 0x0f00) >> 8;
> +	      reg = GET_TARGET_REG (inst);
>  	      if (reg < 14)
>  		{
>  		  sav_reg = reg;
> -		  offset = (((inst & 0xff) ^ 0x80) - 0x80) << 1;
> +		  offset = (inst & 0xff) << 2;
>  		  sav_offset =
> -		    read_memory_integer (((pc + 4) & ~3) + offset, 4);
> +		    read_memory_integer (((pc & 0xfffffffc) + 4) + offset, 4);
>  		}
>  	    }
>  	}
> 
> -- 
> Corinna Vinschen
> Cygwin Developer
> Red Hat, Inc.

-- 
Corinna Vinschen
Cygwin Developer
Red Hat, Inc.

Re: [RFA] sh-tdep.c: New patch solving gdb1291.exp (was Re: [RFA] Fix PR tdep/1291, SH prologue scanning bug)

[Elena Zannoni]

Corinna Vinschen writes:
 > On Feb 19 23:44, Fred Fish wrote:
 > > This patch fixes the bug reported in PR 1291.  It is based on the suggested
 > > patch included in the PR.  I believe it is small enough to not need a
 > > copyright assignment, but recent events may have changed that.  :-(
 > > 
 > > -Fred
 > > 
 > > 2004-02-19  Fred Fish  <fnf@redhat.com>
 > > 
 > > 	Fix for PR tdep/1291 as suggested by inaba@src.ricoh.co.jp
 > > 	* sh-tdep.c (IS_MOV_R3): Rename to IS_MOV_IMM_R3 and fix pattern.
 > > 	(IS_ADD_R3SP): Rename to IS_ADD_R3_SP for consistency.
 > > 	(IS_MOVW_R1): New macro.
 > > 	(IS_MOVL_R1): New macro.
 > > 	(IS_SUB_R1_SP): New macro.
 > > 	(sh_analyze_prologue): Add r1_val local var and initialize to zero.
 > > 	Use IS_MOVW_R1, IS_MOVL_R1, and IS_SUB_R1_SP to recognize use of
 > > 	stack allocation via constant loaded into r1.
 > 
 > I've looked into this one and found that there's a very simple patch
 > to solve that issue.  Basically the evaluation of the memory address
 > to access in PC relative addressing was misbehaving.  The below patch
 > evaluates the PC relative memory location now exactly according to the
 > descriptions of the PC relative addressing modes with 8 bit displacement
 > in the official SH documentation:
 > 
 >   FOO.w @(disp:8,PC):
 > 
 >     displacement = (instruction & 0xff) << 1;
 >     address = (PC + 4) + displacement;
 > 
 >   FOO.l @(disp:8,PC):
 > 
 >     displacement = (instruction & 0xff) << 2;
 >     address = ((PC & 0xfffffffc) + 4) + displacement;
 > 
 > I checked the entire testsuite against sh2, sh2e, sh3, sh4 and sh4-nofpu.
 > In all cases, the FAIL count has been reduced by exactly one, the FAIL
 > from gdb1291.exp.
 > 
 > Is that ok to checkin?
 > 


If this fixes PR/1291, it should refer to it in the changelog, and
also the pr should be closed.

otherwise ok.

 > 
 > Corinna
 > 
 > 	* sh-tdep.c (sh_analyze_prologue): Align PC relative addressing
 > 	to official documentation.
 > 

Re: [RFA] sh-tdep.c: New patch solving gdb1291.exp (was Re: [RFA] Fix PR tdep/1291, SH prologue scanning bug)

[Corinna Vinschen]

On Mar  5 17:31, Elena Zannoni wrote:
> If this fixes PR/1291, it should refer to it in the changelog, and
> also the pr should be closed.
> 
> otherwise ok.

Thanks.  Applied and PR is closed.

Corinna

-- 
Corinna Vinschen
Cygwin Developer
Red Hat, Inc.

Re: [RFA] sh-tdep.c: New patch solving gdb1291.exp (was Re: [RFA] Fix PR tdep/1291, SH prologue scanning bug)

[Corinna Vinschen]

On Mar  5 17:31, Elena Zannoni wrote:
> If this fixes PR/1291, it should refer to it in the changelog, and
> also the pr should be closed.
> 
> otherwise ok.

Thanks.  Applied and PR is closed.

Corinna

-- 
Corinna Vinschen
Cygwin Developer
Red Hat, Inc.

Re: [RFA] sh-tdep.c: New patch solving gdb1291.exp (was Re: [RFA] Fix PR tdep/1291, SH prologue scanning bug)

[Corinna Vinschen]

Ping?

Corinna

On Feb 25 17:33, Corinna Vinschen wrote:
> On Feb 19 23:44, Fred Fish wrote:
> > This patch fixes the bug reported in PR 1291.  It is based on the suggested
> > patch included in the PR.  I believe it is small enough to not need a
> > copyright assignment, but recent events may have changed that.  :-(
> > 
> > -Fred
> > 
> > 2004-02-19  Fred Fish  <fnf@redhat.com>
> > 
> > 	Fix for PR tdep/1291 as suggested by inaba@src.ricoh.co.jp
> > 	* sh-tdep.c (IS_MOV_R3): Rename to IS_MOV_IMM_R3 and fix pattern.
> > 	(IS_ADD_R3SP): Rename to IS_ADD_R3_SP for consistency.
> > 	(IS_MOVW_R1): New macro.
> > 	(IS_MOVL_R1): New macro.
> > 	(IS_SUB_R1_SP): New macro.
> > 	(sh_analyze_prologue): Add r1_val local var and initialize to zero.
> > 	Use IS_MOVW_R1, IS_MOVL_R1, and IS_SUB_R1_SP to recognize use of
> > 	stack allocation via constant loaded into r1.
> 
> I've looked into this one and found that there's a very simple patch
> to solve that issue.  Basically the evaluation of the memory address
> to access in PC relative addressing was misbehaving.  The below patch
> evaluates the PC relative memory location now exactly according to the
> descriptions of the PC relative addressing modes with 8 bit displacement
> in the official SH documentation:
> 
>   FOO.w @(disp:8,PC):
> 
>     displacement = (instruction & 0xff) << 1;
>     address = (PC + 4) + displacement;
> 
>   FOO.l @(disp:8,PC):
> 
>     displacement = (instruction & 0xff) << 2;
>     address = ((PC & 0xfffffffc) + 4) + displacement;
> 
> I checked the entire testsuite against sh2, sh2e, sh3, sh4 and sh4-nofpu.
> In all cases, the FAIL count has been reduced by exactly one, the FAIL
> from gdb1291.exp.
> 
> Is that ok to checkin?
> 
> 
> Corinna
> 
> 	* sh-tdep.c (sh_analyze_prologue): Align PC relative addressing
> 	to official documentation.
> 
> Index: sh-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/sh-tdep.c,v
> retrieving revision 1.165
> diff -u -p -r1.165 sh-tdep.c
> --- sh-tdep.c	20 Feb 2004 00:16:16 -0000	1.165
> +++ sh-tdep.c	25 Feb 2004 15:47:32 -0000
> @@ -440,9 +440,9 @@ sh_analyze_prologue (CORE_ADDR pc, CORE_
>  	      if (reg < 14)
>  		{
>  		  sav_reg = reg;
> -		  offset = (((inst & 0xff) ^ 0x80) - 0x80) << 1;
> +		  offset = (inst & 0xff) << 1;
>  		  sav_offset =
> -		    read_memory_integer (((pc + 4) & ~3) + offset, 2);
> +		    read_memory_integer ((pc + 4) + offset, 2);
>  		}
>  	    }
>  	}
> @@ -450,13 +450,13 @@ sh_analyze_prologue (CORE_ADDR pc, CORE_
>  	{
>  	  if (sav_reg < 0)
>  	    {
> -	      reg = (inst & 0x0f00) >> 8;
> +	      reg = GET_TARGET_REG (inst);
>  	      if (reg < 14)
>  		{
>  		  sav_reg = reg;
> -		  offset = (((inst & 0xff) ^ 0x80) - 0x80) << 1;
> +		  offset = (inst & 0xff) << 2;
>  		  sav_offset =
> -		    read_memory_integer (((pc + 4) & ~3) + offset, 4);
> +		    read_memory_integer (((pc & 0xfffffffc) + 4) + offset, 4);
>  		}
>  	    }
>  	}
> 
> -- 
> Corinna Vinschen
> Cygwin Developer
> Red Hat, Inc.

-- 
Corinna Vinschen
Cygwin Developer
Red Hat, Inc.

Re: [RFA] sh-tdep.c: New patch solving gdb1291.exp (was Re: [RFA] Fix PR tdep/1291, SH prologue scanning bug)

[Elena Zannoni]

Corinna Vinschen writes:
 > On Feb 19 23:44, Fred Fish wrote:
 > > This patch fixes the bug reported in PR 1291.  It is based on the suggested
 > > patch included in the PR.  I believe it is small enough to not need a
 > > copyright assignment, but recent events may have changed that.  :-(
 > > 
 > > -Fred
 > > 
 > > 2004-02-19  Fred Fish  <fnf@redhat.com>
 > > 
 > > 	Fix for PR tdep/1291 as suggested by inaba@src.ricoh.co.jp
 > > 	* sh-tdep.c (IS_MOV_R3): Rename to IS_MOV_IMM_R3 and fix pattern.
 > > 	(IS_ADD_R3SP): Rename to IS_ADD_R3_SP for consistency.
 > > 	(IS_MOVW_R1): New macro.
 > > 	(IS_MOVL_R1): New macro.
 > > 	(IS_SUB_R1_SP): New macro.
 > > 	(sh_analyze_prologue): Add r1_val local var and initialize to zero.
 > > 	Use IS_MOVW_R1, IS_MOVL_R1, and IS_SUB_R1_SP to recognize use of
 > > 	stack allocation via constant loaded into r1.
 > 
 > I've looked into this one and found that there's a very simple patch
 > to solve that issue.  Basically the evaluation of the memory address
 > to access in PC relative addressing was misbehaving.  The below patch
 > evaluates the PC relative memory location now exactly according to the
 > descriptions of the PC relative addressing modes with 8 bit displacement
 > in the official SH documentation:
 > 
 >   FOO.w @(disp:8,PC):
 > 
 >     displacement = (instruction & 0xff) << 1;
 >     address = (PC + 4) + displacement;
 > 
 >   FOO.l @(disp:8,PC):
 > 
 >     displacement = (instruction & 0xff) << 2;
 >     address = ((PC & 0xfffffffc) + 4) + displacement;
 > 
 > I checked the entire testsuite against sh2, sh2e, sh3, sh4 and sh4-nofpu.
 > In all cases, the FAIL count has been reduced by exactly one, the FAIL
 > from gdb1291.exp.
 > 
 > Is that ok to checkin?
 > 


If this fixes PR/1291, it should refer to it in the changelog, and
also the pr should be closed.

otherwise ok.

 > 
 > Corinna
 > 
 > 	* sh-tdep.c (sh_analyze_prologue): Align PC relative addressing
 > 	to official documentation.
 >