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

Range error in constant #75

Open
andrew-davie opened this issue Aug 24, 2020 · 4 comments
Open

Range error in constant #75

andrew-davie opened this issue Aug 24, 2020 · 4 comments
Labels
bug Something isn't working

Comments

@andrew-davie
Copy link
Member

dasm special-cases negative immediates in, for example...

VAL = -1
  lda #VAL. ; assembles OK

Although numbers are stored internally as 32 bits, and by rights this would be equivalent to...

  lda #$FFFFFFFF

... it assembles OK with dasm using the low bye ($FF) of the internal 32-byte number.
All good so far. It's detecting that the two's complement negative can be expressed as a single byte.

However, long ago when I was "fixing" things, and as a result we were required to write ...

VAL = -1
    lda #VAL&$FF. ; nobody liked this

... well the "fix" was reverted/fixed, and we could once again write just ``lda #-1"
The rationale being that dasm would range-check (I am guessing) to detect if the operand was in the correct (signed or unsigned) range. And now to my point...

The valid signed range is of course 8 bits (2's complement). That is from $80 (-128 signed) to $FF (unsigned).
We should be able to use any value in that range without error. But not outside it.

It seems, however, that dasm is restricting the range to -$FF to +$FF, which is incorrect.
The correct range, as noted, is -$80 to +$FF.

As an example, the following assembles - and it should not...

  lda #-130.  ; whoops!  Range should be -128 to 255 inclusive.
@msaarna
Copy link
Member

msaarna commented Aug 24, 2020

When I first implemented the overflow checks, I started out with a range check from -$80 to $FF. I later noticed that in the negative.asm test Peter F had included some checks that crossed below -$80 that he seems to expect to pass. Though I didn't understand the use case, I didn't want to break the existing assembly, so I relaxed the check from -$FF to $FF.

If someone wants to tighten it up again, I have no objection.

@andrew-davie
Copy link
Member Author

More specifically, the range allowed should be based on the 32 bit value, and then the low byte used.
So, -128 decimal to 255 decimal. Not $80 to $FF in hex values, but actually $FFFFFF80 to $000000FF hex.

@msaarna
Copy link
Member

msaarna commented Aug 25, 2020

Here's the original implementation for .byte and .word.

I had a look just now, and the range check for negative operands is handled a bit differently, and should probably be made to match the .byte case. Here's what it currently looks like...

        //FIX: for negative operands...
        if ( (addrmode == AM_IMM8) && (sym->value <0) )
        {
            opsize=1;
            sym->value=(char)(sym->value & 255);
            break;
        }

...after the dust has settled from the big merge, and I'll have a go at fixing it.

@andrew-davie
Copy link
Member Author

Probably all it needs is...

    if ( (addrmode == AM_IMM8) && (sym->value >= -128) && (sym->value < 256))

@thrust26 thrust26 added the bug Something isn't working label Apr 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants