diff --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h b/lldb/include/lldb/Interpreter/ScriptInterpreter.h --- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h +++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h @@ -183,17 +183,18 @@ return error; } - virtual Status GenerateBreakpointCommandCallbackData( - StringList &input, - std::string &output, - bool has_extra_args) { + virtual Status GenerateBreakpointCommandCallbackData(StringList &input, + std::string &output, + bool has_extra_args, + const bool is_callback) { Status error; error.SetErrorString("not implemented"); return error; } virtual bool GenerateWatchpointCommandCallbackData(StringList &input, - std::string &output) { + std::string &output, + const bool is_callback) { return false; } @@ -359,7 +360,8 @@ } virtual Status GenerateFunction(const char *signature, - const StringList &input) { + const StringList &input, + const bool is_callback) { Status error; error.SetErrorString("unimplemented"); return error; @@ -379,7 +381,8 @@ const char *callback_text); virtual Status SetBreakpointCommandCallback(BreakpointOptions &bp_options, - const char *callback_text) { + const char *callback_text, + const bool is_callback) { Status error; error.SetErrorString("unimplemented"); return error; @@ -410,7 +413,8 @@ /// Set a one-liner as the callback for the watchpoint. virtual void SetWatchpointCommandCallback(WatchpointOptions *wp_options, - const char *oneliner) {} + const char *user_input, + const bool is_callback) {} virtual bool GetScriptedSummary(const char *function_name, lldb::ValueObjectSP valobj, diff --git a/lldb/source/API/SBBreakpoint.cpp b/lldb/source/API/SBBreakpoint.cpp --- a/lldb/source/API/SBBreakpoint.cpp +++ b/lldb/source/API/SBBreakpoint.cpp @@ -645,7 +645,8 @@ bkpt_sp->GetTarget() .GetDebugger() .GetScriptInterpreter() - ->SetBreakpointCommandCallback(bp_options, callback_body_text); + ->SetBreakpointCommandCallback(bp_options, callback_body_text, + /*is_callback=*/false); sb_error.SetError(error); } else sb_error.SetErrorString("invalid breakpoint"); diff --git a/lldb/source/API/SBBreakpointLocation.cpp b/lldb/source/API/SBBreakpointLocation.cpp --- a/lldb/source/API/SBBreakpointLocation.cpp +++ b/lldb/source/API/SBBreakpointLocation.cpp @@ -263,7 +263,8 @@ .GetTarget() .GetDebugger() .GetScriptInterpreter() - ->SetBreakpointCommandCallback(bp_options, callback_body_text); + ->SetBreakpointCommandCallback(bp_options, callback_body_text, + /*is_callback=*/false); sb_error.SetError(error); } else sb_error.SetErrorString("invalid breakpoint"); diff --git a/lldb/source/API/SBBreakpointName.cpp b/lldb/source/API/SBBreakpointName.cpp --- a/lldb/source/API/SBBreakpointName.cpp +++ b/lldb/source/API/SBBreakpointName.cpp @@ -593,11 +593,11 @@ m_impl_up->GetTarget()->GetAPIMutex()); BreakpointOptions &bp_options = bp_name->GetOptions(); - Status error = - m_impl_up->GetTarget() - ->GetDebugger() - .GetScriptInterpreter() - ->SetBreakpointCommandCallback(bp_options, callback_body_text); + Status error = m_impl_up->GetTarget() + ->GetDebugger() + .GetScriptInterpreter() + ->SetBreakpointCommandCallback( + bp_options, callback_body_text, /*is_callback=*/false); sb_error.SetError(error); if (!sb_error.Fail()) UpdateName(*bp_name); diff --git a/lldb/source/Commands/CommandObjectWatchpointCommand.cpp b/lldb/source/Commands/CommandObjectWatchpointCommand.cpp --- a/lldb/source/Commands/CommandObjectWatchpointCommand.cpp +++ b/lldb/source/Commands/CommandObjectWatchpointCommand.cpp @@ -415,17 +415,18 @@ // Special handling for one-liner specified inline. if (m_options.m_use_one_liner) { script_interp->SetWatchpointCommandCallback( - wp_options, m_options.m_one_liner.c_str()); + wp_options, m_options.m_one_liner.c_str(), + /*is_callback=*/false); } // Special handling for using a Python function by name instead of // extending the watchpoint callback data structures, we just // automatize what the user would do manually: make their watchpoint // command be a function call else if (!m_options.m_function_name.empty()) { - std::string oneliner(m_options.m_function_name); - oneliner += "(frame, wp, internal_dict)"; + std::string function_signature = m_options.m_function_name; + function_signature += "(frame, wp, internal_dict)"; script_interp->SetWatchpointCommandCallback( - wp_options, oneliner.c_str()); + wp_options, function_signature.c_str(), /*is_callback=*/true); } else { script_interp->CollectDataForWatchpointCommandCallback(wp_options, result); diff --git a/lldb/source/Interpreter/ScriptInterpreter.cpp b/lldb/source/Interpreter/ScriptInterpreter.cpp --- a/lldb/source/Interpreter/ScriptInterpreter.cpp +++ b/lldb/source/Interpreter/ScriptInterpreter.cpp @@ -112,7 +112,8 @@ const char *callback_text) { Status error; for (BreakpointOptions &bp_options : bp_options_vec) { - error = SetBreakpointCommandCallback(bp_options, callback_text); + error = SetBreakpointCommandCallback(bp_options, callback_text, + /*is_callback=*/false); if (!error.Success()) break; } diff --git a/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h b/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h --- a/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h +++ b/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h @@ -88,7 +88,8 @@ CommandReturnObject &result) override; Status SetBreakpointCommandCallback(BreakpointOptions &bp_options, - const char *command_body_text) override; + const char *command_body_text, + const bool is_callback) override; void SetWatchpointCommandCallback(WatchpointOptions *wp_options, const char *command_body_text) override; diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp @@ -521,7 +521,8 @@ StructuredData::ObjectSP empty_args_sp; if (GenerateBreakpointCommandCallbackData(data_up->user_source, data_up->script_source, - false) + /*has_extra_args=*/false, + /*is_callback=*/false) .Success()) { auto baton_sp = std::make_shared( std::move(data_up)); @@ -544,7 +545,8 @@ data_up->user_source.SplitIntoLines(data); if (GenerateWatchpointCommandCallbackData(data_up->user_source, - data_up->script_source)) { + data_up->script_source, + /*is_callback=*/false)) { auto baton_sp = std::make_shared(std::move(data_up)); wp_options->SetCallback( @@ -1158,8 +1160,7 @@ StructuredData::ObjectSP extra_args_sp) { Status error; // For now just cons up a oneliner that calls the provided function. - std::string oneliner("return "); - oneliner += function_name; + std::string function_signature = function_name; llvm::Expected maybe_args = GetMaxPositionalArgumentsForCallable(function_name); @@ -1174,7 +1175,7 @@ bool uses_extra_args = false; if (max_args >= 4) { uses_extra_args = true; - oneliner += "(frame, bp_loc, extra_args, internal_dict)"; + function_signature += "(frame, bp_loc, extra_args, internal_dict)"; } else if (max_args >= 3) { if (extra_args_sp) { error.SetErrorString("cannot pass extra_args to a three argument callback" @@ -1182,7 +1183,7 @@ return error; } uses_extra_args = false; - oneliner += "(frame, bp_loc, internal_dict)"; + function_signature += "(frame, bp_loc, internal_dict)"; } else { error.SetErrorStringWithFormat("expected 3 or 4 argument " "function, %s can only take %zu", @@ -1190,8 +1191,9 @@ return error; } - SetBreakpointCommandCallback(bp_options, oneliner.c_str(), extra_args_sp, - uses_extra_args); + SetBreakpointCommandCallback(bp_options, function_signature.c_str(), + extra_args_sp, uses_extra_args, + /*is_callback=*/true); return error; } @@ -1201,7 +1203,8 @@ Status error; error = GenerateBreakpointCommandCallbackData(cmd_data_up->user_source, cmd_data_up->script_source, - false); + /*has_extra_args=*/false, + /*is_callback=*/false); if (error.Fail()) { return error; } @@ -1213,14 +1216,17 @@ } Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallback( - BreakpointOptions &bp_options, const char *command_body_text) { - return SetBreakpointCommandCallback(bp_options, command_body_text, {},false); + BreakpointOptions &bp_options, const char *command_body_text, + const bool is_callback) { + return SetBreakpointCommandCallback(bp_options, command_body_text, {}, + /*uses_extra_args=*/false, is_callback); } // Set a Python one-liner as the callback for the breakpoint. Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallback( BreakpointOptions &bp_options, const char *command_body_text, - StructuredData::ObjectSP extra_args_sp, bool uses_extra_args) { + StructuredData::ObjectSP extra_args_sp, bool uses_extra_args, + const bool is_callback) { auto data_up = std::make_unique(extra_args_sp); // Split the command_body_text into lines, and pass that to // GenerateBreakpointCommandCallbackData. That will wrap the body in an @@ -1228,9 +1234,9 @@ // That is what the callback will actually invoke. data_up->user_source.SplitIntoLines(command_body_text); - Status error = GenerateBreakpointCommandCallbackData(data_up->user_source, - data_up->script_source, - uses_extra_args); + Status error = GenerateBreakpointCommandCallbackData( + data_up->user_source, data_up->script_source, uses_extra_args, + is_callback); if (error.Success()) { auto baton_sp = std::make_shared(std::move(data_up)); @@ -1243,7 +1249,8 @@ // Set a Python one-liner as the callback for the watchpoint. void ScriptInterpreterPythonImpl::SetWatchpointCommandCallback( - WatchpointOptions *wp_options, const char *oneliner) { + WatchpointOptions *wp_options, const char *user_input, + const bool is_callback) { auto data_up = std::make_unique(); // It's necessary to set both user_source and script_source to the oneliner. @@ -1251,11 +1258,11 @@ // command list) while the latter is used for Python to interpret during the // actual callback. - data_up->user_source.AppendString(oneliner); - data_up->script_source.assign(oneliner); + data_up->user_source.AppendString(user_input); + data_up->script_source.assign(user_input); - if (GenerateWatchpointCommandCallbackData(data_up->user_source, - data_up->script_source)) { + if (GenerateWatchpointCommandCallbackData( + data_up->user_source, data_up->script_source, is_callback)) { auto baton_sp = std::make_shared(std::move(data_up)); wp_options->SetCallback( @@ -1275,7 +1282,8 @@ } Status ScriptInterpreterPythonImpl::GenerateFunction(const char *signature, - const StringList &input) { + const StringList &input, + const bool is_callback) { Status error; int num_lines = input.GetSize(); if (num_lines == 0) { @@ -1292,40 +1300,61 @@ StringList auto_generated_function; auto_generated_function.AppendString(signature); auto_generated_function.AppendString( - " global_dict = globals()"); // Grab the global dictionary + " global_dict = globals()"); // Grab the global dictionary auto_generated_function.AppendString( - " new_keys = internal_dict.keys()"); // Make a list of keys in the - // session dict + " new_keys = internal_dict.keys()"); // Make a list of keys in the + // session dict auto_generated_function.AppendString( - " old_keys = global_dict.keys()"); // Save list of keys in global dict + " old_keys = global_dict.keys()"); // Save list of keys in global dict auto_generated_function.AppendString( - " global_dict.update (internal_dict)"); // Add the session dictionary - // to the - // global dictionary. - - // Wrap everything up inside the function, increasing the indentation. - - auto_generated_function.AppendString(" if True:"); - for (int i = 0; i < num_lines; ++i) { - sstr.Clear(); - sstr.Printf(" %s", input.GetStringAtIndex(i)); - auto_generated_function.AppendString(sstr.GetData()); + " global_dict.update(internal_dict)"); // Add the session dictionary + // to the global dictionary. + + if (is_callback) { + // If the user input is a callback to a python function, make sure the input + // is only 1 line, otherwise appending the user input would break the + // generated wrapped function + if (num_lines == 1) { + sstr.Clear(); + sstr.Printf(" __return_val = %s", input.GetStringAtIndex(0)); + auto_generated_function.AppendString(sstr.GetData()); + } else { + return Status("ScriptInterpreterPythonImpl::GenerateFunction(is_callback=" + "true) = ERROR: python function is multiline."); + } + } else { + auto_generated_function.AppendString( + " __return_val = None"); // Initialize user callback return value. + auto_generated_function.AppendString( + " def __user_code():"); // Create a nested function that will wrap + // the user input. This is necessary to + // capture the return value of the user input + // and prevent early returns. + for (int i = 0; i < num_lines; ++i) { + sstr.Clear(); + sstr.Printf(" %s", input.GetStringAtIndex(i)); + auto_generated_function.AppendString(sstr.GetData()); + } + auto_generated_function.AppendString( + " __return_val = __user_code()"); // Call user code and capture + // return value } auto_generated_function.AppendString( - " for key in new_keys:"); // Iterate over all the keys from session - // dict + " for key in new_keys:"); // Iterate over all the keys from session + // dict auto_generated_function.AppendString( - " internal_dict[key] = global_dict[key]"); // Update session dict - // values + " internal_dict[key] = global_dict[key]"); // Update session dict + // values auto_generated_function.AppendString( - " if key not in old_keys:"); // If key was not originally in - // global dict + " if key not in old_keys:"); // If key was not originally in + // global dict auto_generated_function.AppendString( - " del global_dict[key]"); // ...then remove key/value from - // global dict + " del global_dict[key]"); // ...then remove key/value from + // global dict + auto_generated_function.AppendString( + " return __return_val"); // Return the user callback return value. // Verify that the results are valid Python. - error = ExportFunctionDefinitionToInterpreter(auto_generated_function); return error; @@ -1350,7 +1379,8 @@ sstr.Printf("def %s (valobj, internal_dict):", auto_generated_function_name.c_str()); - if (!GenerateFunction(sstr.GetData(), user_input).Success()) + if (!GenerateFunction(sstr.GetData(), user_input, /*is_callback=*/false) + .Success()) return false; // Store the name of the auto-generated function to be called. @@ -1374,7 +1404,8 @@ sstr.Printf("def %s (debugger, args, exe_ctx, result, internal_dict):", auto_generated_function_name.c_str()); - if (!GenerateFunction(sstr.GetData(), user_input).Success()) + if (!GenerateFunction(sstr.GetData(), user_input, /*is_callback=*/true) + .Success()) return false; // Store the name of the auto-generated function to be called. @@ -1971,8 +2002,8 @@ } Status ScriptInterpreterPythonImpl::GenerateBreakpointCommandCallbackData( - StringList &user_input, std::string &output, - bool has_extra_args) { + StringList &user_input, std::string &output, bool has_extra_args, + const bool is_callback) { static uint32_t num_created_functions = 0; user_input.RemoveBlankLines(); StreamString sstr; @@ -1991,7 +2022,7 @@ sstr.Printf("def %s (frame, bp_loc, internal_dict):", auto_generated_function_name.c_str()); - error = GenerateFunction(sstr.GetData(), user_input); + error = GenerateFunction(sstr.GetData(), user_input, is_callback); if (!error.Success()) return error; @@ -2001,7 +2032,7 @@ } bool ScriptInterpreterPythonImpl::GenerateWatchpointCommandCallbackData( - StringList &user_input, std::string &output) { + StringList &user_input, std::string &output, const bool is_callback) { static uint32_t num_created_functions = 0; user_input.RemoveBlankLines(); StreamString sstr; @@ -2014,7 +2045,7 @@ sstr.Printf("def %s (frame, wp, internal_dict):", auto_generated_function_name.c_str()); - if (!GenerateFunction(sstr.GetData(), user_input).Success()) + if (!GenerateFunction(sstr.GetData(), user_input, is_callback).Success()) return false; // Store the name of the auto-generated function to be called. diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h @@ -188,16 +188,17 @@ lldb_private::CommandReturnObject &cmd_retobj, Status &error, const lldb_private::ExecutionContext &exe_ctx) override; - Status GenerateFunction(const char *signature, - const StringList &input) override; + Status GenerateFunction(const char *signature, const StringList &input, + const bool is_callback) override; - Status GenerateBreakpointCommandCallbackData( - StringList &input, - std::string &output, - bool has_extra_args) override; + Status GenerateBreakpointCommandCallbackData(StringList &input, + std::string &output, + bool has_extra_args, + const bool is_callback) override; bool GenerateWatchpointCommandCallbackData(StringList &input, - std::string &output) override; + std::string &output, + const bool is_callback) override; bool GetScriptedSummary(const char *function_name, lldb::ValueObjectSP valobj, StructuredData::ObjectSP &callee_wrapper_sp, @@ -260,7 +261,8 @@ /// Set the callback body text into the callback for the breakpoint. Status SetBreakpointCommandCallback(BreakpointOptions &bp_options, - const char *callback_body) override; + const char *callback_body, + const bool is_callback) override; Status SetBreakpointCommandCallbackFunction( BreakpointOptions &bp_options, const char *function_name, @@ -274,11 +276,13 @@ Status SetBreakpointCommandCallback(BreakpointOptions &bp_options, const char *command_body_text, StructuredData::ObjectSP extra_args_sp, - bool uses_extra_args); + bool uses_extra_args, + const bool is_callback); /// Set a one-liner as the callback for the watchpoint. void SetWatchpointCommandCallback(WatchpointOptions *wp_options, - const char *oneliner) override; + const char *user_input, + const bool is_callback) override; const char *GetDictionaryName() { return m_dictionary_name.c_str(); } diff --git a/lldb/test/API/commands/watchpoints/watchpoint_commands/command/TestWatchpointCommandPython.py b/lldb/test/API/commands/watchpoints/watchpoint_commands/command/TestWatchpointCommandPython.py --- a/lldb/test/API/commands/watchpoints/watchpoint_commands/command/TestWatchpointCommandPython.py +++ b/lldb/test/API/commands/watchpoints/watchpoint_commands/command/TestWatchpointCommandPython.py @@ -1,4 +1,4 @@ -""" +""" Test 'watchpoint command'. """ @@ -22,6 +22,8 @@ # Find the line number to break inside main(). self.line = line_number( self.source, '// Set break point at this line.') + self.second_line = line_number( + self.source, '// Set another breakpoint here.') # And the watchpoint variable declaration line number. self.decl = line_number(self.source, '// Watchpoint variable declaration.') @@ -143,6 +145,32 @@ self.expect("thread backtrace", STOPPED_DUE_TO_WATCHPOINT, substrs=['stop reason = watchpoint']) + # We should have hit the watchpoint once, set cookie to 888, since the + # user callback returned True. + self.expect("frame variable --show-globals cookie", + substrs=['(int32_t)', 'cookie = 888']) + + self.runCmd("process continue") + + # We should be stopped again due to the watchpoint (write type). + # The stop reason of the thread should be watchpoint. + self.expect("thread backtrace", STOPPED_DUE_TO_WATCHPOINT, + substrs=['stop reason = watchpoint']) + + # We should have hit the watchpoint a second time, set cookie to 666, + # even if the user callback didn't return anything and then continue. + self.expect("frame variable --show-globals cookie", + substrs=['(int32_t)', 'cookie = 666']) + + # Add a breakpoint to set a watchpoint when stopped on the breakpoint. + lldbutil.run_break_set_by_file_and_line( + self, None, self.second_line, num_expected_locations=1) + + self.runCmd("process continue") + + self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT, + substrs=['stop reason = breakpoint']) + # We should have hit the watchpoint once, set cookie to 888, then continued to the # second hit and set it to 999 self.expect("frame variable --show-globals cookie", diff --git a/lldb/test/API/commands/watchpoints/watchpoint_commands/command/main.cpp b/lldb/test/API/commands/watchpoints/watchpoint_commands/command/main.cpp --- a/lldb/test/API/commands/watchpoints/watchpoint_commands/command/main.cpp +++ b/lldb/test/API/commands/watchpoints/watchpoint_commands/command/main.cpp @@ -16,5 +16,5 @@ modify(global); printf("global=%d\n", global); - printf("cookie=%d\n", cookie); + printf("cookie=%d\n", cookie); // Set another breakpoint here. } diff --git a/lldb/test/API/commands/watchpoints/watchpoint_commands/command/watchpoint_command.py b/lldb/test/API/commands/watchpoints/watchpoint_commands/command/watchpoint_command.py --- a/lldb/test/API/commands/watchpoints/watchpoint_commands/command/watchpoint_command.py +++ b/lldb/test/API/commands/watchpoints/watchpoint_commands/command/watchpoint_command.py @@ -9,7 +9,12 @@ print ("I stopped the first time") frame.EvaluateExpression("cookie = 888") num_hits += 1 - frame.thread.process.Continue() + return True + if num_hits == 1: + print ("I stopped the second time, but with no return") + frame.EvaluateExpression("cookie = 666") + num_hits += 1 else: print ("I stopped the %d time" % (num_hits)) frame.EvaluateExpression("cookie = 999") + return False # This cause the process to continue.