[LLVMdev] MachineCodeEmitter Patch

[Evan Cheng]

Committed. Thanks.

Evan

On Dec 8, 2008, at 3:51 PM, "Thomas B. Jablin"  
<tjablin at cs.princeton.edu> wrote:

> Thanks. I do not have commit privilege.
> Tom
>
> ----- Original Message -----
> From: "Evan Cheng" <evan.cheng at apple.com>
> To: "LLVM Developers Mailing List" <llvmdev at cs.uiuc.edu>
> Sent: Monday, December 8, 2008 5:39:33 PM GMT -05:00 US/Canada Eastern
> Subject: Re: [LLVMdev] MachineCodeEmitter Patch
>
> Looks good. Do you have commit privilege?
>
> Evan
>
> On Nov 22, 2008, at 1:19 PM, Thomas Jablin wrote:
>
>> 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
>>>
>>
>> <FixEmitter.diff>_______________________________________________
>> 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
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev