Bug 89387 - Double delete in lp_bld_misc.cpp
Summary: Double delete in lp_bld_misc.cpp
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: 10.5
Hardware: Other All
: medium normal
Assignee: mesa-dev
QA Contact: Jose Fonseca
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-01 23:12 UTC by Chris Vine
Modified: 2015-03-12 10:03 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Vine 2015-03-01 23:12:26 UTC
I have been asked in bug #86958 to open a separate bug about the resolution of the compilation error reported there, where mesa is compiled against llvm>=3.6.

To fix that compilation error, at line 504 of file lp_bld_misc.cpp the ShaderMemoryManager* object MM is passed to a unique_ptr object, which takes ownership of MM.  However, in the event of the call to EngineBuilder::create() at line 523 failing, at line 530 delete is called manually on MM, thus leading to a possible double delete (since the destructor of the unique_ptr object having ownership will also attempt to delete MM).

This may beg 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 also.  However, whatever the answer to that, the fix for bug 86958 is on the fact of it wrong.
Comment 1 Jose Fonseca 2015-03-04 14:11:52 UTC
Would this fix it?

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
index 5210acc..7387ffb 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
+++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
@@ -502,6 +502,7 @@ lp_build_create_jit_compiler_for_module(LLVMExecutionEngineRef *OutJIT,
 
 #if HAVE_LLVM >= 0x0306
        builder.setMCJITMemoryManager(std::unique_ptr<RTDyldMemoryManager>(MM));
+       MM = NULL;
 #else
        builder.setMCJITMemoryManager(MM);
 #endif
Comment 2 Chris Vine 2015-03-05 17:27:51 UTC
That will work fine.

This does beg the question whether the code is correct for llvm < 3.6 (it fixes llvm >= 3.6).
Comment 3 Jose Fonseca 2015-03-12 10:03:11 UTC
(In reply to Chris Vine from comment #2)
> That will work fine.

Thanks.

> This does beg the question whether the code is correct for llvm < 3.6 (it
> fixes llvm >= 3.6).

I believe so.

LLVM 3.4 and 3.3's documentation for  setMCJITMemoryManager/setJITMemoryManager state they only take onwership if builder.create() is successful.

And indeed we only delete MM when builder.create() fails.



Fixed on http://cgit.freedesktop.org/mesa/mesa/commit/?id=70dc8a9930f561d7ce6db7e58b5bc9b4d940e37b


bug/show.html.tmpl processed on Jan 18, 2017 at 04:12:06.
(provided by the Example extension).