diff mbox series

RX: Incorrect output for displacement of subtruct expression

Message ID 20241226035313.1691591-1-ysato@users.sourceforge.jp
State New
Headers
Series RX: Incorrect output for displacement of subtruct expression |

Commit Message

Yoshinori Sato Dec. 26, 2024, 3:53 a.m. UTC 
  Assembling the following code results in incorrect displacement values:
1: mov.L 2f-1b[r0],r0

If you write a subtraction formula in the displacement field, the value
will simply be the result of the subtraction, which will not be the
expected value for word and longword sizes.

gas/
	* config/rx-parse.y (displacement): use BFD_RELOC_RX_GPREL in subtract expression.
	* testsuite/gas/rx/mov.sm: Add subtruct expression displacement.
	* testsuite/gas/rx/mov.d: Likewise.

Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
---
 gas/config/rx-parse.y       | 35 ++++++++++++-----------------------
 gas/testsuite/gas/rx/mov.d  |  3 +++
 gas/testsuite/gas/rx/mov.sm |  7 +++++++
 3 files changed, 22 insertions(+), 23 deletions(-)

Comments

Oleg Endo Dec. 26, 2024, 7:53 a.m. UTC | #1 
On Thu, 2024-12-26 at 12:53 +0900, Yoshinori Sato wrote:
> Assembling the following code results in incorrect displacement values:
> 1: mov.L 2f-1b[r0],r0
> 
> If you write a subtraction formula in the displacement field, the value
> will simply be the result of the subtraction, which will not be the
> expected value for word and longword sizes.
> 
> gas/
> 	* config/rx-parse.y (displacement): use BFD_RELOC_RX_GPREL in subtract expression.
> 	* testsuite/gas/rx/mov.sm: Add subtruct expression displacement.
> 	* testsuite/gas/rx/mov.d: Likewise.
> 

Hi!

Wow .. how did you find this bug?  I'm curious because I've been using an RX
system with GCC 8 and older binutils for several years.

Thanks for posting the patch!

Best regards,
Oleg Endo
Yoshinori Sato Dec. 27, 2024, 2:11 p.m. UTC | #2 
On Thu, 26 Dec 2024 16:53:59 +0900,
Oleg Endo wrote:
> 
> 
> On Thu, 2024-12-26 at 12:53 +0900, Yoshinori Sato wrote:
> > Assembling the following code results in incorrect displacement values:
> > 1: mov.L 2f-1b[r0],r0
> > 
> > If you write a subtraction formula in the displacement field, the value
> > will simply be the result of the subtraction, which will not be the
> > expected value for word and longword sizes.
> > 
> > gas/
> > 	* config/rx-parse.y (displacement): use BFD_RELOC_RX_GPREL in subtract expression.
> > 	* testsuite/gas/rx/mov.sm: Add subtruct expression displacement.
> > 	* testsuite/gas/rx/mov.d: Likewise.
> > 
> 
> Hi!
> 
> Wow .. how did you find this bug?  I'm curious because I've been using an RX
> system with GCC 8 and older binutils for several years.

I am trying to get FDPIC support in RX.
gcc, which supports this FDPIC, generated the following code for a switch
statement:

1:      mvfc    pc, r3
	shll    #2, r14
        add     r3, r14
        mov.L   .L282 - 1b[r14], r14

1:      bra     r14
        .balign 4
.L282:
        .long .L284 - 1b
        .long .L280 - 1b
        .long .L280 - 1b

I investigated why this code wasn't working as expected and discovered that it
was due to this bug.
This kind of code is not output by the release version of gcc, and even when
written in assembler, it is not written in this way, so I don't think anyone
noticed the bug.

