Add ASan history threads into process_sp->GetExtendedThreadList, so they don't get freed too early

[kubamracek]

Summary

1. added the "process_sp->GetExtendedThreadList().AddThread (new_thread_sp);"

line when the ThreadSP is created, and also added this into the test case

2. refactored the duplicate code into a CreateHistoryThreadFromValueObject function that is called twice.

[jasonmolenda]

Looks good, a couple changes to match the lldb coding style better; please commit.

source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp

119

Please split into two lines,

if (count <= 0)
  return;

124

open brace ('{') on the next line,

for (int i = 0; i < count; i++)
{

126

Pls plist condition expr & command so they're on separate lines.

[kubamracek]

Thanks. Landed in r218323.

source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp

memory_history_asan_command_format = R"(
     } t;
 
     t.alloc_count = ((size_t (*) (void *, void **, size_t, int *))__asan_get_alloc_stack)((void *)0x%)" PRIx64 R"(, t.alloc_trace, 256, &t.alloc_tid);
     t.free_count = ((size_t (*) (void *, void **, size_t, int *))__asan_get_free_stack)((void *)0x%)" PRIx64 R"(, t.free_trace, 256, &t.free_tid);
 
     t;
 )";
 
+static void CreateHistoryThreadFromValueObject(ProcessSP process_sp, ValueObjectSP return_value_sp, const char *type, char *thread_name, HistoryThreads & result)
+{
+    std::string count_path = "." + std::string(type) + "_count";
+    std::string tid_path = "." + std::string(type) + "_tid";
+    std::string trace_path = "." + std::string(type) + "_trace";
+
+    int count = return_value_sp->GetValueForExpressionPath(count_path.c_str())->GetValueAsUnsigned(0);
+    tid_t tid = return_value_sp->GetValueForExpressionPath(tid_path.c_str())->GetValueAsUnsigned(0);
+
+    if (count <= 0) return;
+
+    ValueObjectSP trace_sp = return_value_sp->GetValueForExpressionPath(trace_path.c_str());
+
+    std::vector<lldb::addr_t> pcs;
+    for (int i = 0; i < count; i++) {
+        addr_t pc = trace_sp->GetChildAtIndex(i, true)->GetValueAsUnsigned(0);
+        if (pc == 0 || pc == 1 || pc == LLDB_INVALID_ADDRESS) continue;
+        pcs.push_back(pc);
+    }
+    
+    HistoryThread *history_thread = new HistoryThread(*process_sp, tid, pcs, 0, false);
+    ThreadSP new_thread_sp(history_thread);
+    // let's use thread name for the type of history thread, since history threads don't have names anyway
+    history_thread->SetThreadName(thread_name);
+    // Save this in the Process' ExtendedThreadList so a strong pointer retains the object
+    process_sp->GetExtendedThreadList().AddThread (new_thread_sp);
+    result.push_back(new_thread_sp);
+}
+
 #define GET_STACK_FUNCTION_TIMEOUT_USEC 2*1000*1000
 
 HistoryThreads
 MemoryHistoryASan::GetHistoryThreads(lldb::addr_t address)
 {
     ProcessSP process_sp = m_process_sp;
     ThreadSP thread_sp = m_process_sp->GetThreadList().GetSelectedThread();
     StackFrameSP frame_sp = thread_sp->GetSelectedFrame();
if (m_process_sp->GetTarget().EvaluateExpression(expr.GetData(), frame_sp.get(), return_value_sp, options) != eExpressionCompleted)
         return HistoryThreads();
     }
     if (!return_value_sp)
     {
         return HistoryThreads();
     }
     
     HistoryThreads result;
-    
-    int alloc_count = return_value_sp->GetValueForExpressionPath(".alloc_count")->GetValueAsUnsigned(0);
-    int free_count = return_value_sp->GetValueForExpressionPath(".free_count")->GetValueAsUnsigned(0);
-    tid_t alloc_tid = return_value_sp->GetValueForExpressionPath(".alloc_tid")->GetValueAsUnsigned(0);
-    tid_t free_tid = return_value_sp->GetValueForExpressionPath(".free_tid")->GetValueAsUnsigned(0);
-    
-    if (alloc_count > 0)
-    {
-        std::vector<lldb::addr_t> pcs;
-        ValueObjectSP trace_sp = return_value_sp->GetValueForExpressionPath(".alloc_trace");
-        for (int i = 0; i < alloc_count; i++) {
-            addr_t pc = trace_sp->GetChildAtIndex(i, true)->GetValueAsUnsigned(0);
-            pcs.push_back(pc);
-        }
-        
-        HistoryThread *history_thread = new HistoryThread(*process_sp, alloc_tid, pcs, 0, false);
-        ThreadSP new_thread_sp(history_thread);
-        // let's use thread name for the type of history thread, since history threads don't have names anyway
-        history_thread->SetThreadName("Memory allocated at");
-        result.push_back(new_thread_sp);
-    }
-    
-    if (free_count > 0)
-    {
-        std::vector<lldb::addr_t> pcs;
-        ValueObjectSP trace_sp = return_value_sp->GetValueForExpressionPath(".free_trace");
-        for (int i = 0; i < free_count; i++) {
-            addr_t pc = trace_sp->GetChildAtIndex(i, true)->GetValueAsUnsigned(0);
-            pcs.push_back(pc);
-        }
-        
-        HistoryThread *history_thread = new HistoryThread(*process_sp, free_tid, pcs, 0, false);
-        ThreadSP new_thread_sp(history_thread);
-        // let's use thread name for the type of history thread, since history threads don't have names anyway
-        history_thread->SetThreadName("Memory deallocated at");
-        result.push_back(new_thread_sp);
-    }
+    CreateHistoryThreadFromValueObject(process_sp, return_value_sp, "alloc", "Memory allocated at", result);
+    CreateHistoryThreadFromValueObject(process_sp, return_value_sp, "free", "Memory deallocated at", result);
     
     return result;
 }

test/functionalities/asan/TestAsan.py

def asan_tests (self):
         self.assertEqual(history_thread.frames[1].GetLineEntry().GetFileSpec().GetFilename(), "main.c")
         self.assertEqual(history_thread.frames[1].GetLineEntry().GetLine(), self.line_malloc)
         
         history_thread = threads.GetThreadAtIndex(1)
         self.assertTrue(history_thread.num_frames >= 2)
         self.assertEqual(history_thread.frames[1].GetLineEntry().GetFileSpec().GetFilename(), "main.c")
         self.assertEqual(history_thread.frames[1].GetLineEntry().GetLine(), self.line_free)
 
+        # let's free the container (SBThreadCollection) and see if the SBThreads still live
+        threads = None
+        self.assertTrue(history_thread.num_frames >= 2)
+        self.assertEqual(history_thread.frames[1].GetLineEntry().GetFileSpec().GetFilename(), "main.c")
+        self.assertEqual(history_thread.frames[1].GetLineEntry().GetLine(), self.line_free)
+
         # now let's break when an ASan report occurs and try the API then
         self.runCmd("breakpoint set -n __asan_report_error")
 
         self.runCmd("continue")
 
         # the stop reason of the thread should be breakpoint.
         self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
             substrs = ['stopped', 'stop reason = breakpoint'])