hoglet67 / PiTubeDirect

Bare-metal Raspberry Pi project that attaches to the Acorn TUBE interface and emulates many BBC Micro Co Processors
GNU General Public License v3.0
187 stars 23 forks source link

ARM Native *DIS always aligning addresses on 32-bit boundary #173

Closed mincebert closed 1 year ago

mincebert commented 1 year ago

Sorry — another problem with *DIS and ADR on the ARM Native copro in the commit c0ddeb5, I think! It seems to be aligning calculated addresses on a 32-bit boundary. For example, if I do:

00009054                    OPT opt%
00009054 E28F100C           ADR R1,start
00009058 E28F200F           ADR R2,end
0000905C E58F2000           STR R2,addr
00009060 E1A0F00E           MOV PC,LR
00009064          .addr     
00009064 00000000           EQUD 0
00009068          .start    
00009068                    EQUS "7 BYTES"
0000906F          .end

Then I disassemble with *DIS, the address for ADR R2,end shows as &906C when it is actually &906F:

>*DIS 9054
00009054 E28F100C ADR r1, &9068
00009058 E28F200F ADR r2, &906c
0000905C E58F2000 STR r2, [PC]
00009060 E1A0F00E MOV PC, LR
00009064 00000000 ANDEQ r0, r0, r0
00009068 59422037 ***
0000906C 25534554 LDRBCS r4, [r3, #-1364]
00009070 646F632D STRBTVS r6, [PC], #-813
00009074 FF0D2565 ***
...

And, if I print the stored address, that confirms it should be &906F:

>CALL &9054
>P.~!addr
      906F

I'm wondering if the code that calculates it is aligning it for branches but shouldn't do that when the address is for data, but you'll understand what's going on there much more than me!

(As an aside, perhaps the addresses for ADR printed in the disassembly should be in upper case to match the addresses in the left column, but maybe that's a deliberate choice.)

Thanks again!

mincebert commented 1 year ago

In addition, the STR r2, [PC] should arguably be disassembled as STR r2,&9064, as that’s probably more useful and what the programmer meant.

hoglet67 commented 1 year ago

No need to appologise. Thanks for reporting these.

I'll make sure they get fixed in Indigo.

(I'd rather not keep re-releasing the hognose-fixes branch unless the issue is critical)

Dave

dp111 commented 1 year ago

I do wonder if we should change to using : https://gitlab.riscosopen.org/RiscOS/Library/-/tree/master/Misc/decgen

On Sun, 2 Jul 2023 at 11:40, David Banks @.***> wrote:

No need to appologise. Thanks for reporting these.

I'll make sure they get fixed in Indigo.

(I'd rather not keep re-releasing the hognose-fixes branch unless the issue is critical)

Dave

— Reply to this email directly, view it on GitHub https://github.com/hoglet67/PiTubeDirect/issues/173#issuecomment-1616587072, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEVVFIRSTWF4DBYA2HPDQH3XOFF2HANCNFSM6AAAAAAZ3ACF6Y . You are receiving this because you are subscribed to this thread.Message ID: @.***>

mincebert commented 1 year ago

I haven't ploughed through that table but I note that *DIS will show LDR Rn,addr as LDR Rn, [PC, #offset], which is also probably not the most intuitive way to show it, so maybe that would help this, too.

dp111 commented 1 year ago

Fixed in indigo-alpha2. Leave using decgen till another time as it is bit of work to integrate.