Critical BUG in DATA PACKED

[artrag]

I suspect (say I'm almost sure) there is something wrong in DATA PACKED

 

I've compiled the code in attach and the sequence of data 

 

  DATA PACKED  -1,128,128,128,128,128,128,128,128,128,128,128,128,128,128,128,128,128,128,128
  DATA PACKED  -1,  0, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 65,129
  DATA PACKED  -1,130, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,129, -1
  DATA PACKED 128,128,128,128,128,128,128,128,128,128,128,128,128,128,128,128,128,128, -1, -1

 

Becomes in the ROM (using hex explorer)

 

db FFh,80h,80h,80h,80h,80h,80h,80h,80h,80h,80h,80h,80h,80h,80h,80h,80h,80h,80h,80h

db FFh,00h,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,41h,81h

db FFh,82h,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh

db 80h,80h,80h,80h,80h,80h,80h,80h,80h,80h,80h,80h,80h,80h,80h,80h,80h,80h,FFh,FFh

As you may see the 3rd line ends by FFh,FFh but it should have been 81h,FFh

 

Same, later,  for 

DATA PACKED  -1, -1,130, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,  1, -1

that becomes

db FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh,FFh

 

The problem occurs on each 3rd line more or less, but I cannot see a clear pattern 

testpackedattributes.bas romtest.bin

[+DZ-Jay]

Try replacing -1 with 255.  It seems it overflows.  The pattern seems to be when you have -1 right next to an 8-bit value to be packed together.  I'm thinking it is interpreting -1 as an unsigned 16-bit integer.

 

According to the documentation, DATA PACK is intended to pack two bytes per word, such as for character strings, so the assumption of 8-bit values seems to be reasonable.

 

I guess the compiler should actually mask the LSB before packing, but in the meantime, restricting it to 8-bit values should work.

    DATA PACKED  255,128,128,128,128,128,128,128,128,128,128,128,128,128,128,128,128,128,128,128
    DATA PACKED  255,  0,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255, 65,129
    DATA PACKED  255,130,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,129,255
    DATA PACKED  128,128,128,128,128,128,128,128,128,128,128,128,128,128,128,128,128,128,255,255

  (I just tested it and it does the trick.)

 

    -dZ.

Edited January 31, 2021 by DZ-Jay

[carlsson]

19 minutes ago, artrag said:

The problem occurs on each 3rd line more or less

No, it occurs every time you have a negative value in LSB.

 

I tried this:

datatest:
  DATA PACKED -1,128,128,-1
  DATA PACKED 127,-1
  DATA PACKED 128,-1
  DATA PACKED 129,-1
  DATA PACKED 130,-1
  DATA PACKED 128,-2
  DATA PACKED 128,-3
  DATA PACKED 135,-3
  DATA PACKED 170,-3

 

and by looking at the list file, it generates the following:

FF80, FFFF, FFFF, FFFF, FFFF, FFFE, FFFD, FFFD, FFFD

 

So like DZ-Jay suggests, some kind of overflow or that the compiler really doesn't expect two-complement numbers there.

 

Edit: For that matter, if it was two-complement, the range would be -128 to +127 so all numbers 128-130 would actually be -128 to -126.

Edited January 31, 2021 by carlsson

[+DZ-Jay]

And the plot thickens ...

 

I think @carlsson is right in that it is a problem with two's complemented numbers, i.e. negative 8-bit numbers.  Including a positive value larger than 255 is gated properly and results in an error:

Error: integer out of 8-bits range in DATA PACKED in line 27

 

However, negative numbers are accepted, but the strange output suggest they are not really expected.  (Funnily enough, using -128 will result in the same 8-bit range error, which suggests that the compiler does expect to support signed 8-bit values.)

 

Digging a little deeper, it does look like an overflow of some kind:

    DATA PACKED  1,  128  ' Results in $0180
    DATA PACKED  1, -128  ' Results in $FF80

It seems that any negative value in an even position will cause this.

 

Conversely, putting the negative value in the odd position will result in another unexpected output:

    DATA PACKED   128, 1  ' Results in $8001
    DATA PACKED  -128  1  ' Results in $8001

That's probably not what you were expecting, but considering that the DATA PACK command was intended to deal with packing characters -- which are 8-bit values -- then you really should have expected it.

 

That said, if unsigned 8-bit values is all that it supports, then it should either fail on values lesser than zero, or mask them to 8-bit in all cases, just to be consistent.

 

    -dZ.

Edited January 31, 2021 by DZ-Jay

[artrag]

Definitely it doesn't handle correctly negative numbers...

Now I've processed my data to make them fit in 8bit unsigned and it works.

 

[+DZ-Jay]

Looking at the compiler code, it does seem to want to deal with signed 8-bit values:

// ... Somewhere at the start of the "PACKED" handler ...
int c;
int v;
int v1;
int v2;


// ... Removed the previous case for strings ...

} else {
    class node *tree;
    int type;
    
    tree = eval_level0(&type);
    if (tree->node_type() != C_NUM) {
        emit_error("not a constant expression in DATA");
        break;
    }
    if (v == 0) {
        v1 = tree->node_value();
        if (v1 < -128 || v1 > 255)
            emit_error("integer out of 8-bits range in DATA PACKED");
    } else {
        v2 = tree->node_value();
        if (v2 < -128 || v2 > 255)
            emit_error("integer out of 8-bits range in DATA PACKED");
    }
    delete tree;
    tree = NULL;
    v = !v;
    if (v == 0)
        output->emit_d(N_DECLE, (v1 << 8) | v2);
}

 

I think that last line is problematic:  The first value is shifted and bitwise-OR'ed to the second value, but the underlying variables in the compiler are not 8 bits long, which will necessarily cause the OR to misbehave for negative values.  It's been a while since I used C++, but from what I can see, they are declared as "int," so I think they are 32-bits.

 

I guess something like this would work:

output->emit_d(N_DECLE, (v1 << 8) | (v2 & 255));  // mask LSB of second value

 

The first value is fine, because any upper bits will be discarded when crammed into a "DECLE".

 

    -dZ.

Edited January 31, 2021 by DZ-Jay

[+DZ-Jay]

17 minutes ago, artrag said:

Definitely it doesn't handle correctly negative numbers...

Now I've processed my data to make them fit in 8bit unsigned and it works.

 

 

For the moment, you can work around the problem with representing your negative values as unsigned 8-bit numbers.

[+nanochess]

For sure it is a bug. ?

 

I'll correct it in the next iteration.

 

Thanks to everyone for finding ways around