[Lldb-commits] [PATCH] Introduce a new UnwindAssemblySP shared pointer type and use it to fix a memory leak. We are never deleting UnwindAssembly.

[Jean-Daniel Dupas]

Thank you for the explanation. I missed that point when I read about 'shared_from_this'.

Le 3 févr. 2014 à 20:49, Greg Clayton <gclayton at apple.com> a écrit :

> UnwindAssembly::FindPlugin() needs to be modified to return a UnwindAssemblySP and when each UnwindAssembly object is created you need to immediately put it into a shared pointer upon construction. When using enable_shared_from_this() you _must_ create the object by putting it into a shared pointer right away, otherwise when you use "unwinder.shared_from_this()" you will crash as the std::weak_ptr inside the enable_shared_from_this() class won't have been initialized correctly.
> 
> On Feb 2, 2014, at 6:00 AM, Jean-Daniel Dupas <devlists at shadowlab.org> wrote:
> 
>> Add enable_shared_from_this to UnwindAssembly for consistency with other sharable classes.
>> 
>> http://llvm-reviews.chandlerc.com/D2491
>> 
>> CHANGE SINCE LAST DIFF
>> http://llvm-reviews.chandlerc.com/D2491?vs=6308&id=6821#toc
>> 
>> Files:
>> include/lldb/Symbol/FuncUnwinders.h
>> include/lldb/Symbol/UnwindTable.h
>> include/lldb/Target/UnwindAssembly.h
>> include/lldb/lldb-forward.h
>> source/Symbol/FuncUnwinders.cpp
>> source/Symbol/UnwindTable.cpp
>> 
>> Index: include/lldb/Symbol/FuncUnwinders.h
>> ===================================================================
>> --- include/lldb/Symbol/FuncUnwinders.h
>> +++ include/lldb/Symbol/FuncUnwinders.h
>> @@ -31,7 +31,7 @@
>>    // instructions are finished for migrating breakpoints past the 
>>    // stack frame setup instructions when we don't have line table information.
>> 
>> -    FuncUnwinders (lldb_private::UnwindTable& unwind_table, lldb_private::UnwindAssembly *assembly_profiler, AddressRange range);
>> +    FuncUnwinders (lldb_private::UnwindTable& unwind_table, const lldb::UnwindAssemblySP& assembly_profiler, AddressRange range);
>> 
>>    ~FuncUnwinders ();
>> 
>> @@ -77,7 +77,7 @@
>> 
>> private:
>>    UnwindTable& m_unwind_table;
>> -    UnwindAssembly *m_assembly_profiler;
>> +    lldb::UnwindAssemblySP m_assembly_profiler;
>>    AddressRange m_range;
>> 
>>    Mutex m_mutex;
>> Index: include/lldb/Symbol/UnwindTable.h
>> ===================================================================
>> --- include/lldb/Symbol/UnwindTable.h
>> +++ include/lldb/Symbol/UnwindTable.h
>> @@ -57,7 +57,7 @@
>> 
>>    bool                m_initialized;  // delay some initialization until ObjectFile is set up
>> 
>> -    UnwindAssembly* m_assembly_profiler;
>> +    lldb::UnwindAssemblySP m_assembly_profiler;
>> 
>>    DWARFCallFrameInfo* m_eh_frame;
>> 
>> Index: include/lldb/Target/UnwindAssembly.h
>> ===================================================================
>> --- include/lldb/Target/UnwindAssembly.h
>> +++ include/lldb/Target/UnwindAssembly.h
>> @@ -17,6 +17,7 @@
>> namespace lldb_private {
>> 
>> class UnwindAssembly :
>> +   public std::enable_shared_from_this<UnwindAssembly>,
>>   public PluginInterface
>> {
>> public:
>> Index: include/lldb/lldb-forward.h
>> ===================================================================
>> --- include/lldb/lldb-forward.h
>> +++ include/lldb/lldb-forward.h
>> @@ -384,6 +384,7 @@
>> #ifndef LLDB_DISABLE_PYTHON
>>    typedef std::shared_ptr<lldb_private::ScriptedSyntheticChildren> ScriptedSyntheticChildrenSP;
>> #endif
>> +    typedef std::shared_ptr<lldb_private::UnwindAssembly> UnwindAssemblySP;
>>    typedef std::shared_ptr<lldb_private::UnwindPlan> UnwindPlanSP;
>>    typedef lldb_private::SharingPtr<lldb_private::ValueObject> ValueObjectSP;
>>    typedef std::shared_ptr<lldb_private::Value> ValueSP;
>> Index: source/Symbol/FuncUnwinders.cpp
>> ===================================================================
>> --- source/Symbol/FuncUnwinders.cpp
>> +++ source/Symbol/FuncUnwinders.cpp
>> @@ -28,7 +28,7 @@
>> FuncUnwinders::FuncUnwinders
>> (
>>    UnwindTable& unwind_table, 
>> -    UnwindAssembly *assembly_profiler, 
>> +    const lldb::UnwindAssemblySP& assembly_profiler,
>>    AddressRange range
>> ) : 
>>    m_unwind_table(unwind_table), 
>> Index: source/Symbol/UnwindTable.cpp
>> ===================================================================
>> --- source/Symbol/UnwindTable.cpp
>> +++ source/Symbol/UnwindTable.cpp
>> @@ -57,7 +57,7 @@
>>    ArchSpec arch;
>>    if (m_object_file.GetArchitecture (arch))
>>    {
>> -        m_assembly_profiler = UnwindAssembly::FindPlugin (arch);
>> +        m_assembly_profiler.reset(UnwindAssembly::FindPlugin (arch));
>>        m_initialized = true;
>>    }
>> }
>> <D2491.2.patch>_______________________________________________
>> lldb-commits mailing list
>> lldb-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
> 

-- Jean-Daniel