Hi, in the future, please add "lldb-commits" as a subscriber when you are having your patches reviewed.
pl On 3 November 2015 at 06:24, Ravitheja Addepally via lldb-commits <lldb-commits@lists.llvm.org> wrote: > Author: ravitheja > Date: Tue Nov 3 08:24:24 2015 > New Revision: 251917 > > URL: http://llvm.org/viewvc/llvm-project?rev=251917&view=rev > Log: > Changes for Bug 25251 > > Summary: > The solution to bug 24074,rL249673 needed > to parse the function information from the Dwarf in order > to set the SymbolContext. For that, GetFunction was called > for the parent in GetTypeForDIE, which parses the > ChildParameters and in the flow, GetTypeForDIE was called > for one of the sibling die and so an infinite > loop was triggered by calling GetFunction repeatedly for the > same function. > > The changes in this revision modify the GetTypeForDIE to only > resolve the function context in the Type Lookup flow and so > prevent the infinite loop. > > A testcase has also been added to check for regression in the > future and a test vector had been added to the testcase of > 24074. > > Reviewers: jingham, tberghammer, clayborg > > Differential Revision: http://reviews.llvm.org/D14202 > > Added: > > lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/ > > lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/Makefile > > lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/ParallelTask.cpp > (with props) > > lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/TestBacktraceAll.py > Modified: > lldb/trunk/packages/Python/lldbsuite/test/lang/c/typedef/Testtypedef.py > lldb/trunk/packages/Python/lldbsuite/test/lang/c/typedef/main.c > lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp > lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h > > Added: > lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/Makefile > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/Makefile?rev=251917&view=auto > ============================================================================== > --- > lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/Makefile > (added) > +++ > lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/Makefile > Tue Nov 3 08:24:24 2015 > @@ -0,0 +1,6 @@ > +LEVEL = ../../../make > + > +CXXFLAGS += -std=c++11 > +CXX_SOURCES := ParallelTask.cpp > +ENABLE_STD_THREADS := YES > +include $(LEVEL)/Makefile.rules > > Added: > lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/ParallelTask.cpp > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/ParallelTask.cpp?rev=251917&view=auto > ============================================================================== > --- > lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/ParallelTask.cpp > (added) > +++ > lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/ParallelTask.cpp > Tue Nov 3 08:24:24 2015 > @@ -0,0 +1,151 @@ > +#include <cstdint> > +#include <thread> > +#include <vector> > +#include <queue> > +#include <future> > +#include <iostream> > +#include <cassert> > + > +class TaskPoolImpl > +{ > +public: > + TaskPoolImpl(uint32_t num_threads) : > + m_stop(false) > + { > + for (uint32_t i = 0; i < num_threads; ++i) > + m_threads.emplace_back(Worker, this); > + } > + > + ~TaskPoolImpl() > + { > + Stop(); > + } > + > + template<typename F, typename... Args> > + std::future<typename std::result_of<F(Args...)>::type> > + AddTask(F&& f, Args&&... args) > + { > + auto task = std::make_shared<std::packaged_task<typename > std::result_of<F(Args...)>::type()>>( > + std::bind(std::forward<F>(f), std::forward<Args>(args)...)); > + > + std::unique_lock<std::mutex> lock(m_tasks_mutex); > + assert(!m_stop && "Can't add task to TaskPool after it is stopped"); > + m_tasks.emplace([task](){ (*task)(); }); > + lock.unlock(); > + m_tasks_cv.notify_one(); > + > + return task->get_future(); > + } > + > + void > + Stop() > + { > + std::unique_lock<std::mutex> lock(m_tasks_mutex); > + m_stop = true; > + m_tasks_mutex.unlock(); > + m_tasks_cv.notify_all(); > + for (auto& t : m_threads) > + t.join(); > + } > + > +private: > + static void > + Worker(TaskPoolImpl* pool) > + { > + while (true) > + { > + std::unique_lock<std::mutex> lock(pool->m_tasks_mutex); > + if (pool->m_tasks.empty()) > + pool->m_tasks_cv.wait(lock, [pool](){ return > !pool->m_tasks.empty() || pool->m_stop; }); > + if (pool->m_tasks.empty()) > + break; > + > + std::function<void()> f = pool->m_tasks.front(); > + pool->m_tasks.pop(); > + lock.unlock(); > + > + f(); > + } > + } > + > + std::queue<std::function<void()>> m_tasks; > + std::mutex m_tasks_mutex; > + std::condition_variable m_tasks_cv; > + bool m_stop; > + std::vector<std::thread> m_threads; > +}; > + > +class TaskPool > +{ > +public: > + // Add a new task to the thread pool and return a std::future belongs > for the newly created task. > + // The caller of this function have to wait on the future for this task > to complete. > + template<typename F, typename... Args> > + static std::future<typename std::result_of<F(Args...)>::type> > + AddTask(F&& f, Args&&... args) > + { > + return GetImplementation().AddTask(std::forward<F>(f), > std::forward<Args>(args)...); > + } > + > + // Run all of the specified tasks on the thread pool and wait until all > of them are finished > + // before returning > + template<typename... T> > + static void > + RunTasks(T&&... t) > + { > + RunTaskImpl<T...>::Run(std::forward<T>(t)...); > + } > + > +private: > + static TaskPoolImpl& > + GetImplementation() > + { > + static TaskPoolImpl > g_task_pool_impl(std::thread::hardware_concurrency()); > + return g_task_pool_impl; > + } > + > + template<typename... T> > + struct RunTaskImpl; > +}; > + > +template<typename H, typename... T> > +struct TaskPool::RunTaskImpl<H, T...> > +{ > + static void > + Run(H&& h, T&&... t) > + { > + auto f = AddTask(std::forward<H>(h)); > + RunTaskImpl<T...>::Run(std::forward<T>(t)...); > + f.wait(); > + } > +}; > + > +template<> > +struct TaskPool::RunTaskImpl<> > +{ > + static void > + Run() {} > +}; > + > +int main() > +{ > + std::vector<std::future<uint32_t>> tasks; > + for (int i = 0; i < 100000; ++i) > + { > + tasks.emplace_back(TaskPool::AddTask([](int i){ > + uint32_t s = 0; > + for (int j = 0; j <= i; ++j) > + s += j; > + return s; > + }, > + i)); > + } > + > + for (auto& it : tasks) // Set breakpoint here > + it.wait(); > + > + TaskPool::RunTasks( > + []() { return 1; }, > + []() { return "aaaa"; } > + ); > +} > > Propchange: > lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/ParallelTask.cpp > ------------------------------------------------------------------------------ > svn:executable = * > > Added: > lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/TestBacktraceAll.py > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/TestBacktraceAll.py?rev=251917&view=auto > ============================================================================== > --- > lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/TestBacktraceAll.py > (added) > +++ > lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/backtrace_all/TestBacktraceAll.py > Tue Nov 3 08:24:24 2015 > @@ -0,0 +1,54 @@ > +""" > +Test regression for Bug 25251. > +""" > + > +import os, time > +import unittest2 > +import lldb > +from lldbsuite.test.lldbtest import * > +import lldbsuite.test.lldbutil as lldbutil > + > +class BreakpointAfterJoinTestCase(TestBase): > + > + mydir = TestBase.compute_mydir(__file__) > + > + def setUp(self): > + # Call super's setUp(). > + TestBase.setUp(self) > + # Find the line number for our breakpoint. > + self.breakpoint = line_number('ParallelTask.cpp', '// Set breakpoint > here') > + > + def test(self): > + """Test breakpoint handling after a thread join.""" > + self.build(dictionary=self.getBuildFlags()) > + > + exe = os.path.join(os.getcwd(), "a.out") > + self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET) > + > + # This should create a breakpoint > + lldbutil.run_break_set_by_file_and_line (self, "ParallelTask.cpp", > self.breakpoint, num_expected_locations=-1) > + > + # The breakpoint list should show 1 location. > + self.expect("breakpoint list -f", "Breakpoint location shown > correctly", > + substrs = ["1: file = 'ParallelTask.cpp', line = %d, exact_match > = 0" % self.breakpoint]) > + > + # Run the program. > + self.runCmd("run", RUN_SUCCEEDED) > + > + # The stop reason of the thread should be breakpoint. > + self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT, > + substrs = ['stopped', > + 'stop reason = breakpoint']) > + > + # This should not result in a segmentation fault > + self.expect("thread backtrace all", STOPPED_DUE_TO_BREAKPOINT, > + substrs = ["stop reason = breakpoint 1."]) > + > + # Run to completion > + self.runCmd("continue") > + > +if __name__ == '__main__': > + import atexit > + lldb.SBDebugger.Initialize() > + atexit.register(lambda: lldb.SBDebugger.Terminate()) > + unittest2.main() > > Modified: > lldb/trunk/packages/Python/lldbsuite/test/lang/c/typedef/Testtypedef.py > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/typedef/Testtypedef.py?rev=251917&r1=251916&r2=251917&view=diff > ============================================================================== > --- lldb/trunk/packages/Python/lldbsuite/test/lang/c/typedef/Testtypedef.py > (original) > +++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/typedef/Testtypedef.py > Tue Nov 3 08:24:24 2015 > @@ -32,7 +32,7 @@ class TypedefTestCase(TestBase): > """Test 'image lookup -t a' at different scopes and check for > correct display.""" > exe = os.path.join(os.getcwd(), "a.out") > self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET) > - typearray = ("float", "float", "char", "float", "int", "double", > "float", "float") > + typearray = ("float", "float", "char", "double *", "float", "int", > "double", "float", "float") > arraylen = len(typearray)+1 > for i in range(1,arraylen): > loc_line = line_number('main.c', '// Set break point ' + str(i) > + '.') > > Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/c/typedef/main.c > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/typedef/main.c?rev=251917&r1=251916&r2=251917&view=diff > ============================================================================== > --- lldb/trunk/packages/Python/lldbsuite/test/lang/c/typedef/main.c (original) > +++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/typedef/main.c Tue Nov > 3 08:24:24 2015 > @@ -6,6 +6,11 @@ > // License. See LICENSE.TXT for details. > // > > //===----------------------------------------------------------------------===// > +void test() > +{ > + typedef double * a; > + a b = 0; // Set break point 4. > +} > int main (int argc, char const *argv[]) > { > typedef float a; > @@ -16,25 +21,26 @@ int main (int argc, char const *argv[]) > typedef char a; > i++; > a charvariable = 'a'; // Set break point 3. > + test(); > } > { > int c = 0; > - c++; // Set break point 4. > + c++; // Set break point 5. > for(i = 0 ; i < 1 ; i++) > { > typedef int a; > a b; > - b = 7; // Set break point 5. > + b = 7; // Set break point 6. > } > for(i = 0 ; i < 1 ; i++) > { > typedef double a; > a b; > - b = 3.14; // Set break point 6. > + b = 3.14; // Set break point 7. > } > - c = 1; // Set break point 7. > + c = 1; // Set break point 8. > } > floatvariable = 2.5; > - floatvariable = 2.8; // Set break point 8. > + floatvariable = 2.8; // Set break point 9. > return 0; > } > > Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp?rev=251917&r1=251916&r2=251917&view=diff > ============================================================================== > --- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (original) > +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Tue Nov 3 > 08:24:24 2015 > @@ -1559,14 +1559,14 @@ SymbolFileDWARF::CompleteType (CompilerT > } > > Type* > -SymbolFileDWARF::ResolveType (const DWARFDIE &die, bool > assert_not_being_parsed) > +SymbolFileDWARF::ResolveType (const DWARFDIE &die, bool > assert_not_being_parsed, bool resolve_function_context) > { > if (die) > { > Type *type = GetDIEToType().lookup (die.GetDIE()); > > if (type == NULL) > - type = GetTypeForDIE (die).get(); > + type = GetTypeForDIE (die, resolve_function_context).get(); > > if (assert_not_being_parsed) > { > @@ -2927,7 +2927,7 @@ SymbolFileDWARF::FindTypes (const Symbol > if (!DIEInDeclContext(parent_decl_ctx, die)) > continue; // The containing decl contexts don't match > > - Type *matching_type = ResolveType (die); > + Type *matching_type = ResolveType (die, true, true); > if (matching_type) > { > // We found a type pointer, now find the shared pointer > form our type list > @@ -3065,7 +3065,7 @@ SymbolFileDWARF::FindNamespace (const Sy > } > > TypeSP > -SymbolFileDWARF::GetTypeForDIE (const DWARFDIE &die) > +SymbolFileDWARF::GetTypeForDIE (const DWARFDIE &die, bool > resolve_function_context) > { > TypeSP type_sp; > if (die) > @@ -3084,7 +3084,7 @@ SymbolFileDWARF::GetTypeForDIE (const DW > parent_die = parent_die->GetParent(); > } > SymbolContext sc_backup = sc; > - if (parent_die != nullptr && > !GetFunction(DWARFDIE(die.GetCU(),parent_die), sc)) > + if (resolve_function_context && parent_die != nullptr && > !GetFunction(DWARFDIE(die.GetCU(),parent_die), sc)) > sc = sc_backup; > > type_sp = ParseType(sc, die, NULL); > @@ -3270,7 +3270,7 @@ SymbolFileDWARF::FindCompleteObjCDefinit > > if (try_resolving_type) > { > - Type *resolved_type = ResolveType (type_die, false); > + Type *resolved_type = ResolveType (type_die, false, > true); > if (resolved_type && resolved_type != > DIE_IS_BEING_PARSED) > { > DEBUG_PRINTF ("resolved 0x%8.8" PRIx64 " from %s > to 0x%8.8" PRIx64 " (cu 0x%8.8" PRIx64 ")\n", > > Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h?rev=251917&r1=251916&r2=251917&view=diff > ============================================================================== > --- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h (original) > +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h Tue Nov 3 > 08:24:24 2015 > @@ -148,7 +148,8 @@ public: > > lldb_private::Type * > ResolveType (const DWARFDIE &die, > - bool assert_not_being_parsed = true); > + bool assert_not_being_parsed = true, > + bool resolve_function_context = false); > > lldb_private::CompilerDecl > GetDeclForUID (lldb::user_id_t uid) override; > @@ -434,7 +435,7 @@ protected: > lldb_private::SymbolContextList& sc_list); > > lldb::TypeSP > - GetTypeForDIE (const DWARFDIE &die); > + GetTypeForDIE (const DWARFDIE &die, bool resolve_function_context = > false); > > void > Index(); > > > _______________________________________________ > lldb-commits mailing list > lldb-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
