Bug 8387 - sim: arm: Thumb add pc,rn should not word align pc
Summary: sim: arm: Thumb add pc,rn should not word align pc
Status: NEW
Alias: None
Product: gdb
Classification: Unclassified
Component: sim (show other bugs)
Version: 5.3
: P3 enhancement
Target Milestone: ---
Assignee: Nick Clifton
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-07-14 12:08 UTC by dmcq
Modified: 2022-11-02 16:46 UTC (History)
4 users (show)

See Also:
Host:
Target: arm-*
Build:
Last reconfirmed:
Project(s) to access:
ssh public key:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description dmcq 2003-07-14 12:08:00 UTC
[Converted from Gnats 1282]

The Thumb 'add pc,rn' instruction should I believe jump to the address of the add op+4+contents of rn. However the simulator rounds the result down to a word boundary. I've tested on an ARM7 processor and it gave me my expected results.

The instruction was used to avoid having fixups or affecting r14 in a long branch.

Release:
GDB-5.3

Environment:
I'm running the ARM simulator in my own environment and don't actually use GDB or any *nix.

How-To-Repeat:
If you could run this thumb subroutine somehow:

    .align 4
    mov r0,#2
    add r15, r0
    add r0,#2
    add r0, #2
    add r0, #2
    add r0, #2
    mov r15, r14

it should return 6 and not 8.
Comment 1 dmcq 2003-07-14 12:08:00 UTC
Fix:
I've found this using the latest versions in

    src/sim/arm

The logical place for a fix is

    thumbemu.c 1.5

in the code for 'Format 5' ADD, but it would be out of step with how that's written.

I've attached a proposed fix in diff.txt where the arm2 directory holds the new code for

    armemu.c 1.30
    armemu.h 1.15

where I instead change the code to only do the rounding down in those thumb operation equivalents that should round down, i.e. ADD rn,pc,#imm and LDR rn,[pc,#imm] - so it is a bit less certain of just affecting what should be affected than doing something in thumbemu.c.
Comment 2 dmcq2002 2003-07-14 14:04:25 UTC
From: =?iso-8859-1?q?David=20McQUillan?= <dmcq2002@yahoo.co.uk>
To: gdb-gnats@sources.redhat.com
Cc:  
Subject: Re: sim/1282: Thumb add pc,rn should not word align pc
Date: Mon, 14 Jul 2003 15:04:25 +0100 (BST)

 --0-1719933879-1058191465=:27518
 Content-Type: text/plain; charset=iso-8859-1
 Content-Transfer-Encoding: 8bit
 Content-Id: 
 Content-Disposition: inline
 
 Sorry, here is that diff file I mentioned in my
 report.
 I'm also just myself at this yahoo mailbox, please
 discount that company mailbox I used as any
 communication using that should have some enormous
 legal disclaimer. 
 
 David McQuillan
 
 
 
 ________________________________________________________________________
 Want to chat instantly with your online friends?  Get the FREE Yahoo!
 Messenger http://uk.messenger.yahoo.com/
 --0-1719933879-1058191465=:27518
 Content-Type: text/plain; name="diff.txt"
 Content-Description: diff.txt
 Content-Disposition: inline; filename="diff.txt"
 
 diff -pcw arm/armemu.c arm2/armemu.c
 *** arm/armemu.c	Mon Jul 14 12:09:42 2003
 --- arm2/armemu.c	Mon Jul 14 14:30:38 2003
 *************** check_PMUintr:
 *** 2015,2021 ****
   	      break;
   
   	    case 0x28:		/* ADD immed */
 ! 	      dest = LHS + DPImmRHS;
   	      WRITEDEST (dest);
   	      break;
   
 --- 2015,2022 ----
   	      break;
   
   	    case 0x28:		/* ADD immed */
 ! 	      /* Word align pc for Thumb equivalent. */
 ! 	      dest = LHSTW + DPImmRHS;
   	      WRITEDEST (dest);
   	      break;
   
 *************** check_PMUintr:
 *** 2473,2479 ****
   	      break;
   
   	    case 0x59:		/* Load Word, No WriteBack, Pre Inc, Immed.  */
 ! 	      (void) LoadWord (state, instr, LHS + LSImmRHS);
   	      break;
   
   	    case 0x5a:		/* Store Word, WriteBack, Pre Inc, Immed.  */
 --- 2474,2481 ----
   	      break;
   
   	    case 0x59:		/* Load Word, No WriteBack, Pre Inc, Immed.  */
 ! 	      /* Word align pc for Thumb equivalent. */
 ! 	      (void) LoadWord (state, instr, LHSTW + LSImmRHS);
   	      break;
   
   	    case 0x5a:		/* Store Word, WriteBack, Pre Inc, Immed.  */
 diff -pcw arm/armemu.h arm2/armemu.h
 *** arm/armemu.h	Mon Jul 14 12:10:04 2003
 --- arm2/armemu.h	Mon Jul 14 14:28:16 2003
 *************** extern ARMword isize;
 *** 312,324 ****
   #define DEST (state->Reg[DESTReg])
   
   #ifdef MODE32
   #ifdef MODET
 ! #define LHS ((LHSReg == 15) ? (state->Reg[15] & 0xFFFFFFFC): (state->Reg[LHSReg]))
   #else
 ! #define LHS (state->Reg[LHSReg])
   #endif
   #else
 ! #define LHS ((LHSReg == 15) ? R15PC : (state->Reg[LHSReg]))
   #endif
   
   #define MULDESTReg (BITS (16, 19))
 --- 312,330 ----
   #define DEST (state->Reg[DESTReg])
   
   #ifdef MODE32
 + #define LHS (state->Reg[LHSReg])
 + #else
 + #define LHS ((LHSReg == 15) ? R15PC : (state->Reg[LHSReg]) )
 + #endif
 + 
 + #ifdef MODE32
   #ifdef MODET
 ! #define LHSTW ((LHSReg == 15) ? (state->Reg[15] & 0xFFFFFFFC): (state->Reg[LHSReg]))
   #else
 ! #define LHSTW (state->Reg[LHSReg])
   #endif
   #else
 ! #define LHSTW ((LHSReg == 15) ? R15PC : (state->Reg[LHSReg]) )
   #endif
   
   #define MULDESTReg (BITS (16, 19))
 
 --0-1719933879-1058191465=:27518--
Comment 3 Luis Machado 2022-11-02 11:33:53 UTC
Hi Nick, are you still planning to address this?
Comment 4 Nick Clifton 2022-11-02 16:46:59 UTC
(In reply to Luis Machado from comment #3)
> Hi Nick, are you still planning to address this?

No.  I looked at the proposed patch, and I can see how it would fix the problem in the testcase shown.  But I feel that it is likely to break other code sequences because of its change to the LHS definition.  Unfortunately my knowledge of the ARM ISA is now so out of date, that I am not sure that I can reasonably review the patch.