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

[Michael Muller]

Thanks, Jim.

I should make it clear that I have no commit privs - can you commit this?


On Thu, Jan 10, 2013 at 4:05 PM, Jim Grosbach <grosbach at apple.com> wrote:

> 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>
>
>
>


-- 
Michael Muller
Ym9yZWQ/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130111/71aa0bf2/attachment.html>