dimitriv / Ziria

A domain-specific-language and compiler for low-level bitstream processing.
92 stars 18 forks source link

LUT generation #124

Closed ghost closed 8 years ago

ghost commented 8 years ago

There is some illegal code in LUT generation. @mainland's suggestion does fix https://github.com/dimitriv/Ziria/issues/123, but the problem is a bit more general.

For example, there is:

calign unsigned char clut350[512][2];
void clut_gen353()
{
    for (unsigned long _lidx = 0; _lidx < 512; _lidx++) {
        uint16 idx352 = (uint16) _lidx;
        calign unsigned char hdata354[4] = {0};
...
        orig_idx356 = idx352;
        bitArrWrite((BitArrPtr) &orig_idx356, 16, 8, (BitArrPtr) hdata354);
...
        dbg_idx357 |= (uint16) (*(uint8*) &((BitArrPtr) hdata354)[2] & 255);
...
        if (idx352 != dbg_idx357) {
            printf("Fatal bug in LUT generation: packIdx/unpackIdx mismatch.\n");
            printf("Location: %s\n",
                   "blocks/../../transmitter/parsePLCPHeader.blk:196:10-22");
            exit(-1);
        }
...
}

The problem is, we are reading 256 in hdata354[2] using uint8 pointer. My guess is, 256 read through uint8 pointer should return 0, but it returns 256 since this bug was not hit before. We could just cast to uint16 pointer as follows, and that would fix this particular case:

dbg_idx357 |= (uint16) (*(uint16*) &((BitArrPtr) hdata354)[2] & 511);

But there's also:

calign unsigned char clut334[131072][2];
void clut_gen337()

{
    for (unsigned long _lidx = 0; _lidx < 131072; _lidx++) {
        uint32 idx336 = (uint32) _lidx;
        calign unsigned char hdata338[4] = {0};
...
        orig_idx340 = idx336;
        bitArrWrite((BitArrPtr) &orig_idx340, 8, 16, (BitArrPtr) hdata338);
...
        dbg_idx341 |= (uint32) (*(uint16*) &((BitArrPtr) hdata338)[1] & 65535);
...
        if (idx336 != dbg_idx341) {
...
            printf("Fatal bug in LUT generation: packIdx/unpackIdx mismatch.\n");
            printf("Location: %s\n",
                   "blocks/../../transmitter/parsePLCPHeader.blk:180:8-25");
            exit(-1);
        }
...
}

The fix here is different, since we're actually trying to read 65,536 in hdata338[1] as uint16. Firstly, 65,536 is beyond the range of uint16. Secondly, the bitArrWrite function only wrote uint16, so casting correctly to uint32 in codegen may not be sufficient.

ghost commented 8 years ago

This was just misaligned pointer access, which Geoff mentioned works fine on newer hardware. Closing as invalid.