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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: FixEmitter.diff
Type: text/x-patch
Size: 13801 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20081122/eeab4fe0/attachment.bin>
Actually, there is a problem with the patch. Please delay review. Thomas Jablin wrote:> 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 > ------------------------------------------------------------------------ > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Here is the corrected version. Thomas Jablin wrote:> Actually, there is a problem with the patch. Please delay review. > > Thomas Jablin wrote: > >> 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 >> ------------------------------------------------------------------------ >> >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-------------- next part -------------- A non-text attachment was scrubbed... Name: FixEmitter.diff Type: text/x-patch Size: 13801 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20081122/6a6311f5/attachment.bin>