[llvm-commits] Fwd: Re: [LLVMdev] [PATCH] Fix for bug in JIT exception table allocation

[Michael Muller]

Ping.  (looking for someone to review or commit this, thanks)

Michael Muller wrote:
> 
> Thanks, Duncan -
> 
> > Hi Michael,
> > 
> > > --- lib/ExecutionEngine/JIT/JITEmitter.cpp  (revision 163478)
> > > +++ lib/ExecutionEngine/JIT/JITEmitter.cpp  (working copy)
> > > @@ -974,14 +974,24 @@
> > >      SavedBufferBegin = BufferBegin;
> > >      SavedBufferEnd = BufferEnd;
> > >      SavedCurBufferPtr = CurBufferPtr;
> > > +    uint8_t *FrameRegister;
> > >
> > > -    BufferBegin = CurBufferPtr =
> > > MemMgr->startExceptionTable(F.getFunction(),
> > > -                                                             ActualSize);
> > > -    BufferEnd = BufferBegin+ActualSize;
> > > -    EmittedFunctions[F.getFunction()].ExceptionTable = BufferBegin;
> > > -    uint8_t *EhStart;
> > > -    uint8_t *FrameRegister = DE->EmitDwarfTable(F, *this, FnStart, FnEnd,
> > > -                                                EhStart);
> > > +    while (true) {
> > > +      BufferBegin = CurBufferPtr =
> > > MemMgr->startExceptionTable(F.getFunction(),
> > > +
> > > ActualSize);
> > > +      BufferEnd = BufferBegin+ActualSize;
> > > +      EmittedFunctions[F.getFunction()].ExceptionTable = BufferBegin;
> > > +      uint8_t *EhStart;
> > > +      FrameRegister = DE->EmitDwarfTable(F, *this, FnStart, FnEnd,
> > > EhStart);
> > > +
> > > +      // If we've run out of memory, try again with twice as much space.
> > > +      if (CurBufferPtr == BufferEnd) {
> > > +        ActualSize = (CurBufferPtr-BufferBegin)*2;
> > > +        MemMgr->deallocateExceptionTable(BufferBegin);
> > > +      } else {
> > > +        break;
> > > +      }
> > 
> > here I think it would be more conformant with LLVM style to do:
> > 
> >         // If the buffer was large enough to hold the table then we are
> >         done.
> >         if (CurBufferPtr != BufferEnd)
> >           break;
> > 
> >         // Try again with twice as much space.
> >         ActualSize = (CurBufferPtr-BufferBegin)*2;
> >         MemMgr->deallocateExceptionTable(BufferBegin);
> 
> Done.  New patch attached.
> 
> > 
> > Otherwise looks good to me, though that's not saying much since I don't know
> > this part of LLVM.
> 
> 
> It looks like that code segment has been touched by a number of people:
> 
> 
>   147615     rafael   if (JITExceptionHandling) {
>    49924   geoffray     uintptr_t ActualSize = 0;
>    91464   jyasskin     SavedBufferBegin = BufferBegin;
>    91464   jyasskin     SavedBufferEnd = BufferEnd;
>    91464   jyasskin     SavedCurBufferPtr = CurBufferPtr;
>    82418        rnk 
>    47079   geoffray     BufferBegin = CurBufferPtr =
>   MemMgr->startExceptionTable(F.getFunction(),
>    47079   geoffray
>   ActualSize);
>    47079   geoffray     BufferEnd = BufferBegin+ActualSize;
>    84651   jyasskin     EmittedFunctions[F.getFunction()].ExceptionTable =
>   BufferBegin;
>    82418        rnk     uint8_t *EhStart;
>    82418        rnk     uint8_t *FrameRegister = DE->EmitDwarfTable(F, *this,
>   FnStart, FnEnd,
>    82418        rnk                                                 EhStart);
>    48019    lattner     MemMgr->endExceptionTable(F.getFunction(), BufferBegin,
>   CurBufferPtr,
>    48019    lattner                               FrameRegister);
>    91464   jyasskin     BufferBegin = SavedBufferBegin;
>    91464   jyasskin     BufferEnd = SavedBufferEnd;
>    91464   jyasskin     CurBufferPtr = SavedCurBufferPtr;
>    47079   geoffray 
>   102865   baldrick     if (JITExceptionHandling) {
>   127047   echristo       TheJIT->RegisterTable(F.getFunction(), FrameRegister);
>    82418        rnk     }
>    47079   geoffray   }
> 
> Any suggestions as to who would be the best qualified to look at it?
> 
> 
> > 
> > Ciao, Duncan.
> > 
> > > +    }
> > >      MemMgr->endExceptionTable(F.getFunction(), BufferBegin, CurBufferPtr,
> > >                                FrameRegister);
> > >      BufferBegin = SavedBufferBegin;
> 
> 
> =============================================================================
> michaelMuller = mmuller at enduden.com | http://www.mindhog.net/~mmuller
> -----------------------------------------------------------------------------
> Government is not reason, it is not eloquence; it is force.  Like fire, it
> is a dangerous servant and a fearsome master.  - George Washington
> =============================================================================


=============================================================================
michaelMuller = mmuller at enduden.com | http://www.mindhog.net/~mmuller
-----------------------------------------------------------------------------
Government is not reason, it is not eloquence; it is force.  Like fire, it
is a dangerous servant and a fearsome master.  - George Washington
=============================================================================