[llvm-commits] [llvm] r84651 - in /llvm/trunk: include/llvm/ExecutionEngine/JITMemoryManager.h lib/ExecutionEngine/JIT/JITEmitter.cpp lib/ExecutionEngine/JIT/JITMemoryManager.cpp unittests/ExecutionEngine/JIT/JITMemoryManagerTest.cpp

Jeffrey Yasskin jyasskin at google.com
Wed Oct 21 16:01:29 PDT 2009


Oops!

You probably don't need the "if (DwarfExceptionHandling ||
JITEmitDebugInfo)", and it could probably also work to require the
deallocate functions to check for NULL (by analogy to free()).

But this patch is fine with me. Please check it in with or without the
above tweaks.

On Wed, Oct 21, 2009 at 8:06 AM, Nicolas Geoffray
<nicolas.geoffray at lip6.fr> wrote:
> Hi Jeffrey,
>
> There is a problem with your patch when the JIT runs out of memory, tries to
> deallocate the current buffer allocated for the function, and SIGSEGV
> because there is no buffer allocated for the exception table of the
> function.
>
> The following patch fixes the issue. OK to commit?
>
> Thanks,
> Nicolas
>
> Jeffrey Yasskin wrote:
>>
>> Author: jyasskin
>> Date: Tue Oct 20 13:13:21 2009
>> New Revision: 84651
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=84651&view=rev
>> Log:
>> Move the Function*->allocated blocks map from the JITMemoryManager to the
>> JITEmitter.
>>
>> I'm gradually making Functions auto-remove themselves from the JIT when
>> they're
>> destroyed. In this case, the Function needs to be removed from the
>> JITEmitter,
>> but the map recording which Functions need to be removed lived behind the
>> JITMemoryManager interface, which made things difficult.
>>
>> This patch replaces the deallocateMemForFunction(Function*) method with a
>> pair
>> of methods deallocateFunctionBody(void *) and
>> deallocateExceptionTable(void *)
>> corresponding to the two startFoo/endFoo pairs.
>>
>>
>> Modified:
>>    llvm/trunk/include/llvm/ExecutionEngine/JITMemoryManager.h
>>    llvm/trunk/lib/ExecutionEngine/JIT/JITEmitter.cpp
>>    llvm/trunk/lib/ExecutionEngine/JIT/JITMemoryManager.cpp
>>    llvm/trunk/unittests/ExecutionEngine/JIT/JITMemoryManagerTest.cpp
>>
>> Modified: llvm/trunk/include/llvm/ExecutionEngine/JITMemoryManager.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/JITMemoryManager.h?rev=84651&r1=84650&r2=84651&view=diff
>>
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/ExecutionEngine/JITMemoryManager.h (original)
>> +++ llvm/trunk/include/llvm/ExecutionEngine/JITMemoryManager.h Tue Oct 20
>> 13:13:21 2009
>> @@ -132,9 +132,11 @@
>>   ///
>>   virtual uint8_t *allocateGlobal(uintptr_t Size, unsigned Alignment) = 0;
>>  -  /// deallocateMemForFunction - Free JIT memory for the specified
>> function.
>> -  /// This is never called when the JIT is currently emitting a function.
>> -  virtual void deallocateMemForFunction(const Function *F) = 0;
>> +  /// deallocateFunctionBody - Free the specified function body.  The
>> argument
>> +  /// must be the return value from a call to startFunctionBody() that
>> hasn't
>> +  /// been deallocated yet.  This is never called when the JIT is
>> currently
>> +  /// emitting a function.
>> +  virtual void deallocateFunctionBody(void *Body) = 0;
>>     /// startExceptionTable - When we finished JITing the function, if
>> exception
>>   /// handling is set, we emit the exception table.
>> @@ -146,6 +148,12 @@
>>   virtual void endExceptionTable(const Function *F, uint8_t *TableStart,
>>                                  uint8_t *TableEnd, uint8_t*
>> FrameRegister) = 0;
>>  +  /// deallocateExceptionTable - Free the specified exception table's
>> memory.
>> +  /// The argument must be the return value from a call to
>> startExceptionTable()
>> +  /// that hasn't been deallocated yet.  This is never called when the
>> JIT is
>> +  /// currently emitting an exception table.
>> +  virtual void deallocateExceptionTable(void *ET) = 0;
>> +
>>   /// CheckInvariants - For testing only.  Return true if all internal
>>   /// invariants are preserved, or return false and set ErrorStr to a
>> helpful
>>   /// error message.
>>
>> Modified: llvm/trunk/lib/ExecutionEngine/JIT/JITEmitter.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/JIT/JITEmitter.cpp?rev=84651&r1=84650&r2=84651&view=diff
>>
>>
>> ==============================================================================
>> --- llvm/trunk/lib/ExecutionEngine/JIT/JITEmitter.cpp (original)
>> +++ llvm/trunk/lib/ExecutionEngine/JIT/JITEmitter.cpp Tue Oct 20 13:13:21
>> 2009
>> @@ -42,6 +42,7 @@
>>  #include "llvm/System/Disassembler.h"
>>  #include "llvm/System/Memory.h"
>>  #include "llvm/Target/TargetInstrInfo.h"
>> +#include "llvm/ADT/DenseMap.h"
>>  #include "llvm/ADT/SmallPtrSet.h"
>>  #include "llvm/ADT/SmallVector.h"
>>  #include "llvm/ADT/Statistic.h"
>> @@ -548,6 +549,13 @@
>>     /// finishFunction.
>>     JITEvent_EmittedFunctionDetails EmissionDetails;
>>  +    struct EmittedCode {
>> +      void *FunctionBody;
>> +      void *ExceptionTable;
>> +      EmittedCode() : FunctionBody(0), ExceptionTable(0) {}
>> +    };
>> +    DenseMap<const Function *, EmittedCode> EmittedFunctions;
>> +
>>     // CurFnStubUses - For a given Function, a vector of stubs that it
>>     // references.  This facilitates the JIT detecting that a stub is no
>>     // longer used, so that it may be deallocated.
>> @@ -1011,7 +1019,8 @@
>>   BufferBegin = CurBufferPtr = MemMgr->startFunctionBody(F.getFunction(),
>>                                                          ActualSize);
>>   BufferEnd = BufferBegin+ActualSize;
>> -  +  EmittedFunctions[F.getFunction()].FunctionBody = BufferBegin;
>> +
>>   // Ensure the constant pool/jump table info is at least 4-byte aligned.
>>   emitAlignment(16);
>>  @@ -1201,6 +1210,7 @@
>>     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);
>> @@ -1244,7 +1254,13 @@
>>  /// deallocateMemForFunction - Deallocate all memory for the specified
>>  /// function body.  Also drop any references the function has to stubs.
>>  void JITEmitter::deallocateMemForFunction(const Function *F) {
>> -  MemMgr->deallocateMemForFunction(F);
>> +  DenseMap<const Function *, EmittedCode>::iterator Emitted =
>> +    EmittedFunctions.find(F);
>> +  if (Emitted != EmittedFunctions.end()) {
>> +    MemMgr->deallocateFunctionBody(Emitted->second.FunctionBody);
>> +    MemMgr->deallocateExceptionTable(Emitted->second.ExceptionTable);
>> +    EmittedFunctions.erase(Emitted);
>> +  }
>>     // TODO: Do we need to unregister exception handling information from
>> libgcc
>>   // here?
>>
>> Modified: llvm/trunk/lib/ExecutionEngine/JIT/JITMemoryManager.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/JIT/JITMemoryManager.cpp?rev=84651&r1=84650&r2=84651&view=diff
>>
>>
>> ==============================================================================
>> --- llvm/trunk/lib/ExecutionEngine/JIT/JITMemoryManager.cpp (original)
>> +++ llvm/trunk/lib/ExecutionEngine/JIT/JITMemoryManager.cpp Tue Oct 20
>> 13:13:21 2009
>> @@ -297,9 +297,6 @@
>>       uint8_t *GOTBase;     // Target Specific reserved memory
>>     void *DlsymTable;     // Stub external symbol information
>> -
>> -    std::map<const Function*, MemoryRangeHeader*> FunctionBlocks;
>> -    std::map<const Function*, MemoryRangeHeader*> TableBlocks;
>>   public:
>>     DefaultJITMemoryManager();
>>     ~DefaultJITMemoryManager();
>> @@ -414,7 +411,6 @@
>>              "Mismatched function start/end!");
>>         uintptr_t BlockSize = FunctionEnd - (uint8_t *)CurBlock;
>> -      FunctionBlocks[F] = CurBlock;
>>         // Release the memory at the end of this block that isn't needed.
>>       FreeMemoryList =CurBlock->TrimAllocationToSize(FreeMemoryList,
>> BlockSize);
>> @@ -464,7 +460,6 @@
>>              "Mismatched table start/end!");
>>             uintptr_t BlockSize = TableEnd - (uint8_t *)CurBlock;
>> -      TableBlocks[F] = CurBlock;
>>         // Release the memory at the end of this block that isn't needed.
>>       FreeMemoryList =CurBlock->TrimAllocationToSize(FreeMemoryList,
>> BlockSize);
>> @@ -478,15 +473,9 @@
>>       return DlsymTable;
>>     }
>>     -    /// deallocateMemForFunction - Deallocate all memory for the
>> specified
>> -    /// function body.
>> -    void deallocateMemForFunction(const Function *F) {
>> -      std::map<const Function*, MemoryRangeHeader*>::iterator
>> -        I = FunctionBlocks.find(F);
>> -      if (I == FunctionBlocks.end()) return;
>> -      +    void deallocateBlock(void *Block) {
>>       // Find the block that is allocated for this function.
>> -      MemoryRangeHeader *MemRange = I->second;
>> +      MemoryRangeHeader *MemRange =
>> static_cast<MemoryRangeHeader*>(Block) - 1;
>>       assert(MemRange->ThisAllocated && "Block isn't allocated!");
>>         // Fill the buffer with garbage!
>> @@ -496,27 +485,18 @@
>>         // Free the memory.
>>       FreeMemoryList = MemRange->FreeBlock(FreeMemoryList);
>> -      -      // Finally, remove this entry from FunctionBlocks.
>> -      FunctionBlocks.erase(I);
>> -      -      I = TableBlocks.find(F);
>> -      if (I == TableBlocks.end()) return;
>> -      -      // Find the block that is allocated for this function.
>> -      MemRange = I->second;
>> -      assert(MemRange->ThisAllocated && "Block isn't allocated!");
>> +    }
>>  -      // Fill the buffer with garbage!
>> -      if (PoisonMemory) {
>> -        memset(MemRange+1, 0xCD, MemRange->BlockSize-sizeof(*MemRange));
>> -      }
>> +    /// deallocateFunctionBody - Deallocate all memory for the specified
>> +    /// function body.
>> +    void deallocateFunctionBody(void *Body) {
>> +      deallocateBlock(Body);
>> +    }
>>  -      // Free the memory.
>> -      FreeMemoryList = MemRange->FreeBlock(FreeMemoryList);
>> -      -      // Finally, remove this entry from TableBlocks.
>> -      TableBlocks.erase(I);
>> +    /// deallocateExceptionTable - Deallocate memory for the specified
>> +    /// exception table.
>> +    void deallocateExceptionTable(void *ET) {
>> +      deallocateBlock(ET);
>>     }
>>       /// setMemoryWritable - When code generation is in progress,
>>
>> Modified:
>> llvm/trunk/unittests/ExecutionEngine/JIT/JITMemoryManagerTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/JIT/JITMemoryManagerTest.cpp?rev=84651&r1=84650&r2=84651&view=diff
>>
>>
>> ==============================================================================
>> --- llvm/trunk/unittests/ExecutionEngine/JIT/JITMemoryManagerTest.cpp
>> (original)
>> +++ llvm/trunk/unittests/ExecutionEngine/JIT/JITMemoryManagerTest.cpp Tue
>> Oct 20 13:13:21 2009
>> @@ -32,37 +32,36 @@
>>   OwningPtr<JITMemoryManager> MemMgr(
>>       JITMemoryManager::CreateDefaultMemManager());
>>   uintptr_t size;
>> -  uint8_t *start;
>>   std::string Error;
>>     // Allocate the functions.
>>   OwningPtr<Function> F1(makeFakeFunction());
>>   size = 1024;
>> -  start = MemMgr->startFunctionBody(F1.get(), size);
>> -  memset(start, 0xFF, 1024);
>> -  MemMgr->endFunctionBody(F1.get(), start, start + 1024);
>> +  uint8_t *FunctionBody1 = MemMgr->startFunctionBody(F1.get(), size);
>> +  memset(FunctionBody1, 0xFF, 1024);
>> +  MemMgr->endFunctionBody(F1.get(), FunctionBody1, FunctionBody1 + 1024);
>>   EXPECT_TRUE(MemMgr->CheckInvariants(Error)) << Error;
>>     OwningPtr<Function> F2(makeFakeFunction());
>>   size = 1024;
>> -  start = MemMgr->startFunctionBody(F2.get(), size);
>> -  memset(start, 0xFF, 1024);
>> -  MemMgr->endFunctionBody(F2.get(), start, start + 1024);
>> +  uint8_t *FunctionBody2 = MemMgr->startFunctionBody(F2.get(), size);
>> +  memset(FunctionBody2, 0xFF, 1024);
>> +  MemMgr->endFunctionBody(F2.get(), FunctionBody2, FunctionBody2 + 1024);
>>   EXPECT_TRUE(MemMgr->CheckInvariants(Error)) << Error;
>>     OwningPtr<Function> F3(makeFakeFunction());
>>   size = 1024;
>> -  start = MemMgr->startFunctionBody(F3.get(), size);
>> -  memset(start, 0xFF, 1024);
>> -  MemMgr->endFunctionBody(F3.get(), start, start + 1024);
>> +  uint8_t *FunctionBody3 = MemMgr->startFunctionBody(F3.get(), size);
>> +  memset(FunctionBody3, 0xFF, 1024);
>> +  MemMgr->endFunctionBody(F3.get(), FunctionBody3, FunctionBody3 + 1024);
>>   EXPECT_TRUE(MemMgr->CheckInvariants(Error)) << Error;
>>     // Deallocate them out of order, in case that matters.
>> -  MemMgr->deallocateMemForFunction(F2.get());
>> +  MemMgr->deallocateFunctionBody(FunctionBody2);
>>   EXPECT_TRUE(MemMgr->CheckInvariants(Error)) << Error;
>> -  MemMgr->deallocateMemForFunction(F1.get());
>> +  MemMgr->deallocateFunctionBody(FunctionBody1);
>>   EXPECT_TRUE(MemMgr->CheckInvariants(Error)) << Error;
>> -  MemMgr->deallocateMemForFunction(F3.get());
>> +  MemMgr->deallocateFunctionBody(FunctionBody3);
>>   EXPECT_TRUE(MemMgr->CheckInvariants(Error)) << Error;
>>  }
>>  @@ -72,7 +71,6 @@
>>   OwningPtr<JITMemoryManager> MemMgr(
>>       JITMemoryManager::CreateDefaultMemManager());
>>   uintptr_t size;
>> -  uint8_t *start;
>>   std::string Error;
>>     // Big functions are a little less than the largest block size.
>> @@ -83,26 +81,26 @@
>>   // Allocate big functions
>>   OwningPtr<Function> F1(makeFakeFunction());
>>   size = bigFuncSize;
>> -  start = MemMgr->startFunctionBody(F1.get(), size);
>> +  uint8_t *FunctionBody1 = MemMgr->startFunctionBody(F1.get(), size);
>>   ASSERT_LE(bigFuncSize, size);
>> -  memset(start, 0xFF, bigFuncSize);
>> -  MemMgr->endFunctionBody(F1.get(), start, start + bigFuncSize);
>> +  memset(FunctionBody1, 0xFF, bigFuncSize);
>> +  MemMgr->endFunctionBody(F1.get(), FunctionBody1, FunctionBody1 +
>> bigFuncSize);
>>   EXPECT_TRUE(MemMgr->CheckInvariants(Error)) << Error;
>>     OwningPtr<Function> F2(makeFakeFunction());
>>   size = bigFuncSize;
>> -  start = MemMgr->startFunctionBody(F2.get(), size);
>> +  uint8_t *FunctionBody2 = MemMgr->startFunctionBody(F2.get(), size);
>>   ASSERT_LE(bigFuncSize, size);
>> -  memset(start, 0xFF, bigFuncSize);
>> -  MemMgr->endFunctionBody(F2.get(), start, start + bigFuncSize);
>> +  memset(FunctionBody2, 0xFF, bigFuncSize);
>> +  MemMgr->endFunctionBody(F2.get(), FunctionBody2, FunctionBody2 +
>> bigFuncSize);
>>   EXPECT_TRUE(MemMgr->CheckInvariants(Error)) << Error;
>>     OwningPtr<Function> F3(makeFakeFunction());
>>   size = bigFuncSize;
>> -  start = MemMgr->startFunctionBody(F3.get(), size);
>> +  uint8_t *FunctionBody3 = MemMgr->startFunctionBody(F3.get(), size);
>>   ASSERT_LE(bigFuncSize, size);
>> -  memset(start, 0xFF, bigFuncSize);
>> -  MemMgr->endFunctionBody(F3.get(), start, start + bigFuncSize);
>> +  memset(FunctionBody3, 0xFF, bigFuncSize);
>> +  MemMgr->endFunctionBody(F3.get(), FunctionBody3, FunctionBody3 +
>> bigFuncSize);
>>   EXPECT_TRUE(MemMgr->CheckInvariants(Error)) << Error;
>>     // Check that each large function took it's own slab.
>> @@ -111,43 +109,46 @@
>>   // Allocate small functions
>>   OwningPtr<Function> F4(makeFakeFunction());
>>   size = smallFuncSize;
>> -  start = MemMgr->startFunctionBody(F4.get(), size);
>> +  uint8_t *FunctionBody4 = MemMgr->startFunctionBody(F4.get(), size);
>>   ASSERT_LE(smallFuncSize, size);
>> -  memset(start, 0xFF, smallFuncSize);
>> -  MemMgr->endFunctionBody(F4.get(), start, start + smallFuncSize);
>> +  memset(FunctionBody4, 0xFF, smallFuncSize);
>> +  MemMgr->endFunctionBody(F4.get(), FunctionBody4,
>> +                          FunctionBody4 + smallFuncSize);
>>   EXPECT_TRUE(MemMgr->CheckInvariants(Error)) << Error;
>>     OwningPtr<Function> F5(makeFakeFunction());
>>   size = smallFuncSize;
>> -  start = MemMgr->startFunctionBody(F5.get(), size);
>> +  uint8_t *FunctionBody5 = MemMgr->startFunctionBody(F5.get(), size);
>>   ASSERT_LE(smallFuncSize, size);
>> -  memset(start, 0xFF, smallFuncSize);
>> -  MemMgr->endFunctionBody(F5.get(), start, start + smallFuncSize);
>> +  memset(FunctionBody5, 0xFF, smallFuncSize);
>> +  MemMgr->endFunctionBody(F5.get(), FunctionBody5,
>> +                          FunctionBody5 + smallFuncSize);
>>   EXPECT_TRUE(MemMgr->CheckInvariants(Error)) << Error;
>>     OwningPtr<Function> F6(makeFakeFunction());
>>   size = smallFuncSize;
>> -  start = MemMgr->startFunctionBody(F6.get(), size);
>> +  uint8_t *FunctionBody6 = MemMgr->startFunctionBody(F6.get(), size);
>>   ASSERT_LE(smallFuncSize, size);
>> -  memset(start, 0xFF, smallFuncSize);
>> -  MemMgr->endFunctionBody(F6.get(), start, start + smallFuncSize);
>> +  memset(FunctionBody6, 0xFF, smallFuncSize);
>> +  MemMgr->endFunctionBody(F6.get(), FunctionBody6,
>> +                          FunctionBody6 + smallFuncSize);
>>   EXPECT_TRUE(MemMgr->CheckInvariants(Error)) << Error;
>>     // Check that the small functions didn't allocate any new slabs.
>>   EXPECT_EQ(3U, MemMgr->GetNumCodeSlabs());
>>     // Deallocate them out of order, in case that matters.
>> -  MemMgr->deallocateMemForFunction(F2.get());
>> +  MemMgr->deallocateFunctionBody(FunctionBody2);
>>   EXPECT_TRUE(MemMgr->CheckInvariants(Error)) << Error;
>> -  MemMgr->deallocateMemForFunction(F1.get());
>> +  MemMgr->deallocateFunctionBody(FunctionBody1);
>>   EXPECT_TRUE(MemMgr->CheckInvariants(Error)) << Error;
>> -  MemMgr->deallocateMemForFunction(F4.get());
>> +  MemMgr->deallocateFunctionBody(FunctionBody4);
>>   EXPECT_TRUE(MemMgr->CheckInvariants(Error)) << Error;
>> -  MemMgr->deallocateMemForFunction(F3.get());
>> +  MemMgr->deallocateFunctionBody(FunctionBody3);
>>   EXPECT_TRUE(MemMgr->CheckInvariants(Error)) << Error;
>> -  MemMgr->deallocateMemForFunction(F5.get());
>> +  MemMgr->deallocateFunctionBody(FunctionBody5);
>>   EXPECT_TRUE(MemMgr->CheckInvariants(Error)) << Error;
>> -  MemMgr->deallocateMemForFunction(F6.get());
>> +  MemMgr->deallocateFunctionBody(FunctionBody6);
>>   EXPECT_TRUE(MemMgr->CheckInvariants(Error)) << Error;
>>  }
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>




More information about the llvm-commits mailing list