Debugger: Re-use base infrastructure for LLDB breakpoint handling

Change-Id: If32b1f421e45dc4ee446e193e03c959d7c700948
Reviewed-by: hjk <hjk@theqtcompany.com>

share/qtcreator/debugger/lldbbridge.py

@@ -643,7 +643,8 @@ class Dumper(DumperBase):
             self.target = self.debugger.CreateTarget(None, None, None, True, error)
 
         if self.target.IsValid():
-            self.handleBreakpoints(args)
+            for bp in args['bkpts']:
+                self.insertBreakpoint(bp)
 
         state = "inferiorsetupok" if self.target.IsValid() else "inferiorsetupfailed"
         self.report('state="%s",msg="%s",exe="%s"' % (state, error, self.executable_))
@@ -1309,7 +1310,9 @@ class Dumper(DumperBase):
         return self.target.BreakpointCreateByName(
             "main", self.target.GetExecutable().GetFilename())
 
-    def addBreakpoint(self, args):
+    def insertBreakpoint(self, args):
+        more = True
+        modelId = args['modelid']
         bpType = args["type"]
         if bpType == BreakpointByFileAndLine:
             bp = self.target.BreakpointCreateByLocation(
@@ -1342,68 +1345,45 @@ class Dumper(DumperBase):
                 bp = self.target.WatchAddress(value.GetLoadAddress(),
                     value.GetByteSize(), False, True, error)
             except:
-                return self.target.BreakpointCreateByName(None)
+                bp = self.target.BreakpointCreateByName(None)
         else:
             # This leaves the unhandled breakpoint in a (harmless)
             # "pending" state.
-            return self.target.BreakpointCreateByName(None)
+            bp = self.target.BreakpointCreateByName(None)
-        bp.SetIgnoreCount(int(args["ignorecount"]))
+            more = False
-        if hasattr(bp, 'SetCondition'):
+
-            bp.SetCondition(self.hexdecode(args["condition"]))
+        if more:
-        bp.SetEnabled(int(args["enabled"]))
+            bp.SetIgnoreCount(int(args["ignorecount"]))
-        if hasattr(bp, 'SetOneShot'):
+            if hasattr(bp, 'SetCondition'):
-            bp.SetOneShot(int(args["oneshot"]))
+                bp.SetCondition(self.hexdecode(args["condition"]))
-        self.breakpointsToCheck.add(bp.GetID())
+            bp.SetEnabled(int(args["enabled"]))
-        return bp
+            if hasattr(bp, 'SetOneShot'):
+                bp.SetOneShot(int(args["oneshot"]))
+            self.breakpointsToCheck.add(bp.GetID())
+        self.report('breakpoint-added={%s,modelid="%s"}' % (self.describeBreakpoint(bp), modelId))
 
     def changeBreakpoint(self, args):
-        id = int(args["lldbid"])
+        lldbId = int(args["lldbid"])
-        if id > qqWatchpointOffset:
+        modelId = args['modelid']
-            bp = self.target.FindWatchpointByID(id)
+        if lldbId > qqWatchpointOffset:
+            bp = self.target.FindWatchpointByID(lldbId)
         else:
-            bp = self.target.FindBreakpointByID(id)
+            bp = self.target.FindBreakpointByID(lldbId)
         bp.SetIgnoreCount(int(args["ignorecount"]))
         bp.SetCondition(self.hexdecode(args["condition"]))
         bp.SetEnabled(int(args["enabled"]))
         if hasattr(bp, 'SetOneShot'):
             bp.SetOneShot(int(args["oneshot"]))
-        return bp
+        self.report('breakpoint-changed={%s,modelid="%s"}'
+              % (self.describeBreakpoint(bp), modelId))
 
     def removeBreakpoint(self, args):
-        id = int(args['lldbid'])
+        lldbId = int(args['lldbid'])
-        if id > qqWatchpointOffset:
+        modelId = args['modelid']
-            return self.target.DeleteWatchpoint(id - qqWatchpointOffset)
+        if lldbId > qqWatchpointOffset:
-        return self.target.BreakpointDelete(id)
+            res = self.target.DeleteWatchpoint(lldbId - qqWatchpointOffset)
-
+        res = self.target.BreakpointDelete(lldbId)
-    def handleBreakpoints(self, args):
+        self.report('breakpoint-removed={modelid="%s"}' % modelId)
-        # This seems to be only needed on Linux.
-        needStop = False
-        if self.process and platform.system() == "Linux":
-            needStop = self.process.GetState() != lldb.eStateStopped
-        if needStop:
-            error = self.process.Stop()
-
-        for bp in args['bkpts']:
-            operation = bp['operation']
-            modelId = bp['modelid']
-
-            if operation == 'add':
-                bpNew = self.addBreakpoint(bp)
-                self.report('breakpoint-added={%s,modelid="%s"}'
-                    % (self.describeBreakpoint(bpNew), modelId))
-
-            elif operation == 'change':
-                bpNew = self.changeBreakpoint(bp)
-                self.report('breakpoint-changed={%s,modelid="%s"}'
-                    % (self.describeBreakpoint(bpNew), modelId))
-
-            elif operation == 'remove':
-                bpDead = self.removeBreakpoint(bp)
-                self.report('breakpoint-removed={modelid="%s"}' % modelId)
-
-        if needStop:
-            error = self.process.Continue()
-
 
     def listModules(self, args):
         result = 'modules=['

src/plugins/debugger/lldb/lldbengine.cpp

@@ -309,7 +309,21 @@ void LldbEngine::setupInferior()
     cmd.arg("useTerminal", sp.useTerminal);
     cmd.arg("startMode", sp.startMode);
 
-    attemptBreakpointSynchronizationHelper(&cmd);
+    cmd.beginList("bkpts");
+    foreach (Breakpoint bp, breakHandler()->unclaimedBreakpoints()) {
+        if (acceptsBreakpoint(bp)) {
+            showMessage(_("TAKING OWNERSHIP OF BREAKPOINT %1 IN STATE %2")
+                            .arg(bp.id().toString()).arg(bp.state()));
+            bp.setEngine(this);
+            cmd.beginGroup();
+            insertBreakpointHelper(&cmd, bp);
+            cmd.endGroup();
+        } else {
+            showMessage(_("BREAKPOINT %1 IN STATE %2 IS NOT ACCEPTABLE")
+                .arg(bp.id().toString()).arg(bp.state()));
+        }
+    }
+    cmd.endList();
 
     cmd.beginList("processArgs");
     foreach (const QString &arg, args.toUnixArgs())
@@ -520,113 +534,81 @@ void LldbEngine::selectThread(ThreadId threadId)
     runCommand(DebuggerCommand("selectThread").arg("id", threadId.raw()));
 }
 
