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

[Eli Bendersky]

Committed in r172214.

Eli


On Fri, Jan 11, 2013 at 8:30 AM, Michael Muller <mmuller at google.com> wrote:
> oops, I svn diff'ed this from the unit test directory.  Re-submitting.
>
>
> On Fri, Jan 11, 2013 at 11:16 AM, Eli Bendersky <eliben at google.com> wrote:
>>
>> On Fri, Jan 11, 2013 at 6:01 AM, Michael Muller <mmuller at google.com>
>> wrote:
>> > Thanks, Jim.
>> >
>> > I should make it clear that I have no commit privs - can you commit
>> > this?
>> >
>>
>> I will commit this for you a bit later today, Michael.
>>
>> Eli
>>
>>
>>
>>
>> >
>> > 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/
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> >
>
>
>
>
> --
> Michael Muller
> Ym9yZWQ/