Range checking for 65816 mode when abs addresses are assumed rather than known

[bbbradsmith]

This is a possible resolution to issue: #879

It looks like a satisfactory resolution to me, but I could use code review and/or assistance in testing before I would be confident that this should be merged.

.

What I found was that when the address size of an operand was not sufficiently known in 65816 mode, it would use GenWordExpr which creates an EXPR_WORD0 type expression that automatically truncates to 16-bit. Otherwise if it is known at that line, the assembler will just generate an instruction of known size.

See:

cc65/src/ca65/instr.c

Line 1220 in 392e6e1

Emit2 (A->Opcode, GenWordExpr (A->Expr));

I believe the idea is that by truncating it, the address was prepared in advance for the linker, which in 65816 correctly has to truncate near addresses to remove their bank byte. However, this disabled the ability to check if this was a valid truncation.

My replacement creates a new expression type EXPR_NEARADDR, which the linker will treat the same as EXPR_WORD0 as a 16-bit truncation, but now the assembler can range check it in SegDone (unless the relaxed checking mode is used).

This change seems to correctly catch the error of accidentally making a reference to a symbol from a far segment before that segment is declared. If the instruction was assumed near, but the symbol was later declared far, this now gets caught during SegDone.

One caveat: a later (or prior) export via .global + : far does not override the address modifier of the symbol's containing segment. I think this is correct behaviour, so that near addresses used within the file can be exported globally, but the consequence might be unintuitive: a .global : far will cause a symbol to assume far addressing until its containing segment is reached, which overrides it. (This feels correct to me, but still weird to think through.)

.

Also, was it appropriate to add the EXPR_NEARADDR enum in the middle of the list, or should it have been appended to the end? (Would that invalidate existing object files?)

[oliverschmidt]

Unfortunately I can't add any value to this topic - I have no idea about the 65816 at all.

The only thing I can say is that the commits provided so far don't look so large and/or complex that I'd have to reject them for further-code-base-maintenance reasons. However, I'd like to see others comment on the question if the proposed change is correct/helpful/desirable/.

[oliverschmidt]

Last call: If anybody has reasons against merging this pull request, please speak up and provide an explanation. Otherwise I'll merge it in the next days even if I don't understand it - simply because I don't see a reason not to do so...

[oliverschmidt]

Is #879 supposed to be closed?

[bbbradsmith]

Yes, this fixed that issue. I will close it. Thanks!

[oliverschmidt]

It's me who has to thank for the fix!