Bug 86958

Summary: lp_bld_misc.cpp:503:40: error: no matching function for call to ‘llvm::EngineBuilder::setMCJITMemoryManager(ShaderMemoryManager*&)’
Product: Mesa Reporter: Vinson Lee <vlee>
Component: Mesa coreAssignee: mesa-dev
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium Keywords: bisected
Version: git   
Hardware: x86-64 (AMD64)   
OS: All   
Whiteboard:
i915 platform: i915 features:

Description Vinson Lee 2014-12-03 05:37:05 UTC
mesa: 02cc9e9f9e48795371298dcf25a9de6215138880 (master 10.5.0-devel)

Build error with llvm-3.6.0svn.

  Compiling src/gallium/auxiliary/gallivm/lp_bld_misc.cpp ...
src/gallium/auxiliary/gallivm/lp_bld_misc.cpp: In function ‘LLVMBool lp_build_create_jit_compiler_for_module(LLVMOpaqueExecutionEngine**, lp_generated_code**, LLVMModuleRef, LLVMMCJITMemoryManagerRef, unsigned int, int, char**)’:
src/gallium/auxiliary/gallivm/lp_bld_misc.cpp:503:40: error: no matching function for call to ‘llvm::EngineBuilder::setMCJITMemoryManager(ShaderMemoryManager*&)’
        builder.setMCJITMemoryManager(MM);
                                        ^
Comment 1 Vinson Lee 2014-12-03 06:36:08 UTC
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
Comment 2 Jose Fonseca 2014-12-03 07:51:47 UTC
Thanks Vinson.

It should be fixed with ef7e0b39a24966526b102643523feac765771842
Comment 3 Chris Vine 2015-02-28 11:53:12 UTC
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.
Comment 4 Chris Vine 2015-02-28 12:04:02 UTC
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).
Comment 5 Emil Velikov 2015-02-28 14:51:38 UTC
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
Comment 6 Chris Vine 2015-02-28 22:52:59 UTC
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?
Comment 7 Chris Vine 2015-03-01 23:13:10 UTC
 I have posted bug #89387, entitled "Double delete in lp_bld_misc.cpp".

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.