From 0ea010aecb591df0d857de32945bb4ba840a67e4 Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Fri, 12 Feb 2016 00:03:19 +0000 Subject: [PATCH] When calling TypeSystemMap::Clear, objects being destroyed in the process of clearing the map ended up calling back into the TypeSystemMap to do lookups. Not a good idea, and in this case it would cause a deadlock. You would only see this when replacing the target contents after an exec, and only if you had stopped before the exec, evaluated an expression, then continued on to the point where you did the exec. Fixed this by making sure the TypeSystemMap::Clear tears down the TypeSystems in the map before clearing the map. I also add an expression before exec to the TestExec.py so that we'll catch this issue if it crops up again in the future. llvm-svn: 260624 --- lldb/include/lldb/Symbol/ClangASTContext.h | 5 ++- lldb/include/lldb/Symbol/TypeSystem.h | 8 ++++ .../test/functionalities/exec/TestExec.py | 33 ++++++++-------- lldb/source/Symbol/ClangASTContext.cpp | 38 +++++++++++-------- lldb/source/Symbol/TypeSystem.cpp | 22 ++++++++++- 5 files changed, 70 insertions(+), 36 deletions(-) diff --git a/lldb/include/lldb/Symbol/ClangASTContext.h b/lldb/include/lldb/Symbol/ClangASTContext.h index 3eb7446f6d07..d30fb2eb2237 100644 --- a/lldb/include/lldb/Symbol/ClangASTContext.h +++ b/lldb/include/lldb/Symbol/ClangASTContext.h @@ -61,6 +61,9 @@ public: ~ClangASTContext() override; + void + Finalize() override; + //------------------------------------------------------------------ // PluginInterface functions //------------------------------------------------------------------ @@ -127,7 +130,7 @@ public: void Clear(); - + const char * GetTargetTriple (); diff --git a/lldb/include/lldb/Symbol/TypeSystem.h b/lldb/include/lldb/Symbol/TypeSystem.h index 7df974fcd32b..83372ad4aaf6 100644 --- a/lldb/include/lldb/Symbol/TypeSystem.h +++ b/lldb/include/lldb/Symbol/TypeSystem.h @@ -92,6 +92,12 @@ public: static lldb::TypeSystemSP CreateInstance (lldb::LanguageType language, Target *target); + + // Free up any resources associated with this TypeSystem. Done before removing + // all the TypeSystems from the TypeSystemMap. + virtual void + Finalize() {} + virtual DWARFASTParser * GetDWARFParser () { @@ -584,6 +590,8 @@ protected: TypeSystemMap (); ~TypeSystemMap(); + // Clear calls Finalize on all the TypeSystems managed by this map, and then + // empties the map. void Clear (); diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py b/lldb/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py index e84ddd6cd28a..912d51fb6b02 100644 --- a/lldb/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py +++ b/lldb/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py @@ -56,15 +56,18 @@ class ExecTestCase(TestBase): self.assertTrue(process.GetState() == lldb.eStateStopped, STOPPED_DUE_TO_BREAKPOINT) - thread = process.GetThreadAtIndex (0) + threads = lldbutil.get_threads_stopped_at_breakpoint(process, breakpoint) + self.assertTrue(len(threads) == 1) - self.assertTrue (thread.IsValid(), - "Process stopped at 'main' should have a valid thread"); + # We had a deadlock tearing down the TypeSystemMap on exec, but only if some + # expression had been evaluated. So make sure we do that here so the teardown + # is not trivial. - stop_reason = thread.GetStopReason() - - self.assertTrue (stop_reason == lldb.eStopReasonBreakpoint, - "Thread in process stopped in 'main' should have a stop reason of eStopReasonBreakpoint"); + thread = threads[0] + value = thread.frames[0].EvaluateExpression("1 + 2") + self.assertTrue(value.IsValid(), "Expression evaluated successfully") + int_value = value.GetValueAsSigned() + self.assertTrue(int_value == 3, "Expression got the right result.") # Run and we should stop due to exec process.Continue() @@ -72,15 +75,11 @@ class ExecTestCase(TestBase): self.assertTrue(process.GetState() == lldb.eStateStopped, "Process should be stopped at __dyld_start") - thread = process.GetThreadAtIndex (0) - - self.assertTrue (thread.IsValid(), - "Process stopped at exec should have a valid thread"); - - stop_reason = thread.GetStopReason() - - self.assertTrue (stop_reason == lldb.eStopReasonExec, - "Thread in process stopped on exec should have a stop reason of eStopReasonExec"); - + threads = lldbutil.get_stopped_threads(process, lldb.eStopReasonExec) + self.assertTrue(len(threads) == 1, "We got a thread stopped for exec.") + # Run and we should stop at breakpoint in main after exec process.Continue() + + threads = lldbutil.get_threads_stopped_at_breakpoint(process, breakpoint) + self.assertTrue(len(threads) == 1, "Stopped at breakpoint in exec'ed process.") diff --git a/lldb/source/Symbol/ClangASTContext.cpp b/lldb/source/Symbol/ClangASTContext.cpp index f953ddb21e4c..1d4d265e7025 100644 --- a/lldb/source/Symbol/ClangASTContext.cpp +++ b/lldb/source/Symbol/ClangASTContext.cpp @@ -333,22 +333,7 @@ ClangASTContext::ClangASTContext (const char *target_triple) : //---------------------------------------------------------------------- ClangASTContext::~ClangASTContext() { - if (m_ast_ap.get()) - { - GetASTMap().Erase(m_ast_ap.get()); - if (!m_ast_owned) - m_ast_ap.release(); - } - - m_builtins_ap.reset(); - m_selector_table_ap.reset(); - m_identifier_table_ap.reset(); - m_target_info_ap.reset(); - m_target_options_rp.reset(); - m_diagnostics_engine_ap.reset(); - m_source_manager_ap.reset(); - m_language_options_ap.reset(); - m_ast_ap.reset(); + Finalize(); } ConstString @@ -472,6 +457,27 @@ ClangASTContext::Terminate() PluginManager::UnregisterPlugin (CreateInstance); } +void +ClangASTContext::Finalize() +{ + if (m_ast_ap.get()) + { + GetASTMap().Erase(m_ast_ap.get()); + if (!m_ast_owned) + m_ast_ap.release(); + } + + m_builtins_ap.reset(); + m_selector_table_ap.reset(); + m_identifier_table_ap.reset(); + m_target_info_ap.reset(); + m_target_options_rp.reset(); + m_diagnostics_engine_ap.reset(); + m_source_manager_ap.reset(); + m_language_options_ap.reset(); + m_ast_ap.reset(); + m_scratch_ast_source_ap.reset(); +} void ClangASTContext::Clear() diff --git a/lldb/source/Symbol/TypeSystem.cpp b/lldb/source/Symbol/TypeSystem.cpp index 5b8e935c71d7..5354f22a10fa 100644 --- a/lldb/source/Symbol/TypeSystem.cpp +++ b/lldb/source/Symbol/TypeSystem.cpp @@ -176,8 +176,26 @@ TypeSystemMap::~TypeSystemMap() void TypeSystemMap::Clear () { - Mutex::Locker locker (m_mutex); - m_map.clear(); + collection map; + { + Mutex::Locker locker (m_mutex); + map = m_map; + } + std::set visited; + for (auto pair : map) + { + TypeSystem *type_system = pair.second.get(); + if (type_system && !visited.count(type_system)) + { + visited.insert(type_system); + type_system->Finalize(); + } + } + map.clear(); + { + Mutex::Locker locker (m_mutex); + m_map.clear(); + } }