[llvm-commits] [llvm] r78127 - in /llvm/trunk: lib/ExecutionEngine/ExecutionEngine.cpp unittests/ExecutionEngine/ExecutionEngineTest.cpp unittests/ExecutionEngine/Makefile

[Chris Lattner]

On Aug 4, 2009, at 9:12 PM, Jeffrey Yasskin wrote:

> On Tue, Aug 4, 2009 at 7:30 PM, Chris Lattner<clattner at apple.com>  
> wrote:
>>
>> On Aug 4, 2009, at 4:53 PM, Jeffrey Yasskin wrote:
>>
>>> Author: jyasskin
>>> Date: Tue Aug  4 18:53:16 2009
>>> New Revision: 78127
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=78127&view=rev
>>> Log:
>>> Make ExecutionEngine::updateGlobalMapping(GV, NULL) properly remove
>>> GV's old
>>> address from the reverse mapping, and add a test that this works  
>>> now.
>>
>> Hey Jeffrey,
>>
>> Should the GlobalMapping move to using AssertingVH?  I think that
>> would define this class of bugs away.
>
> I'd actually like to move them and the other maps in ExecutionEngine
> to using CallbackVH so that users don't have to deal with
> removing/changing global mappings unless the object actually moves.

That would also be nice. :)

> But whether or not I do that, this bug made it impossible to do  
> things like:
>
> T1 *obj1 = new T1;
> engine.addGlobalMapping(GV, obj1);
> ...
> engine.updateGlobalMapping(GV, NULL);
> delete obj1;
> ...
> T2 *obj2 = new T2;  // Happens to land at the same address.
> engine.addGlobalMapping(UnrelatedGV, obj2);
>
> and I don't see how AssertingVH would make any of that illegal or  
> work better.

If "updateglobalmapping" to null deletes the entry out of the map,  
then assertingvh should work fine.

-Chris