mib updated this revision to Diff 500349.
mib retitled this revision from "[lldb] Fix {break,watch}point command function
stopping behavior" to "[lldb] Fix {break,watch}point command function stopping
behaviour".
mib edited the summary of this revision.
mib added a comment.
The third implementation is the charm 😝:
- If the user input is a oneliner, we wrap it into a nested function.
- Then we call the function (either the nested function or the user provided
python function) and capture the return value in a variable.
- Finally, after resetting the global dictionary to its previous state, we
return the variable that contains the user's return value.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144688/new/
https://reviews.llvm.org/D144688
Files:
lldb/include/lldb/Interpreter/ScriptInterpreter.h
lldb/source/Commands/CommandObjectWatchpointCommand.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
lldb/test/API/commands/watchpoints/watchpoint_commands/command/TestWatchpointCommandPython.py
lldb/test/API/commands/watchpoints/watchpoint_commands/command/main.cpp
lldb/test/API/commands/watchpoints/watchpoint_commands/command/watchpoint_command.py
Index: lldb/test/API/commands/watchpoints/watchpoint_commands/command/watchpoint_command.py
===================================================================
--- lldb/test/API/commands/watchpoints/watchpoint_commands/command/watchpoint_command.py
+++ 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.
Index: lldb/test/API/commands/watchpoints/watchpoint_commands/command/main.cpp
===================================================================
--- lldb/test/API/commands/watchpoints/watchpoint_commands/command/main.cpp
+++ 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.
}
Index: lldb/test/API/commands/watchpoints/watchpoint_commands/command/TestWatchpointCommandPython.py
===================================================================
--- lldb/test/API/commands/watchpoints/watchpoint_commands/command/TestWatchpointCommandPython.py
+++ 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",
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ 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_oneliner) 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_oneliner) override;
bool GenerateWatchpointCommandCallbackData(StringList &input,
- std::string &output) override;
+ std::string &output,
+ const bool is_oneliner) override;
bool GetScriptedSummary(const char *function_name, lldb::ValueObjectSP valobj,
StructuredData::ObjectSP &callee_wrapper_sp,
@@ -274,11 +275,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_oneliner = true);
/// 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_oneliner) override;
const char *GetDictionaryName() { return m_dictionary_name.c_str(); }
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ 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_oneliner=*/true)
.Success()) {
auto baton_sp = std::make_shared<BreakpointOptions::CommandBaton>(
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_oneliner=*/true)) {
auto baton_sp =
std::make_shared<WatchpointOptions::CommandBaton>(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<unsigned> 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_oneliner=*/false);
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_oneliner=*/true);
if (error.Fail()) {
return error;
}
@@ -1214,13 +1217,15 @@
Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallback(
BreakpointOptions &bp_options, const char *command_body_text) {
- return SetBreakpointCommandCallback(bp_options, command_body_text, {},false);
+ return SetBreakpointCommandCallback(bp_options, command_body_text, {}, false,
+ true);
}
// 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_oneliner) {
auto data_up = std::make_unique<CommandDataPython>(extra_args_sp);
// Split the command_body_text into lines, and pass that to
// GenerateBreakpointCommandCallbackData. That will wrap the body in an
@@ -1228,9 +1233,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_oneliner);
if (error.Success()) {
auto baton_sp =
std::make_shared<BreakpointOptions::CommandBaton>(std::move(data_up));
@@ -1243,7 +1248,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_oneliner) {
auto data_up = std::make_unique<WatchpointOptions::CommandData>();
// It's necessary to set both user_source and script_source to the oneliner.
@@ -1251,11 +1257,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_oneliner)) {
auto baton_sp =
std::make_shared<WatchpointOptions::CommandBaton>(std::move(data_up));
wp_options->SetCallback(
@@ -1275,7 +1281,8 @@
}
Status ScriptInterpreterPythonImpl::GenerateFunction(const char *signature,
- const StringList &input) {
+ const StringList &input,
+ const bool is_oneliner) {
Status error;
int num_lines = input.GetSize();
if (num_lines == 0) {
@@ -1292,40 +1299,54 @@
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_oneliner) {
+ auto_generated_function.AppendString(
+ " __return_val = None"); // Initialize user callback return value.
+ auto_generated_function.AppendString(" def __user_code():");
+ 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
+ } else {
+ 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_oneliner="
+ "false) = ERROR: python function is multiline.");
+ }
}
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
auto_generated_function.AppendString(
- " internal_dict[key] = global_dict[key]"); // Update session dict
- // values
+ " if key not in old_keys:"); // If key was not originally in
+ // global dict
auto_generated_function.AppendString(
- " if key not in old_keys:"); // If key was not originally in
- // global dict
+ " del global_dict[key]"); // ...then remove key/value from
+ // global dict
auto_generated_function.AppendString(
- " del global_dict[key]"); // ...then remove key/value from
- // global dict
+ " 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 +1371,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_oneliner=*/true)
+ .Success())
return false;
// Store the name of the auto-generated function to be called.
@@ -1374,7 +1396,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_oneliner=*/false)
+ .Success())
return false;
// Store the name of the auto-generated function to be called.
@@ -1971,8 +1994,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_oneliner) {
static uint32_t num_created_functions = 0;
user_input.RemoveBlankLines();
StreamString sstr;
@@ -1991,7 +2014,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_oneliner);
if (!error.Success())
return error;
@@ -2001,7 +2024,7 @@
}
bool ScriptInterpreterPythonImpl::GenerateWatchpointCommandCallbackData(
- StringList &user_input, std::string &output) {
+ StringList &user_input, std::string &output, const bool is_oneliner) {
static uint32_t num_created_functions = 0;
user_input.RemoveBlankLines();
StreamString sstr;
@@ -2014,7 +2037,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_oneliner).Success())
return false;
// Store the name of the auto-generated function to be called.
Index: lldb/source/Commands/CommandObjectWatchpointCommand.cpp
===================================================================
--- lldb/source/Commands/CommandObjectWatchpointCommand.cpp
+++ 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_oneliner=*/true);
}
// 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_oneliner=*/false);
} else {
script_interp->CollectDataForWatchpointCommandCallback(wp_options,
result);
Index: lldb/include/lldb/Interpreter/ScriptInterpreter.h
===================================================================
--- lldb/include/lldb/Interpreter/ScriptInterpreter.h
+++ 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_oneliner) {
Status error;
error.SetErrorString("not implemented");
return error;
}
virtual bool GenerateWatchpointCommandCallbackData(StringList &input,
- std::string &output) {
+ std::string &output,
+ const bool is_oneliner) {
return false;
}
@@ -359,7 +360,8 @@
}
virtual Status GenerateFunction(const char *signature,
- const StringList &input) {
+ const StringList &input,
+ const bool is_oneliner) {
Status error;
error.SetErrorString("unimplemented");
return error;
@@ -410,7 +412,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_oneliner) {}
virtual bool GetScriptedSummary(const char *function_name,
lldb::ValueObjectSP valobj,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
