llvm dev - Aug 2012 - [LLVMdev] [PATCH] Fix for bug in JIT exception table allocation (no test yet)

Hi, I found a bug in the code that generates exception tables, I've attached
what I think is the correct fix.

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.

I haven't implemented a test case that reproduces the problem, but I will do
so.  (I've verified the problem and the fix in the scope of a much larger
system) I'm open to suggestions as to how best to test it, I'm currently
thinking of trying to create a highly contrived situation to force exception
tables to be written at the end of a buffer that won't be long enough.

============================================================================michaelMuller
= mmuller at enduden.com | http://www.mindhog.net/~mmuller
-----------------------------------------------------------------------------
In this book it is spoken of the Sephiroth, and the Paths, of Spirits and
Conjurations; of Gods, Spheres, Planes and many other things which may or
may not exist.  It is immaterial whether they exist or not.  By doing
certain things certain results follow. - Aleister Crowley
============================================================================--------------
next part --------------
Index: lib/ExecutionEngine/JIT/JITEmitter.cpp
==================================================================---
lib/ExecutionEngine/JIT/JITEmitter.cpp	(revision 162311)
+++ 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;
+      }
+    }
     MemMgr->endExceptionTable(F.getFunction(), BufferBegin, CurBufferPtr,
                               FrameRegister);
     BufferBegin = SavedBufferBegin;

On Aug 21, 2012, at 2:12 PM, Michael Muller <mmuller at enduden.com>
wrote:
> 
> Hi, I found a bug in the code that generates exception tables, I've
attached
> what I think is the correct fix.
> 
> 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.
> 
> I haven't implemented a test case that reproduces the problem, but I
will do
> so.  (I've verified the problem and the fix in the scope of a much
larger
> system) I'm open to suggestions as to how best to test it, I'm
currently
> thinking of trying to create a highly contrived situation to force
exception
> tables to be written at the end of a buffer that won't be long enough.
I'm actually somewhat curious at this point why it doesn't emit the
tables before
deciding it's done with the function. That'd make it possible to move
all of
the eh table code earlier in the method and use retryWith... instead of the
loop.

-eric

Eric Christopher wrote:
> 
> On Aug 21, 2012, at 2:12 PM, Michael Muller <mmuller at enduden.com>
wrote:
> 
> > 
> > Hi, I found a bug in the code that generates exception tables,
I've attached
> > what I think is the correct fix.
> > 
> > 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.
> > 
> > I haven't implemented a test case that reproduces the problem, but
I will do
> > so.  (I've verified the problem and the fix in the scope of a much
larger
> > system) I'm open to suggestions as to how best to test it, I'm
currently
> > thinking of trying to create a highly contrived situation to force
exception
> > tables to be written at the end of a buffer that won't be long
enough.
> 
> I'm actually somewhat curious at this point why it doesn't emit the
tables before
> deciding it's done with the function. That'd make it possible to
move all of
> the eh table code earlier in the method and use retryWith... instead of the
loop.
It looks like it might be using a different slab for the function, though
I'm
not quite sure why.  I'll investigate further when I get back to it.  Making
it part of the function's "transaction" seemed cleaner to me as
well.
> 
> -eric
> 

============================================================================michaelMuller
= mmuller at enduden.com | http://www.mindhog.net/~mmuller
-----------------------------------------------------------------------------
The people can always be brought to the bidding of the leaders. That is easy.
All you have to do is tell them they are being attacked and denounce the
pacifists for lack of patriotism and exposing the country to danger. It works
the same way in any country. - Hermann Goering (Hitler's right-hand man)
=============================================================================