MachineCodeEmitter Patch

Hi,
The following code:

#include<stdio.h>

char bigArray[0x1000000];

int main(int argc, char **argv) {
  printf("mem: 0x%x\n", (unsigned) bigArray);
  return 0;
}

causes lli to silently fail, even though it compiles correctly with llc. The reason is that in JITEmitter.cpp only checks to see if CurBufferPtr == BufferEnd at the beginning of the function and not after all relocations have been handled. I have fixed this bug by adding an additional check after all relocations have been completed. In the process of fixing this bug, I happened to look through the code in MachineCodeEmitter.h. The buffer size checks in MachineCodeEmitter.h all suffer from an integer overflow bug. For example in allocateSpace the code reads:

    // Allocate the space.
    CurBufferPtr += Size;
       // Check for buffer overflow.
    if (CurBufferPtr >= BufferEnd) {

This is wrong because Size + CurBufferPtr can cause an integer overflow and thus appear to be less than BufferEnd. The correct way to check for the end of a buffer is always:

(Size >= BufferEnd-CurBufferPtr)

This integer overflow bug causes the program:
#include<stdio.h>

char b = 'b';
char c[0x8000000];

int main(int argc, char **argv) {
  printf("%c\n", c[0]);
  return 0;
}
to segfault in lli.

Finally, I have changed several instances of intptr_t to uintptr_t to avoid dangerous comparisons between signed and unsigned types. Code review of the enclosed patch would be greatly appreciated. Thanks
Tom

FixEmitter.diff (13.5 KB)

Actually, there is a problem with the patch. Please delay review.

Thomas Jablin wrote:

Here is the corrected version.

Thomas Jablin wrote:

FixEmitter.diff (13.5 KB)

Thanks. But we need to match the type changes in all the target. e.g. ARMJITInfo.cpp, X86CodeEmitter.cpp. Also in MachineRelocation, e.g. getBB. Could you prepare a patch with all those fixed as well?

Evan

Looks good. Do you have commit privilege?

Evan

Thanks. I do not have commit privilege.
Tom

Committed. Thanks.

Evan