diff --git a/lldb/include/lldb/Interpreter/CommandObject.h b/lldb/include/lldb/Interpreter/CommandObject.h --- a/lldb/include/lldb/Interpreter/CommandObject.h +++ b/lldb/include/lldb/Interpreter/CommandObject.h @@ -276,14 +276,13 @@ /// The command arguments. /// /// \return - /// nullptr if there is no special repeat command - it will use the + /// llvm::None if there is no special repeat command - it will use the /// current command line. - /// Otherwise a pointer to the command to be repeated. - /// If the returned string is the empty string, the command won't be - /// repeated. - virtual const char *GetRepeatCommand(Args ¤t_command_args, - uint32_t index) { - return nullptr; + /// Otherwise a std::string containing the command to be repeated. + /// If the string is empty, the command won't be allow repeating. + virtual llvm::Optional + GetRepeatCommand(Args ¤t_command_args, uint32_t index) { + return llvm::None; } bool HasOverrideCallback() const { diff --git a/lldb/include/lldb/Interpreter/CommandObjectMultiword.h b/lldb/include/lldb/Interpreter/CommandObjectMultiword.h --- a/lldb/include/lldb/Interpreter/CommandObjectMultiword.h +++ b/lldb/include/lldb/Interpreter/CommandObjectMultiword.h @@ -55,8 +55,8 @@ void HandleCompletion(CompletionRequest &request) override; - const char *GetRepeatCommand(Args ¤t_command_args, - uint32_t index) override; + llvm::Optional GetRepeatCommand(Args ¤t_command_args, + uint32_t index) override; bool Execute(const char *args_string, CommandReturnObject &result) override; @@ -120,8 +120,8 @@ HandleArgumentCompletion(CompletionRequest &request, OptionElementVector &opt_element_vector) override; - const char *GetRepeatCommand(Args ¤t_command_args, - uint32_t index) override; + llvm::Optional GetRepeatCommand(Args ¤t_command_args, + uint32_t index) override; /// \return /// An error message to be displayed when the command is executed (i.e. diff --git a/lldb/source/API/SBCommandInterpreter.cpp b/lldb/source/API/SBCommandInterpreter.cpp --- a/lldb/source/API/SBCommandInterpreter.cpp +++ b/lldb/source/API/SBCommandInterpreter.cpp @@ -52,15 +52,15 @@ bool IsRemovable() const override { return true; } /// More documentation is available in lldb::CommandObject::GetRepeatCommand, - /// but in short, if nullptr is returned, the previous command will be + /// but in short, if llvm::None is returned, the previous command will be /// repeated, and if an empty string is returned, no commands will be /// executed. - const char *GetRepeatCommand(Args ¤t_command_args, - uint32_t index) override { + llvm::Optional GetRepeatCommand(Args ¤t_command_args, + uint32_t index) override { if (!m_auto_repeat_command) - return nullptr; + return llvm::None; else - return m_auto_repeat_command->c_str(); + return m_auto_repeat_command; } protected: diff --git a/lldb/source/Breakpoint/BreakpointLocationCollection.cpp b/lldb/source/Breakpoint/BreakpointLocationCollection.cpp --- a/lldb/source/Breakpoint/BreakpointLocationCollection.cpp +++ b/lldb/source/Breakpoint/BreakpointLocationCollection.cpp @@ -123,8 +123,11 @@ size_t i = 0; size_t prev_size = GetSize(); while (i < prev_size) { - // ShouldStop can remove the breakpoint from the list - if (GetByIndex(i)->ShouldStop(context)) + // ShouldStop can remove the breakpoint from the list, or even delete + // it, so we should + BreakpointLocationSP cur_loc_sp = GetByIndex(i); + BreakpointSP keep_bkpt_alive_sp = cur_loc_sp->GetBreakpoint().shared_from_this(); + if (cur_loc_sp->ShouldStop(context)) shouldStop = true; if (prev_size == GetSize()) diff --git a/lldb/source/Commands/CommandObjectCommands.cpp b/lldb/source/Commands/CommandObjectCommands.cpp --- a/lldb/source/Commands/CommandObjectCommands.cpp +++ b/lldb/source/Commands/CommandObjectCommands.cpp @@ -56,9 +56,9 @@ ~CommandObjectCommandsSource() override = default; - const char *GetRepeatCommand(Args ¤t_command_args, - uint32_t index) override { - return ""; + llvm::Optional GetRepeatCommand(Args ¤t_command_args, + uint32_t index) override { + return std::string(""); } void diff --git a/lldb/source/Commands/CommandObjectMemory.cpp b/lldb/source/Commands/CommandObjectMemory.cpp --- a/lldb/source/Commands/CommandObjectMemory.cpp +++ b/lldb/source/Commands/CommandObjectMemory.cpp @@ -345,9 +345,9 @@ Options *GetOptions() override { return &m_option_group; } - const char *GetRepeatCommand(Args ¤t_command_args, - uint32_t index) override { - return m_cmd_name.c_str(); + llvm::Optional GetRepeatCommand(Args ¤t_command_args, + uint32_t index) override { + return m_cmd_name; } protected: @@ -1586,9 +1586,9 @@ ~CommandObjectMemoryHistory() override = default; - const char *GetRepeatCommand(Args ¤t_command_args, - uint32_t index) override { - return m_cmd_name.c_str(); + llvm::Optional GetRepeatCommand(Args ¤t_command_args, + uint32_t index) override { + return m_cmd_name; } protected: @@ -1740,11 +1740,11 @@ return false; } - const char *GetRepeatCommand(Args ¤t_command_args, - uint32_t index) override { + llvm::Optional GetRepeatCommand(Args ¤t_command_args, + uint32_t index) override { // If we repeat this command, repeat it without any arguments so we can // show the next memory range - return m_cmd_name.c_str(); + return m_cmd_name; } lldb::addr_t m_prev_end_addr; diff --git a/lldb/source/Commands/CommandObjectMultiword.cpp b/lldb/source/Commands/CommandObjectMultiword.cpp --- a/lldb/source/Commands/CommandObjectMultiword.cpp +++ b/lldb/source/Commands/CommandObjectMultiword.cpp @@ -290,15 +290,16 @@ sub_command_object->HandleCompletion(request); } -const char *CommandObjectMultiword::GetRepeatCommand(Args ¤t_command_args, - uint32_t index) { +llvm::Optional +CommandObjectMultiword::GetRepeatCommand(Args ¤t_command_args, + uint32_t index) { index++; if (current_command_args.GetArgumentCount() <= index) - return nullptr; + return llvm::None; CommandObject *sub_command_object = GetSubcommandObject(current_command_args[index].ref()); if (sub_command_object == nullptr) - return nullptr; + return llvm::None; return sub_command_object->GetRepeatCommand(current_command_args, index); } @@ -419,12 +420,13 @@ proxy_command->HandleArgumentCompletion(request, opt_element_vector); } -const char *CommandObjectProxy::GetRepeatCommand(Args ¤t_command_args, - uint32_t index) { +llvm::Optional +CommandObjectProxy::GetRepeatCommand(Args ¤t_command_args, + uint32_t index) { CommandObject *proxy_command = GetProxyCommandObject(); if (proxy_command) return proxy_command->GetRepeatCommand(current_command_args, index); - return nullptr; + return llvm::None; } llvm::StringRef CommandObjectProxy::GetUnsupportedError() { diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp --- a/lldb/source/Commands/CommandObjectProcess.cpp +++ b/lldb/source/Commands/CommandObjectProcess.cpp @@ -145,10 +145,10 @@ Options *GetOptions() override { return &m_all_options; } - const char *GetRepeatCommand(Args ¤t_command_args, - uint32_t index) override { + llvm::Optional GetRepeatCommand(Args ¤t_command_args, + uint32_t index) override { // No repeat for "process launch"... - return ""; + return std::string(""); } protected: diff --git a/lldb/source/Commands/CommandObjectSource.cpp b/lldb/source/Commands/CommandObjectSource.cpp --- a/lldb/source/Commands/CommandObjectSource.cpp +++ b/lldb/source/Commands/CommandObjectSource.cpp @@ -728,8 +728,8 @@ Options *GetOptions() override { return &m_options; } - const char *GetRepeatCommand(Args ¤t_command_args, - uint32_t index) override { + llvm::Optional GetRepeatCommand(Args ¤t_command_args, + uint32_t index) override { // This is kind of gross, but the command hasn't been parsed yet so we // can't look at the option values for this invocation... I have to scan // the arguments directly. @@ -738,13 +738,13 @@ return e.ref() == "-r" || e.ref() == "--reverse"; }); if (iter == current_command_args.end()) - return m_cmd_name.c_str(); + return m_cmd_name; if (m_reverse_name.empty()) { m_reverse_name = m_cmd_name; m_reverse_name.append(" -r"); } - return m_reverse_name.c_str(); + return m_reverse_name; } protected: diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp --- a/lldb/source/Commands/CommandObjectThread.cpp +++ b/lldb/source/Commands/CommandObjectThread.cpp @@ -125,6 +125,58 @@ Options *GetOptions() override { return &m_options; } + llvm::Optional GetRepeatCommand(Args ¤t_args, + uint32_t idx) override { + llvm::StringRef count_opt("--count"); + llvm::StringRef start_opt("--start"); + + // If no "count" was provided, we are dumping the entire backtrace, so + // there isn't a repeat command. So we search for the count option in + // the args, and if we find it, we make a copy and insert or modify the + // start option's value to start count indices greater. + + Args copy_args(current_args); + size_t num_entries = copy_args.GetArgumentCount(); + // These two point at the index of the option value if found. + size_t count_idx = 0; + size_t start_idx = 0; + size_t count_val = 0; + size_t start_val = 0; + + for (size_t idx = 0; idx < num_entries; idx++) { + llvm::StringRef arg_string = copy_args[idx].ref(); + if (arg_string.equals("-c") || count_opt.startswith(arg_string)) { + idx++; + if (idx == num_entries) + return llvm::None; + count_idx = idx; + if (copy_args[idx].ref().getAsInteger(0, count_val)) + return llvm::None; + } else if (arg_string.equals("-s") || start_opt.startswith(arg_string)) { + idx++; + if (idx == num_entries) + return llvm::None; + start_idx = idx; + if (copy_args[idx].ref().getAsInteger(0, start_val)) + return llvm::None; + } + } + if (count_idx == 0) + return llvm::None; + + std::string new_start_val = llvm::formatv("{0}", start_val + count_val); + if (start_idx == 0) { + copy_args.AppendArgument(start_opt); + copy_args.AppendArgument(new_start_val); + } else { + copy_args.ReplaceArgumentAtIndex(start_idx, new_start_val); + } + std::string repeat_command; + if (!copy_args.GetQuotedCommandString(repeat_command)) + return llvm::None; + return repeat_command; + } + protected: void DoExtendedBacktrace(Thread *thread, CommandReturnObject &result) { SystemRuntime *runtime = thread->GetProcess()->GetSystemRuntime(); @@ -960,7 +1012,7 @@ uint32_t index_ptr = 0, end_ptr; std::vector address_list; - // Find the beginning & end index of the + // Find the beginning & end index of the AddressRange fun_addr_range = sc.function->GetAddressRange(); Address fun_start_addr = fun_addr_range.GetBaseAddress(); line_table->FindLineEntryByAddress(fun_start_addr, function_start, @@ -2132,11 +2184,11 @@ Options *GetOptions() override { return &m_options; } - const char *GetRepeatCommand(Args ¤t_command_args, - uint32_t index) override { + llvm::Optional GetRepeatCommand(Args ¤t_command_args, + uint32_t index) override { current_command_args.GetCommandString(m_repeat_command); m_create_repeat_command_just_invoked = true; - return m_repeat_command.c_str(); + return m_repeat_command; } protected: diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp --- a/lldb/source/Interpreter/CommandInterpreter.cpp +++ b/lldb/source/Interpreter/CommandInterpreter.cpp @@ -1944,16 +1944,21 @@ // arguments. if (cmd_obj != nullptr) { - if (add_to_history) { + // If we got here when empty_command was true, then this command is a + // stored "repeat command" which we should give a chance to produce it's + // repeat command, even though we don't add repeat commands to the history. + if (add_to_history || empty_command) { Args command_args(command_string); - const char *repeat_command = cmd_obj->GetRepeatCommand(command_args, 0); - if (repeat_command != nullptr) - m_repeat_command.assign(repeat_command); + llvm::Optional repeat_command = + cmd_obj->GetRepeatCommand(command_args, 0); + if (repeat_command) + m_repeat_command.assign(*repeat_command); else m_repeat_command.assign(original_command_string); + } + if (add_to_history) m_command_history.AppendString(original_command_string); - } std::string remainder; const std::size_t actual_cmd_name_len = cmd_obj->GetCommandName().size(); diff --git a/lldb/test/API/commands/thread/backtrace/Makefile b/lldb/test/API/commands/thread/backtrace/Makefile new file mode 100644 --- /dev/null +++ b/lldb/test/API/commands/thread/backtrace/Makefile @@ -0,0 +1,4 @@ +CXX_SOURCES := main.cpp +ENABLE_THREADS := YES + +include Makefile.rules diff --git a/lldb/test/API/commands/thread/backtrace/TestThreadBacktraceRepeat.py b/lldb/test/API/commands/thread/backtrace/TestThreadBacktraceRepeat.py new file mode 100644 --- /dev/null +++ b/lldb/test/API/commands/thread/backtrace/TestThreadBacktraceRepeat.py @@ -0,0 +1,167 @@ +""" +Test that we page getting a long backtrace on more than one thread +""" + + + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestThreadBacktracePage(TestBase): + + mydir = TestBase.compute_mydir(__file__) + NO_DEBUG_INFO_TESTCASE = True + + def test_thread_backtrace_one_thread(self): + """Run a simplified version of the test that just hits one breakpoint and + doesn't care about synchronizing the two threads - hopefully this will + run on more systems.""" + + def test_thread_backtrace_one_thread(self): + self.build() + (self.inferior_target, self.process, thread, bkpt) = lldbutil.run_to_source_breakpoint( + self, self.bkpt_string, lldb.SBFileSpec('main.cpp'), only_one_thread = False) + + # We hit the breakpoint on at least one thread. If we hit it on both threads + # simultaneously, we are ready to run our tests. Otherwise, suspend the thread + # that hit the breakpoint, and continue till the second thread hits + # the breakpoint: + + (breakpoint_threads, other_threads) = ([], []) + lldbutil.sort_stopped_threads(self.process, + breakpoint_threads=breakpoint_threads, + other_threads=other_threads) + self.assertGreater(len(breakpoint_threads), 0, "We hit at least one breakpoint thread") + self.assertGreater(len(breakpoint_threads[0].frames), 2, "I can go up") + thread_id = breakpoint_threads[0].idx + name = breakpoint_threads[0].frame[1].name.split("(")[0] + self.check_one_thread(thread_id, name) + + def setUp(self): + # Call super's setUp(). + TestBase.setUp(self) + # Find the line number for our breakpoint. + self.bkpt_string = '// Set breakpoint here' + + def check_one_thread(self, thread_id, func_name): + # Now issue some thread backtrace commands and make sure they + # get the right answer back. + interp = self.dbg.GetCommandInterpreter() + result = lldb.SBCommandReturnObject() + + # Run the real backtrace, remember to pass True for add_to_history since + # we don't generate repeat commands for commands that aren't going into the history. + interp.HandleCommand("thread backtrace --count 10 {0}".format(thread_id), result, True) + self.assertTrue(result.Succeeded(), "bt with count succeeded") + # There should be 11 lines: + lines = result.GetOutput().splitlines() + self.assertEqual(len(lines), 11, "Got the right number of lines") + # First frame is stop_here: + self.assertNotEqual(lines[1].find("stop_here"), -1, "Found Stop Here") + for line in lines[2:10]: + self.assertNotEqual(line.find(func_name), -1, "Name {0} not found in line: {1}".format(func_name, line)) + # The last entry should be 43: + self.assertNotEqual(lines[10].find("count=43"), -1, "First show ends at 43") + + # Now try a repeat, and make sure we get 10 more on this thread: + #import pdb; pdb.set_trace() + interp.HandleCommand("", result, True) + self.assertTrue(result.Succeeded(), "repeat command failed: {0}".format(result.GetError())) + lines = result.GetOutput().splitlines() + self.assertEqual(len(lines), 11, "Repeat got 11 lines") + # Every line should now be the recurse function: + for line in lines[1:10]: + self.assertNotEqual(line.find(func_name), -1, "Name in every line") + self.assertNotEqual(lines[10].find("count=33"), -1, "Last one is now 33") + + def check_two_threads(self, result_str, thread_id_1, name_1, thread_id_2, name_2, start_idx, end_idx): + # We should have 2 occurrences ot the thread header: + self.assertEqual(result_str.count("thread #{0}".format(thread_id_1)), 1, "One for thread 1") + self.assertEqual(result_str.count("thread #{0}".format(thread_id_2)), 1, "One for thread 2") + # We should have 10 occurrences of each name: + self.assertEqual(result_str.count(name_1), 10, "Found 10 of {0}".format(name_1)) + self.assertEqual(result_str.count(name_2), 10, "Found 10 of {0}".format(name_1)) + # There should be two instances of count= and none of count=: + self.assertEqual(result_str.count("count={0}".format(start_idx)), 2, "Two instances of start_idx") + self.assertEqual(result_str.count("count={0}".format(start_idx-1)), 0, "No instances of start_idx - 1") + # There should be two instances of count= and none of count=: + self.assertEqual(result_str.count("count={0}".format(end_idx)), 2, "Two instances of end_idx") + self.assertEqual(result_str.count("count={0}".format(end_idx+1)), 0, "No instances after end idx") + + # The setup of this test was copied from the step-out test, and I can't tell from + # the comments whether it was getting two threads to the same breakpoint that was + # problematic, or the step-out part. This test stops at the rendevous point so I'm + # removing the skipIfLinux to see if we see any flakiness in just this part of the test. + @expectedFailureAll( + oslist=["linux"], + bugnumber="llvm.org/pr23477 Test occasionally times out on the Linux build bot") + @expectedFailureAll( + oslist=["freebsd"], + bugnumber="llvm.org/pr18066 inferior does not exit") + @skipIfWindows # This test will hang on windows llvm.org/pr21753 + @expectedFailureAll(oslist=["windows"]) + @expectedFailureNetBSD + def test_thread_backtrace_two_threads(self): + """Test that repeat works even when backtracing on more than one thread.""" + self.build() + (self.inferior_target, self.process, thread, bkpt) = lldbutil.run_to_source_breakpoint( + self, self.bkpt_string, lldb.SBFileSpec('main.cpp'), only_one_thread = False) + + # We hit the breakpoint on at least one thread. If we hit it on both threads + # simultaneously, we are ready to run our tests. Otherwise, suspend the thread + # that hit the breakpoint, and continue till the second thread hits + # the breakpoint: + + (breakpoint_threads, other_threads) = ([], []) + lldbutil.sort_stopped_threads(self.process, + breakpoint_threads=breakpoint_threads, + other_threads=other_threads) + if len(breakpoint_threads) == 1: + success = thread.Suspend() + self.assertTrue(success, "Couldn't suspend a thread") + bkpt_threads = lldbutil.continue_to_breakpoint(self.process, + bkpt) + self.assertEqual(len(bkpt_threads), 1, "Second thread stopped") + breakpoint_threads.append(bkpt_threads[0]) + + # Figure out which thread is which: + thread_id_1 = breakpoint_threads[0].idx + self.assertGreater(len(breakpoint_threads[0].frames), 2, "I can go up") + name_1 = breakpoint_threads[0].frame[1].name.split("(")[0] + + thread_id_2 = breakpoint_threads[1].idx + self.assertGreater(len(breakpoint_threads[1].frames), 2, "I can go up") + name_2 = breakpoint_threads[1].frame[1].name.split("(")[0] + + # Check that backtrace and repeat works on one thread, then works on the second + # when we switch to it: + self.check_one_thread(thread_id_1, name_1) + self.check_one_thread(thread_id_2, name_2) + + # The output is looking right at this point, let's just do a couple more quick checks + # to see we handle two threads and a start count: + interp = self.dbg.GetCommandInterpreter() + result = lldb.SBCommandReturnObject() + + interp.HandleCommand("thread backtrace --count 10 --start 10 {0} {1}".format(thread_id_1, thread_id_2), result, True) + self.assertTrue(result.Succeeded(), "command succeeded for two threads") + + result.Clear() + interp.HandleCommand("", result, True) + self.assertTrue(result.Succeeded(), "repeat command succeeded for two threads") + result_str = result.GetOutput() + self.check_two_threads(result_str, thread_id_1, name_1, thread_id_2, name_2, 23, 32) + + # Finally make sure the repeat repeats: + result.Clear() + interp.HandleCommand("", result, True) + self.assertTrue(result.Succeeded(), "repeat command succeeded for two threads") + result_str = result.GetOutput() + self.check_two_threads(result_str, thread_id_1, name_1, thread_id_2, name_2, 13, 22) + + + + diff --git a/lldb/test/API/commands/thread/backtrace/main.cpp b/lldb/test/API/commands/thread/backtrace/main.cpp new file mode 100644 --- /dev/null +++ b/lldb/test/API/commands/thread/backtrace/main.cpp @@ -0,0 +1,64 @@ +// This test is intended to create a situation in which two threads are stopped +// at a breakpoint and the debugger issues a step-out command. + +#include "pseudo_barrier.h" +#include + +pseudo_barrier_t g_barrier; + +volatile int g_test = 0; + +void stop_here() { + g_test += 5; // Set breakpoint here +} + +void recurse_a_bit_1(int count) { + if (count == 50) + stop_here(); + else + recurse_a_bit_1(++count); +} + +void recurse_a_bit_2(int count) { + if (count == 50) + stop_here(); + else + recurse_a_bit_2(++count); +} + +void *thread_func_1() { + // Wait until both threads are running + pseudo_barrier_wait(g_barrier); + + // Start the recursion: + recurse_a_bit_1(0); + + // Return + return NULL; +} + +void *thread_func_2() { + // Wait until both threads are running + pseudo_barrier_wait(g_barrier); + + // Start the recursion: + recurse_a_bit_2(0); + + // Return + return NULL; +} + +int main() { + // Don't let either thread do anything until they're both ready. + pseudo_barrier_init(g_barrier, 2); + + // Create two threads + std::thread thread_1(thread_func_1); + std::thread thread_2(thread_func_2); + + // Wait for the threads to finish + thread_1.join(); + thread_2.join(); + + return 0; +}