+bool LldbEngine::stateAcceptsBreakpointChanges() const
+{
+    switch (state()) {
+    case InferiorSetupRequested:
+    case InferiorRunRequested:
+    case InferiorRunOk:
+    case InferiorStopRequested:
+    case InferiorStopOk:
+        return true;
+    default:
+        return false;
+    }
+}
+
 bool LldbEngine::acceptsBreakpoint(Breakpoint bp) const
 {
-    return bp.parameters().isCppBreakpoint() && startParameters().startMode != AttachCore;
+    if (startParameters().startMode == AttachCore)
+        return false;
+    // We handle QML breakpoint unless specifically disabled.
+    if (isNativeMixedEnabled() && !(startParameters().languages & QmlLanguage))
+        return true;
+    return bp.parameters().isCppBreakpoint();
 }
 
-bool LldbEngine::attemptBreakpointSynchronizationHelper(DebuggerCommand *cmd)
+void LldbEngine::insertBreakpoint(Breakpoint bp)
 {
-    BreakHandler *handler = breakHandler();
+    DebuggerCommand cmd("insertBreakpoint");
-
+    insertBreakpointHelper(&cmd, bp);
-    foreach (Breakpoint bp, handler->unclaimedBreakpoints()) {
+    runCommand(cmd);
-        // Take ownership of the breakpoint. Requests insertion.
-        if (acceptsBreakpoint(bp)) {
-            showMessage(_("TAKING OWNERSHIP OF BREAKPOINT %1 IN STATE %2")
-                .arg(bp.id().toString()).arg(bp.state()));
-            bp.setEngine(this);
-        } else {
-            showMessage(_("BREAKPOINT %1 IN STATE %2 IS NOT ACCEPTABLE")
-                .arg(bp.id().toString()).arg(bp.state()));
-        }
-    }
-
-    bool done = true;
-    cmd->beginList("bkpts");
-    foreach (Breakpoint bp, handler->engineBreakpoints(this)) {
-        const BreakpointResponse &response = bp.response();
-        const BreakpointState bpState = bp.state();
-        switch (bpState) {
-        case BreakpointNew:
-            // Should not happen once claimed.
-            QTC_CHECK(false);
-            break;
-        case BreakpointInsertRequested:
-            done = false;
-            cmd->beginGroup()
-                    .arg("operation", "add")
-                    .arg("modelid", bp.id().toByteArray())
-                    .arg("type", bp.type())
-                    .arg("ignorecount", bp.ignoreCount())
-                    .arg("condition", bp.condition().toHex())
-                    .arg("function", bp.functionName().toUtf8())
-                    .arg("oneshot", bp.isOneShot())
-                    .arg("enabled", bp.isEnabled())
-                    .arg("file", bp.fileName().toUtf8())
-                    .arg("line", bp.lineNumber())
-                    .arg("address", bp.address())
-                    .arg("expression", bp.expression())
-                    .endGroup();
-            bp.notifyBreakpointInsertProceeding();
-            break;
-        case BreakpointChangeRequested:
-            done = false;
-            cmd->beginGroup()
-                    .arg("operation", "change")
-                    .arg("modelid", bp.id().toByteArray())
-                    .arg("lldbid", response.id.toByteArray())
-                    .arg("type", bp.type())
-                    .arg("ignorecount", bp.ignoreCount())
-                    .arg("condition", bp.condition().toHex())
-                    .arg("function", bp.functionName().toUtf8())
-                    .arg("oneshot", bp.isOneShot())
-                    .arg("enabled", bp.isEnabled())
-                    .arg("file", bp.fileName().toUtf8())
-                    .arg("line", bp.lineNumber())
-                    .arg("address", bp.address())
-                    .arg("expression", bp.expression())
-                    .endGroup();
-            bp.notifyBreakpointChangeProceeding();
-            break;
-        case BreakpointRemoveRequested:
-            done = false;
-            cmd->beginGroup()
-                    .arg("operation", "remove")
-                    .arg("modelid", bp.id().toByteArray())
-                    .arg("lldbid", response.id.toByteArray())
-                    .endGroup();
-            bp.notifyBreakpointRemoveProceeding();
-            break;
-        case BreakpointChangeProceeding:
-        case BreakpointInsertProceeding:
-        case BreakpointRemoveProceeding:
-        case BreakpointInserted:
-        case BreakpointDead:
-            QTC_ASSERT(false, qDebug() << "UNEXPECTED STATE" << bpState << "FOR BP " << bp.id());
-            break;
-        default:
-            QTC_ASSERT(false, qDebug() << "UNKNOWN STATE" << bpState << "FOR BP" << bp.id());
-        }
-    }
-    cmd->endList();
-    return done;
 }
 
