[llvm-commits] [PATCH] Fix for bug in exception table allocation

[Jim Grosbach]

Thanks! This is good to commit.

The main style guide is http://llvm.org/docs/CodingStandards.html. When things change, migration is a slow process when there's a change, as we don't generally do a en-masse update of existing code, but rather update things a bit at a time. I think the naming conventions for variables, functions, etc, changed sometime in the last year or so. Something like that, anyway.

-Jim

On Jan 10, 2013, at 1:00 PM, Michael Muller <mmuller at google.com> wrote:

> 
> Done. New patch attached.
> 
> (I'll update the patch in the bug later)
> 
> Was there a recent style normalization effort in the codebase?  (I try to emulate the style of the context)
> 
> 
> On Thu, Jan 10, 2013 at 3:53 PM, Jim Grosbach <grosbach at apple.com> wrote:
> Missed one.
> 
> s/CreateMemoryManager/createMemoryManager/
> 
> OK to commit with that change.
> 
> 
> On Jan 10, 2013, at 11:50 AM, Michael Muller <mmuller at google.com> wrote:
> 
>> Ping?
>> 
>> 
>> On Tue, Jan 8, 2013 at 6:03 PM, Michael Muller <mmuller at google.com> wrote:
>> 
>> 
>> 
>> On Tue, Jan 8, 2013 at 5:52 PM, Jim Grosbach <grosbach at apple.com> wrote:
>> Looks good to me, with a couple nits:
>> 
>> +  virtual RecordingJITMemoryManager* CreateMemoryManager() {
>> 
>> The '*' should bind to the identifier, not the type. The function name should start with lower case. There's a few other instances in the patch of the former. Please double-check.
>> 
>> All fixed. 
>>  
>> 
>> Thanks!
>> 
>> -Jim
>> 
>> 
>> On Jan 8, 2013, at 2:43 PM, Michael Muller <mmuller at google.com> wrote:
>> 
>>> 
>>> New patch attached, incorporates Misha's comments and current trunk.
>>> 
>>> 
>>> On Fri, Jan 4, 2013 at 7:44 PM, Misha Brukman <brukman at gmail.com> wrote:
>>> On Wed, Dec 19, 2012 at 1:55 PM, Michael Muller <mmuller at enduden.com> wrote:
>>> See http://llvm.org/bugs/show_bug.cgi?id=13678 for more history.
>>> 
>>> See also:
>>>   http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-August/052766.html
>>>   http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120910/150422.html
>>>   http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120917/150801.html
>>>   http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-December/056829.html
>>> 
>>> When you run out of space writing to a buffer, the buffer management code
>>> simply stops writing at the end of the buffer.  It is the responsibility of
>>> the caller to verify that it has stayed in bounds and perform a retry with
>>> a larger memory estimate if not.  The function writing code does this, but
>>> the exception table code following it does not.  The end result is that
>>> exception table pointers can get registered pointing to invalid data, causing
>>> seg-faults when an exception is thrown.
>>> 
>>> Minor drive-by style comments (I wish there were a web UI for entering these in context):
>>> s/;;/;/
>>> ASSERT_TRUE(actual == expected) --> ASSERT_EQ(expected, actual)
>>> add space around binary operators (-, *, etc.), e.g.:
>>> +      ActualSize = (CurBufferPtr-BufferBegin)*2;
>>> 
>>> I'm sure I copied that from somewhere, but looking at the file now I see nothing resembling such formatting.  Fixed.
>>>  
>>> fix arg alignment of "ActualSize" here:
>>> +    while (true) {
>>> +      BufferBegin = CurBufferPtr = MemMgr->startExceptionTable(F.getFunction(),
>>> +                                                              ActualSize);
>>> 
>>> Done.
>>>  
>>> Misha
>>> 
>>> 
>>> 
>>> -- 
>>> Michael Muller
>>> Ym9yZWQ/
>>> <ExceptionTableOverflowFix-v5.patch>
>> 
>> 
>> 
>> 
>> -- 
>> Michael Muller
>> Ym9yZWQ/
>> 
>> 
>> 
>> -- 
>> Michael Muller
>> Ym9yZWQ/
> 
> 
> 
> 
> -- 
> Michael Muller
> Ym9yZWQ/
> <ExceptionTableOverflowFix-v7.patch>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130110/cdea9186/attachment.html>