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 copro *DIS command calculates negative offset ADR addresses incorrectly? #172

Closed mincebert closed 1 year ago

mincebert commented 1 year ago

I'm not an expert on ARM but I think the *DIS command in the ARMv7 Native Coprocessor calculates the address used in an ADR instruction incorrectly, when the offset is negative.

For example, if I assemble (in BBC BASIC V) at &9018:

.data
EQUW &87654321
ADR R1,data

Then I do *DIS 9018, I get:

>*DIS 9018
00009018 87654321 STRBHI r4, [r5, -r1, LSR #6]!
0000901C E24F100C ADR r1, &9030

... the ADR instruction is actually a SUB with an offset of &C, minus 8 for the pipeline so should be &9018; I think the offset is always being added and not subtracted, when the instruction is determined to be an ADR.

By comparison, *MEMORYI on RISC OS gives (assembled at &901C):

>*MEMORYI 901C
0000901C : !Ceá : 87654321 : STRHIB  R4,[R5,-R1,LSR #6]!
00009020 : ..O‚ : E24F100C : ADR     R1,&0000901C

... same encoding but the address is correctly shown as &901C (in this case).

hoglet67 commented 1 year ago

Disassembling this with an Online Disassembler I get:

0x0000000000009018:  87 65 43 21    strbhi r4, [r5, -r1, lsr #6]!
0x000000000000901c:  E2 4F 10 0C    sub    r1, pc, #0xc

But I agree there is something wrong here.

dp111 commented 1 year ago

The issue is the disassembler is using banch calculations in the table [I_ADR] = {"cdb"}," in armv7-tbl.c which is only good for forward address calculations.

On Wed, 21 Jun 2023 at 07:42, David Banks @.***> wrote:

Disassembling this with an Online Disassembler https://shell-storm.org/online/Online-Assembler-and-Disassembler/?opcodes=87654321%0D%0AE24F100C&arch=arm&endianness=big&baddr=0x00009018&dis_with_addr=True&dis_with_raw=True&dis_with_ins=True#disassembly I get:

0x0000000000009018: 87 65 43 21 strbhi r4, [r5, -r1, lsr #6]! 0x000000000000901c: E2 4F 10 0C sub r1, pc, #0xc

But I agree there is something wrong here.

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

dp111 commented 1 year ago

So I think in the 'b' case in darm.c ( line 463) the d->U flag needs to be used to select if the (8+ d->imm) is subtracted instead of added.

On Wed, 21 Jun 2023 at 07:49, Dominic Plunkett @.***> wrote:

The issue is the disassembler is using banch calculations in the table [I_ADR] = {"cdb"}," in armv7-tbl.c which is only good for forward address calculations.

On Wed, 21 Jun 2023 at 07:42, David Banks @.***> wrote:

Disassembling this with an Online Disassembler https://shell-storm.org/online/Online-Assembler-and-Disassembler/?opcodes=87654321%0D%0AE24F100C&arch=arm&endianness=big&baddr=0x00009018&dis_with_addr=True&dis_with_raw=True&dis_with_ins=True#disassembly I get:

0x0000000000009018: 87 65 43 21 strbhi r4, [r5, -r1, lsr #6]! 0x000000000000901c: E2 4F 10 0C sub r1, pc, #0xc

But I agree there is something wrong here.

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

hoglet67 commented 1 year ago

OK, I'll fix this now in hognose-fixes and merge across to indigo-dev.

Dominic, can I also merge indigo-dev-dp back to indigo-dev?

dp111 commented 1 year ago

Yes I'm happy for indigo-dev-dp to be merged into indigo-dev

On Wed, 21 Jun 2023 at 10:21, David Banks @.***> wrote:

OK, I'll fix this now in hognose-fixes and merge across to indigo-dev.

Dominic, can I also merge indigo-dev-dp back to indigo-dev?

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

hoglet67 commented 1 year ago

OK, this should now be fixed in hognose-fixes: https://github.com/hoglet67/PiTubeDirect/releases/tag/hognose-fixes