-void LldbEngine::attemptBreakpointSynchronization()
+void LldbEngine::insertBreakpointHelper(DebuggerCommand *cmd, Breakpoint bp) const
 {
-    showMessage(_("ATTEMPT BREAKPOINT SYNCHRONIZATION"));
+    cmd->arg("modelid", bp.id().toByteArray());
-    if (!stateAcceptsBreakpointChanges()) {
+    cmd->arg("type", bp.type());
-        showMessage(_("BREAKPOINT SYNCHRONIZATION NOT POSSIBLE IN CURRENT STATE"));
+    cmd->arg("ignorecount", bp.ignoreCount());
-        return;
+    cmd->arg("condition", bp.condition().toHex());
-    }
+    cmd->arg("function", bp.functionName().toUtf8());
+    cmd->arg("oneshot", bp.isOneShot());
+    cmd->arg("enabled", bp.isEnabled());
+    cmd->arg("file", bp.fileName().toUtf8());
+    cmd->arg("line", bp.lineNumber());
+    cmd->arg("address", bp.address());
+    cmd->arg("expression", bp.expression());
+    bp.notifyBreakpointInsertProceeding();
+}
 
-    DebuggerCommand cmd("handleBreakpoints");
+void LldbEngine::changeBreakpoint(Breakpoint bp)
-    if (!attemptBreakpointSynchronizationHelper(&cmd)) {
+{
-        showMessage(_("BREAKPOINTS ARE NOT FULLY SYNCHRONIZED"));
+    const BreakpointResponse &response = bp.response();
-        runCommand(cmd);
+    DebuggerCommand cmd("changeBreakpoint");
-    } else {
+    cmd.arg("modelid", bp.id().toByteArray());
-        showMessage(_("BREAKPOINTS ARE SYNCHRONIZED"));
+    cmd.arg("lldbid", response.id.toByteArray());
-    }
+    cmd.arg("type", bp.type());
+    cmd.arg("ignorecount", bp.ignoreCount());
+    cmd.arg("condition", bp.condition().toHex());
+    cmd.arg("function", bp.functionName().toUtf8());
+    cmd.arg("oneshot", bp.isOneShot());
+    cmd.arg("enabled", bp.isEnabled());
+    cmd.arg("file", bp.fileName().toUtf8());
+    cmd.arg("line", bp.lineNumber());
+    cmd.arg("address", bp.address());
+    cmd.arg("expression", bp.expression());
+    bp.notifyBreakpointChangeProceeding();
+    runCommand(cmd);
+}
+
+void LldbEngine::removeBreakpoint(Breakpoint bp)
+{
+    const BreakpointResponse &response = bp.response();
+    DebuggerCommand cmd("removeBreakpoint");
+    cmd.arg("modelid", bp.id().toByteArray());
+    cmd.arg("lldbid", response.id.toByteArray());
+    bp.notifyBreakpointRemoveProceeding();
+    runCommand(cmd);
 }
 
 void LldbEngine::updateBreakpointData(const GdbMi &bkpt, bool added)

src/plugins/debugger/lldb/lldbengine.h

@@ -95,9 +95,13 @@ private:
     void activateFrame(int index);
     void selectThread(ThreadId threadId);
 
+    // This should be always the last call in a function.
+    bool stateAcceptsBreakpointChanges() const;
     bool acceptsBreakpoint(Breakpoint bp) const;
-    void attemptBreakpointSynchronization();
+    void insertBreakpoint(Breakpoint bp);
-    bool attemptBreakpointSynchronizationHelper(DebuggerCommand *command);
+    void insertBreakpointHelper(DebuggerCommand *cmd, Breakpoint bp) const;
+    void removeBreakpoint(Breakpoint bp);
+    void changeBreakpoint(Breakpoint bp);
 
     void assignValueInDebugger(const WatchData *data,
         const QString &expr, const QVariant &value);