Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARM Native copro *DIS command calculates negative offset ADR addresses incorrectly? #172

Closed
mincebert opened this issue Jun 20, 2023 · 6 comments
Labels

Comments

@mincebert
Copy link
Contributor

mincebert commented Jun 20, 2023

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
Copy link
Owner

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
Copy link
Collaborator

dp111 commented Jun 21, 2023 via email

@dp111
Copy link
Collaborator

dp111 commented Jun 21, 2023 via email

@hoglet67
Copy link
Owner

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?

@hoglet67 hoglet67 added the bug label Jun 21, 2023
@dp111
Copy link
Collaborator

dp111 commented Jun 21, 2023 via email

hoglet67 added a commit that referenced this issue Jun 21, 2023
Change-Id: Icbfd68adad233ed99a552876214e0ddf37060d2d
@hoglet67
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants