Fix the managing of the session dictionary when you have nested wrappers (#132846)

Since the inner wrapper call might have removed one of the entries from
the global dict that the outer wrapper ALSO was going to delete, make
sure that we check that the key is still in the global dict before
trying to act on it.

Author: jimingham

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

@@ -1267,6 +1267,8 @@ Status ScriptInterpreterPythonImpl::ExportFunctionDefinitionToInterpreter(
     StringList &function_def) {
   // Convert StringList to one long, newline delimited, const char *.
   std::string function_def_string(function_def.CopyList());
+  LLDB_LOG(GetLog(LLDBLog::Script), "Added Function:\n%s\n",
+           function_def_string.c_str());
 
   Status error = ExecuteMultipleLines(
       function_def_string.c_str(), ExecuteScriptOptions().SetEnableIO(false));
@@ -1336,13 +1338,15 @@ Status ScriptInterpreterPythonImpl::GenerateFunction(const char *signature,
       "    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
+      "        if key in old_keys:"); // If key was originally in
+                                      // global dict
   auto_generated_function.AppendString(
-      "        if key not in old_keys:"); // If key was not originally in
-                                          // global dict
+      "            internal_dict[key] = global_dict[key]"); // Update it
   auto_generated_function.AppendString(
-      "            del global_dict[key]"); //  ...then remove key/value from
+      "        elif key in global_dict:"); // Then if it is still in the
+                                           // global dict
+  auto_generated_function.AppendString(
+      "            del global_dict[key]"); //  remove key/value from the
                                            //  global dict
   auto_generated_function.AppendString(
       "    return __return_val"); //  Return the user callback return value.

lldb/test/API/functionalities/breakpoint/nested_breakpoint_commands/Makefile

@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -std=c99
+
+include Makefile.rules

lldb/test/API/functionalities/breakpoint/nested_breakpoint_commands/TestNestedBreakpointCommands.py

@@ -0,0 +1,59 @@
+"""
+Test that a Python breakpoint callback defined in another Python
+breakpoint callback works properly. 
+"""
+
+
+import lldb
+import os
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class TestNestedBreakpointCommands(TestBase):
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def test_nested_commands(self):
+        self.build()
+        self.main_source_file = lldb.SBFileSpec("main.c")
+        self.callback_module = "make_bkpt_cmds"
+        self.do_test()
+
+    def do_test(self):
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+            self, "Set a breakpoint here", self.main_source_file
+        )
+
+        outer_bkpt = target.BreakpointCreateBySourceRegex(
+            "Set outer breakpoint here", self.main_source_file
+        )
+        cmd_file_path = os.path.join(self.getSourceDir(), f"{self.callback_module}.py")
+        self.runCmd(f"command script import {cmd_file_path}")
+        outer_bkpt.SetScriptCallbackFunction(f"{self.callback_module}.outer_callback")
+
+        process.Continue()
+
+        self.assertEqual(
+            thread.stop_reason, lldb.eStopReasonBreakpoint, "Right stop reason"
+        )
+
+        bkpt_no = thread.stop_reason_data[0]
+
+        # We made the callbacks record the new breakpoint ID and the number of
+        # times a callback ran in some globals in the target.  Find them now:
+        exec_module = target.FindModule(target.executable)
+        self.assertTrue(exec_module.IsValid(), "Found executable module")
+        var = exec_module.FindFirstGlobalVariable(target, "g_global")
+        self.assertSuccess(var.GetError(), "Found globals")
+        num_hits = var.GetChildAtIndex(1).GetValueAsUnsigned()
+        inner_id = var.GetChildAtIndex(2).GetValueAsUnsigned()
+
+        # Make sure they have the right values:
+        self.assertEqual(bkpt_no, inner_id, "Hit the right breakpoint")
+        self.assertEqual(num_hits, 2, "Right counter end value")
+        self.assertEqual(thread.frames[0].name, "main", "Got to main")
+
+        self.assertEqual(outer_bkpt.GetHitCount(), 1, "Hit outer breakpoint once")
+
+        inner_bkpt = target.FindBreakpointByID(inner_id)
+        self.assertEqual(inner_bkpt.GetHitCount(), 1, "Hit inner breakpoint once")

lldb/test/API/functionalities/breakpoint/nested_breakpoint_commands/main.c

@@ -0,0 +1,13 @@
+#include <stdio.h>
+
+int g_global[3] = {0, 100000, 100000};
+
+void doSomething() {
+  g_global[0] = 1; // Set outer breakpoint here
+}
+
+int main() {
+  doSomething(); // Set a breakpoint here
+
+  return g_global[0];
+}

lldb/test/API/functionalities/breakpoint/nested_breakpoint_commands/make_bkpt_cmds.py

@@ -0,0 +1,30 @@
+import lldb
+
+
+def set_globals(target, index, value):
+    exe_module = target.FindModule(target.executable)
+    var = exe_module.FindFirstGlobalVariable(target, "g_global")
+    child = var.GetChildAtIndex(index)
+    child.SetValueFromCString(str(value))
+
+
+def outer_callback(frame: lldb.SBFrame, bp_loc, internal_dict):
+    thread = frame.GetThread()
+
+    # address of the next frame
+    next_frame_pc = thread.get_thread_frames()[1].GetPC()
+
+    target = thread.process.target
+    bp = target.BreakpointCreateByAddress(next_frame_pc)
+    bp.SetScriptCallbackFunction(f"{__name__}.nested_bp_callback")
+    set_globals(target, 1, 1)
+    set_globals(target, 2, bp.GetID())
+
+    return False
+
+
+def nested_bp_callback(frame: lldb.SBFrame, bp_loc, extra_args, internal_dict):
+    target = frame.thread.process.target
+    set_globals(target, 1, 2)
+
+    return True