[Target] Do not skip a stop on a breakpoint if a plan was completed

[aleksandr.urakov]

Summary

This patch fixes the next situation. On Windows clang-cl makes no stub before the main function, so the main function is located exactly on module entry point. May be it is the same on other platforms. So consider the following sequence:

• set a breakpoint on main and stop there;
• try to evaluate expression, which requires a code execution on the debuggee side. Such an execution always returns to the module entry, and the plan waits for it there;
• the plan understands that it is complete now and removes its breakpoint. But the breakpoint site is still there, because we also have a breakpoint on entry;
• StopInfo analyzes a situation. It sees that we have stopped on the breakpoint site, and it sees that the breakpoint site has owners, and no one logical breakpoint is internal (because the plan is already completed and it have removed its breakpoint);
• StopInfo thinks that it's a user breakpoint and skips it to avoid recursive computations;
• the program continues.

So in this situation the program continues without a stop right after the expression evaluation. To avoid this I've added an additional check that the plan was completed.

The test is just the case I've caught, but it's Windows-only. It seems that the problem should exist on other platforms too, but I don't know how to test it in a cross-platform way (any tips are appreciated). The test depends on D53759.

[jingham]

This seems safe, but you certainly want to add a comment explaining why you are doing this.

We find the expression breakpoint by calling ObjectFile::GetEntryPointAddress on the main executable. So you should be able to test this everywhere by making that function result available somehow and then setting a user breakpoint there.

If you were making a dotest test I'd suggest adding SBTarget::GetEntryPointAddress and then use that.

[aleksandr.urakov]

Thanks! I've updated the diff according to comments.

[jingham]

LGTM

[aleksandr.urakov]

Thank you!

include/lldb/API/SBModule.h

public:
   /// in folder like '/usr/lib/liba.dylib.dSYM/'.
   ///
   /// @return
   ///     A const reference to the file specification object.
   //------------------------------------------------------------------
   lldb::SBFileSpec GetSymbolFileSpec() const;
 
   lldb::SBAddress GetObjectFileHeaderAddress() const;
+  lldb::SBAddress GetObjectFileEntryPointAddress() const;
 
 private:
   friend class SBAddress;
   friend class SBFrame;
   friend class SBSection;
   friend class SBSymbolContext;
   friend class SBTarget;
 

packages/Python/lldbsuite/test/functionalities/expr-entry-bp/Makefile

+LEVEL = ../../make
+
+C_SOURCES := main.c
+
+include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/functionalities/expr-entry-bp/TestExprEntryBP.py

+"""
+Tests expressions evaluation when the breakpoint on module's entry is set.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+class ExprEntryBPTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def test_expr_entry_bp(self):
+        """Tests expressions evaluation when the breakpoint on module's entry is set."""
+        self.build()
+        self.main_source_file = lldb.SBFileSpec("main.c")
+
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self, "Set a breakpoint here", self.main_source_file)
+
+        self.assertEqual(1, bkpt.GetNumLocations())
+        entry = bkpt.GetLocationAtIndex(0).GetAddress().GetModule().GetObjectFileEntryPointAddress()
+        self.assertTrue(entry.IsValid(), "Can't get a module entry point")
+
+        entry_bp = target.BreakpointCreateBySBAddress(entry)
+        self.assertTrue(entry_bp.IsValid(), "Can't set a breakpoint on the module entry point")
+
+        result = target.EvaluateExpression("sum(7, 1)")
+        self.assertTrue(result.IsValid(), "Can't evaluate expression")
+        self.assertEqual(8, result.GetValueAsSigned())
+
+    def setUp(self):
+        TestBase.setUp(self)

packages/Python/lldbsuite/test/functionalities/expr-entry-bp/main.c

+#include <stdio.h>
+
+int sum(int x, int y) {
+  return x + y;
+}
+
+int main() {
+  printf("Set a breakpoint here.\n");
+  return sum(-1, 1);
+}

scripts/interface/SBModule.i

GetVersion (uint32_t *versions,
                 uint32_t num_versions);
 
     lldb::SBFileSpec
     GetSymbolFileSpec() const;
 
     lldb::SBAddress
     GetObjectFileHeaderAddress() const;
 
+    lldb::SBAddress
+    GetObjectFileEntryPointAddress() const;
+
     bool
     operator == (const lldb::SBModule &rhs) const;
              
     bool
     operator != (const lldb::SBModule &rhs) const;
              
     %pythoncode %{
         class symbols_access(object):

source/API/SBModule.cpp

lldb::SBAddress SBModule::GetObjectFileHeaderAddress() const {
   ModuleSP module_sp(GetSP());
   if (module_sp) {
     ObjectFile *objfile_ptr = module_sp->GetObjectFile();
     if (objfile_ptr)
       sb_addr.ref() = objfile_ptr->GetHeaderAddress();
   }
   return sb_addr;
 }
+
+lldb::SBAddress SBModule::GetObjectFileEntryPointAddress() const {
+  lldb::SBAddress sb_addr;
+  ModuleSP module_sp(GetSP());
+  if (module_sp) {
+    ObjectFile *objfile_ptr = module_sp->GetObjectFile();
+    if (objfile_ptr)
+      sb_addr.ref() = objfile_ptr->GetEntryPointAddress();
+  }
+  return sb_addr;
+}

source/Target/StopInfo.cpp

if (thread_sp) {
             // the function with this breakpoint.
             // TODO: We can keep a list of the breakpoints we've seen while
             // running expressions in the nested
             // PerformAction calls that can arise when the action runs a
             // function that hits another breakpoint, and only stop running
             // commands when we see the same breakpoint hit a second time.
 
             m_should_stop_is_valid = true;
+
+            // It is possible that the user has a breakpoint at the same site
+            // as the completed plan had (e.g. user has a breakpoint
+            // on a module entry point, and `ThreadPlanCallFunction` ends
+            // also there). We can't find an internal breakpoint in the loop
+            // later because it was already removed on the plan completion.
+            // So check if the plan was completed, and stop if so.
+            if (thread_sp->CompletedPlanOverridesBreakpoint()) {
+              m_should_stop = true;
+              thread_sp->ResetStopInfo();
+              return;
+            }
+
             if (log)
               log->Printf("StopInfoBreakpoint::PerformAction - Hit a "
                           "breakpoint while running an expression,"
                           " not running commands to avoid recursion.");
             bool ignoring_breakpoints =
                 process->GetIgnoreBreakpointsInExpressions();
             if (ignoring_breakpoints) {
               m_should_stop = false;