Build error is introduced with llvm-3.6.0svn r223183.


commit 5ab94e7135fe4fabbe9934e344b894de21063d92
Author: Lang Hames <lhames@gmail.com>
Date:   Wed Dec 3 00:51:19 2014 +0000

    [MCJIT] Unique-ptrify the RTDyldMemoryManager member of MCJIT. NFC.
    
    
    
    git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@223183 91177308-0d34-0410-b5e6-96231b3b80d8

Thanks Vinson.

It should be fixed with ef7e0b39a24966526b102643523feac765771842

It looks as if there is another problem with this code with llvm-3.6.  The ShaderMemoryManager object pointed to by MM is deleted at line 530, but if (i) useMCJIT is false, and (ii) HAVE_LLVM >= 0x0306 (ie llvm >= 3.6), then MM is never initialised and delete is being applied to a thin air value.

Either MM need to be initialized to 0 at its declaration, or initialized to 0 at line 517.

In addition, MM looks as if it should be set to 0 after line 504 to prevent a double delete (the initialization of the std::unique_ptr object at line 504 will presumably take ownership).

Chris

About the missing init at line 517. That code will never be reached, as USE_MCJIT/useMCJIT is 1, on llvm 3.6 and larger.

Re line 504, afaik the init of a new class/object (std::unique_ptr<foo>) does/should not change the ownership of the underlying MM object. I'm not an C++ expert though :-\

If you believe that my analysis is a bit off, please open a separate bug with your observations.

Thanks

Emil,

OK about your first point, and thanks.

On your second, as the name suggests the whole purpose of std::unique_ptr is to take unique ownership.  a unique_ptr will delete any object it still owns, in its destructor.  Ownership is transferred from one unique_ptr to another by passing the unique_ptr as an rvalue.  This will release ownership in the movant and pass it to the movee.  You must never call delete manually on an object owned by a unique_ptr or you will get a double delete, so this code is definitely wrong.

Of course this begs the question of what EngineBuilder::setMCJITMemoryManager(ShaderMemoryManager*) does in llvm < 3.6.  If that method also tries to delete its argument when finished with, you would get a double delete in the error case (ie where EngineBuilder::create() fails) for earlier versions of llvm.  In other words, the delete call may be wrong in all cases.

Do you want me to open a separate bug to say so?

 I have posted bug #89387, entitled "Double delete in lp_bld_misc.cpp".