> Thanks for posting the patch!
> 
> Best regards,
> Oleg Endo
>
Oleg Endo Dec. 28, 2024, 12:40 a.m. UTC | #3 
On Fri, 2024-12-27 at 23:11 +0900, Yoshinori Sato wrote:
> 
> I am trying to get FDPIC support in RX.
> gcc, which supports this FDPIC, generated the following code for a switch
> statement:
> 
> 1:      mvfc    pc, r3
> 	shll    #2, r14
>         add     r3, r14
>         mov.L   .L282 - 1b[r14], r14
> 
> 1:      bra     r14
>         .balign 4
> .L282:
>         .long .L284 - 1b
>         .long .L280 - 1b
>         .long .L280 - 1b
> 
> I investigated why this code wasn't working as expected and discovered that it
> was due to this bug.
> This kind of code is not output by the release version of gcc, and even when
> written in assembler, it is not written in this way, so I don't think anyone
> noticed the bug.

Very nice detective work.  Thanks!

Best regards,
Oleg Endo
diff mbox series

Patch

diff --git a/gas/config/rx-parse.y b/gas/config/rx-parse.y
index a8bead28d21..fa632f90d00 100644
--- a/gas/config/rx-parse.y
+++ b/gas/config/rx-parse.y
@@ -1934,32 +1934,21 @@  displacement (expressionS exp, int msize)
   valueT val;
   int vshift = 0;
 
-  if (exp.X_op == O_symbol
-      && exp.X_md)
+  if ((exp.X_op == O_symbol && exp.X_md == BFD_RELOC_GPREL16) ||
+      (exp.X_op == O_subtract))
     {
-      switch (exp.X_md)
+      switch (msize)
 	{
-	case BFD_RELOC_GPREL16:
-	  switch (msize)
-	    {
-	    case BSIZE:
-	      exp.X_md = BFD_RELOC_RX_GPRELB;
-	      break;
-	    case WSIZE:
-	      exp.X_md = BFD_RELOC_RX_GPRELW;
-	      break;
-	    case LSIZE:
-	      exp.X_md = BFD_RELOC_RX_GPRELL;
-	      break;
-	    }
-	  O2 (exp);
-	  return 2;
+	case BSIZE:
+	  exp.X_md = BFD_RELOC_RX_GPRELB;
+	  break;
+	case WSIZE:
+	  exp.X_md = BFD_RELOC_RX_GPRELW;
+	  break;
+	case LSIZE:
+	  exp.X_md = BFD_RELOC_RX_GPRELL;
+	  break;
 	}
-    }
-
-  if (exp.X_op == O_subtract)
-    {
-      exp.X_md = BFD_RELOC_RX_DIFF;
       O2 (exp);
       return 2;
     }
diff --git a/gas/testsuite/gas/rx/mov.d b/gas/testsuite/gas/rx/mov.d
index 276b6d9d898..4c5d2389746 100644
--- a/gas/testsuite/gas/rx/mov.d
+++ b/gas/testsuite/gas/rx/mov.d
@@ -528,3 +528,6 @@  Disassembly of section .*:
  [0-9a-f]+:	fd 2e 0f                      	mov.l	\[-r0\], r15
  [0-9a-f]+:	fd 2e f0                      	mov.l	\[-r15\], r0
  [0-9a-f]+:	fd 2e ff                      	mov.l	\[-r15\], r15
+ [0-9a-f]+:	ce 00 04 00                   	mov.b	4\[r0\], r0
+ [0-9a-f]+:	de 00 02 00                   	mov.w	4\[r0\], r0
+ [0-9a-f]+:	ee 00 01 00                   	mov.l	4\[r0\], r0
diff --git a/gas/testsuite/gas/rx/mov.sm b/gas/testsuite/gas/rx/mov.sm
index 8bebbf5908a..03bec4a7bc1 100644
--- a/gas/testsuite/gas/rx/mov.sm
+++ b/gas/testsuite/gas/rx/mov.sm
@@ -31,3 +31,10 @@ 
 	mov{bwl}	[{reg}+],{reg}
 	mov{bwl}	{reg},[-{reg}]
 	mov{bwl}	[-{reg}],{reg}
+
+1:	mov.B		2f - 1b[r0], r0
+2:
+1:	mov.W		2f - 1b[r0], r0
+2:
+1:	mov.L		2f - 1b[r0], r0
+2: