From 2b80651d7b49c5ef93ba5dafde4fd27d99bd898f Mon Sep 17 00:00:00 2001 From: hjk Date: Tue, 30 Nov 2010 12:47:53 +0100 Subject: [PATCH] debugger: less magic state transitions in the break handler --- src/plugins/debugger/breakhandler.cpp | 80 ++++++++++++---------- src/plugins/debugger/breakhandler.h | 3 + src/plugins/debugger/breakpoint.cpp | 2 +- src/plugins/debugger/gdb/gdbengine.cpp | 119 +++++++++++++++++++-------------- src/plugins/debugger/gdb/gdbengine.h | 1 + 5 files changed, 117 insertions(+), 88 deletions(-) diff --git a/src/plugins/debugger/breakhandler.cpp b/src/plugins/debugger/breakhandler.cpp index f6acc6de7a..751a57ee47 100644 --- a/src/plugins/debugger/breakhandler.cpp +++ b/src/plugins/debugger/breakhandler.cpp @@ -218,7 +218,6 @@ void BreakHandler::setWatchpointByAddress(quint64 address) BreakpointParameters data(Watchpoint); data.address = address; appendBreakpoint(data); - scheduleSynchronization(); } else { qDebug() << "WATCHPOINT EXISTS"; // removeBreakpoint(index); @@ -655,71 +654,73 @@ void BreakHandler::setState(BreakpointId id, BreakpointState state) it->state = state; } +void BreakHandler::notifyBreakpointChangeAfterInsertNeeded(BreakpointId id) +{ + QTC_ASSERT(state(id) == BreakpointInsertProceeding, qDebug() << state(id)); + setState(id, BreakpointChangeRequested); +} + void BreakHandler::notifyBreakpointInsertProceeding(BreakpointId id) { - QTC_ASSERT(state(id) == BreakpointInsertRequested, /**/); + QTC_ASSERT(state(id) == BreakpointInsertRequested, qDebug() << state(id)); setState(id, BreakpointInsertProceeding); } void BreakHandler::notifyBreakpointInsertOk(BreakpointId id) { - QTC_ASSERT(state(id) == BreakpointInsertProceeding, /**/); + QTC_ASSERT(state(id) == BreakpointInsertProceeding, qDebug() << state(id)); setState(id, BreakpointInserted); ConstIterator it = m_storage.find(id); QTC_ASSERT(it != m_storage.end(), return); - //if (it0->needsChange(it->data, it->response)) { - // setState(id, BreakpointChangeRequested); - // scheduleSynchronization(); - //} } void BreakHandler::notifyBreakpointInsertFailed(BreakpointId id) { - QTC_ASSERT(state(id) == BreakpointInsertProceeding, /**/); + QTC_ASSERT(state(id) == BreakpointInsertProceeding, qDebug() << state(id)); setState(id, BreakpointDead); } void BreakHandler::notifyBreakpointRemoveProceeding(BreakpointId id) { - QTC_ASSERT(state(id) == BreakpointRemoveRequested, /**/); + QTC_ASSERT(state(id) == BreakpointRemoveRequested, qDebug() << state(id)); setState(id, BreakpointRemoveProceeding); } void BreakHandler::notifyBreakpointRemoveOk(BreakpointId id) { - QTC_ASSERT(state(id) == BreakpointRemoveProceeding, /**/); + QTC_ASSERT(state(id) == BreakpointRemoveProceeding, qDebug() << state(id)); setState(id, BreakpointDead); cleanupBreakpoint(id); } void BreakHandler::notifyBreakpointRemoveFailed(BreakpointId id) { - QTC_ASSERT(state(id) == BreakpointRemoveProceeding, /**/); + QTC_ASSERT(state(id) == BreakpointRemoveProceeding, qDebug() << state(id)); setState(id, BreakpointDead); cleanupBreakpoint(id); } void BreakHandler::notifyBreakpointChangeProceeding(BreakpointId id) { - QTC_ASSERT(state(id) == BreakpointChangeRequested, /**/); + QTC_ASSERT(state(id) == BreakpointChangeRequested, qDebug() << state(id)); setState(id, BreakpointChangeProceeding); } void BreakHandler::notifyBreakpointChangeOk(BreakpointId id) { - QTC_ASSERT(state(id) == BreakpointChangeProceeding, /**/); + QTC_ASSERT(state(id) == BreakpointChangeProceeding, qDebug() << state(id)); setState(id, BreakpointInserted); } void BreakHandler::notifyBreakpointChangeFailed(BreakpointId id) { - QTC_ASSERT(state(id) == BreakpointChangeProceeding, /**/); + QTC_ASSERT(state(id) == BreakpointChangeProceeding, qDebug() << state(id)); setState(id, BreakpointDead); } void BreakHandler::notifyBreakpointReleased(BreakpointId id) { - //QTC_ASSERT(state(id) == BreakpointChangeProceeding, /**/); + //QTC_ASSERT(state(id) == BreakpointChangeProceeding, qDebug() << state(id)); Iterator it = m_storage.find(id); QTC_ASSERT(it != m_storage.end(), return); it->state = BreakpointNew; @@ -734,14 +735,21 @@ void BreakHandler::notifyBreakpointReleased(BreakpointId id) void BreakHandler::notifyBreakpointAdjusted(BreakpointId id, const BreakpointParameters &data) { - QTC_ASSERT(state(id) == BreakpointInserted, /**/); + QTC_ASSERT(state(id) == BreakpointInserted, qDebug() << state(id)); Iterator it = m_storage.find(id); QTC_ASSERT(it != m_storage.end(), return); it->data = data; - if (it->needsChange()) - setState(id, BreakpointChangeRequested); + //if (it->needsChange()) + // setState(id, BreakpointChangeRequested); } +void BreakHandler::notifyBreakpointNeedsReinsertion(BreakpointId id) +{ + QTC_ASSERT(state(id) == BreakpointInserted, qDebug() << state(id)); + Iterator it = m_storage.find(id); + QTC_ASSERT(it != m_storage.end(), return); + it->state = BreakpointNew; +} void BreakHandler::removeBreakpoint(BreakpointId id) { @@ -833,11 +841,8 @@ void BreakHandler::timerEvent(QTimerEvent *event) QTC_ASSERT(event->timerId() == m_syncTimerId, return); killTimer(m_syncTimerId); m_syncTimerId = -1; - //qDebug() << "BREAKPOINT SYNCRONIZATION STARTED"; - debuggerCore()->synchronizeBreakpoints(); - updateMarkers(); - emit layoutChanged(); saveBreakpoints(); // FIXME: remove? + debuggerCore()->synchronizeBreakpoints(); } void BreakHandler::gotoLocation(BreakpointId id) const @@ -903,7 +908,7 @@ BreakpointIds BreakHandler::engineBreakpointIds(DebuggerEngine *engine) const void BreakHandler::cleanupBreakpoint(BreakpointId id) { - QTC_ASSERT(state(id) == BreakpointDead, /**/); + QTC_ASSERT(state(id) == BreakpointDead, qDebug() << state(id)); BreakpointItem item = m_storage.take(id); item.destroyMarker(); layoutChanged(); @@ -917,19 +922,18 @@ const BreakpointResponse &BreakHandler::response(BreakpointId id) const return it->response; } +bool BreakHandler::needsChange(BreakpointId id) const +{ + ConstIterator it = m_storage.find(id); + QTC_ASSERT(it != m_storage.end(), return false); + return it->needsChange(); +} + void BreakHandler::setResponse(BreakpointId id, const BreakpointResponse &data) { Iterator it = m_storage.find(id); QTC_ASSERT(it != m_storage.end(), return); it->response = data; - //qDebug() << "SET RESPONSE: " << id << it->state << it->needsChange(); - if (it->state == BreakpointChangeProceeding - || it->state == BreakpointInsertProceeding) { - if (it->needsChange()) - setState(id, BreakpointChangeRequested); - else - setState(id, BreakpointInserted); - } it->destroyMarker(); updateMarker(id); } @@ -941,12 +945,14 @@ void BreakHandler::setBreakpointData(BreakpointId id, const BreakpointParameters if (data == it->data) return; it->data = data; - it->destroyMarker(); - updateMarker(id); - layoutChanged(); - if (it->state == BreakpointInserted) + if (it->needsChange()) { setState(id, BreakpointChangeRequested); - scheduleSynchronization(); + scheduleSynchronization(); + } else { + it->destroyMarker(); + updateMarker(id); + layoutChanged(); + } } ////////////////////////////////////////////////////////////////// @@ -1027,6 +1033,8 @@ bool BreakHandler::BreakpointItem::needsChange() const return true; if (data.enabled != response.enabled) return true; + if (data.threadSpec != response.threadSpec) + return true; return false; } diff --git a/src/plugins/debugger/breakhandler.h b/src/plugins/debugger/breakhandler.h index 59589f59ca..36d78c83d8 100644 --- a/src/plugins/debugger/breakhandler.h +++ b/src/plugins/debugger/breakhandler.h @@ -130,8 +130,10 @@ public: void setEngine(BreakpointId id, DebuggerEngine *engine); const BreakpointResponse &response(BreakpointId id) const; void setResponse(BreakpointId id, const BreakpointResponse &data); + bool needsChange(BreakpointId id) const; // State transitions. + void notifyBreakpointChangeAfterInsertNeeded(BreakpointId id); void notifyBreakpointInsertProceeding(BreakpointId id); void notifyBreakpointInsertOk(BreakpointId id); void notifyBreakpointInsertFailed(BreakpointId id); @@ -143,6 +145,7 @@ public: void notifyBreakpointRemoveOk(BreakpointId id); void notifyBreakpointRemoveFailed(BreakpointId id); void notifyBreakpointReleased(BreakpointId id); + void notifyBreakpointNeedsReinsertion(BreakpointId id); void notifyBreakpointAdjusted(BreakpointId id, const BreakpointParameters &data); diff --git a/src/plugins/debugger/breakpoint.cpp b/src/plugins/debugger/breakpoint.cpp index 7ad027e2b8..e92721b280 100644 --- a/src/plugins/debugger/breakpoint.cpp +++ b/src/plugins/debugger/breakpoint.cpp @@ -52,7 +52,7 @@ bool BreakpointParameters::equals(const BreakpointParameters &rhs) const && enabled == rhs.enabled && useFullPath == rhs.useFullPath && fileName == rhs.fileName - && condition == rhs.condition + && conditionsMatch(rhs.condition) && ignoreCount == rhs.ignoreCount && lineNumber == rhs.lineNumber && address == rhs.address diff --git a/src/plugins/debugger/gdb/gdbengine.cpp b/src/plugins/debugger/gdb/gdbengine.cpp index e7f5057d33..d333b09f15 100644 --- a/src/plugins/debugger/gdb/gdbengine.cpp +++ b/src/plugins/debugger/gdb/gdbengine.cpp @@ -482,7 +482,7 @@ void GdbEngine::handleResponse(const QByteArray &buff) // On Windows, the contents seem to depend on the debugger // version and/or OS version used. if (data.startsWith("warning:")) - showMessage(_(data.mid(9)), AppStuff); // cut "warning: " + showMessage(_(data.mid(9)), AppStuff); // Cut "warning: " break; } @@ -499,14 +499,6 @@ void GdbEngine::handleResponse(const QByteArray &buff) if (resultClass == "done") { response.resultClass = GdbResultDone; } else if (resultClass == "running") { - // FIXME: Handle this in the individual result handlers. - //if (state() == InferiorStopOk) { // Result of manual command. - // resetLocation(); - // setTokenBarrier(); - // notifyInferiorRunRequested(); - //} - //notifyInferiorRunOk(); - //showStatusMessage(tr("Running...")); response.resultClass = GdbResultRunning; } else if (resultClass == "connected") { response.resultClass = GdbResultConnected; @@ -526,7 +518,7 @@ void GdbEngine::handleResponse(const QByteArray &buff) response.data.m_type = GdbMi::Tuple; response.data.m_name = "data"; } else { - // Archer has this + // Archer has this. response.data.m_type = GdbMi::Tuple; response.data.m_name = "data"; } @@ -2182,10 +2174,13 @@ void GdbEngine::handleWatchInsert(const GdbResponse &response) const int end = ba.indexOf(':'); const int begin = ba.lastIndexOf(' ', end) + 1; const QByteArray address = ba.mid(end + 3).trimmed(); - BreakpointResponse response = breakHandler()->response(id); + BreakHandler *handler = breakHandler(); + BreakpointResponse response = handler->response(id); response.number = ba.mid(begin, end - begin).toInt(); response.address = address.toULongLong(0, 0); - breakHandler()->setResponse(id, response); + handler->setResponse(id, response); + QTC_ASSERT(!handler->needsChange(id), /**/); + handler->notifyBreakpointInsertOk(id); } else { showMessage(_("CANNOT PARSE WATCHPOINT FROM " + ba)); } @@ -2214,7 +2209,14 @@ void GdbEngine::handleBreakInsert1(const GdbResponse &response) // Interesting only on Mac? GdbMi bkpt = response.data.findChild("bkpt"); updateBreakpointDataFromOutput(id, bkpt); - attemptAdjustBreakpointLocation(id); + BreakHandler *handler = breakHandler(); + if (handler->needsChange(id)) { + handler->notifyBreakpointChangeAfterInsertNeeded(id); + changeBreakpoint(id); + } else { + handler->notifyBreakpointInsertOk(id); + attemptAdjustBreakpointLocation(id); + } } else { // Some versions of gdb like "GNU gdb (GDB) SUSE (6.8.91.20090930-2.4)" // know how to do pending breakpoints using CLI but not MI. So try @@ -2308,22 +2310,39 @@ void GdbEngine::handleBreakDisable(const GdbResponse &response) { QTC_ASSERT(response.resultClass == GdbResultDone, /**/) const BreakpointId id = response.cookie.toInt(); + BreakHandler *handler = breakHandler(); // This should only be the requested state. - QTC_ASSERT(!breakHandler()->isEnabled(id), /* Prevent later recursion */); - BreakpointResponse br = breakHandler()->response(id); + QTC_ASSERT(!handler->isEnabled(id), /* Prevent later recursion */); + BreakpointResponse br = handler->response(id); br.enabled = false; - breakHandler()->setResponse(id, br); + handler->setResponse(id, br); + changeBreakpoint(id); // Maybe there's more to do. } void GdbEngine::handleBreakEnable(const GdbResponse &response) { QTC_ASSERT(response.resultClass == GdbResultDone, /**/) const BreakpointId id = response.cookie.toInt(); + BreakHandler *handler = breakHandler(); // This should only be the requested state. - QTC_ASSERT(breakHandler()->isEnabled(id), /* Prevent later recursion */); - BreakpointResponse br = breakHandler()->response(id); + QTC_ASSERT(handler->isEnabled(id), /* Prevent later recursion */); + BreakpointResponse br = handler->response(id); br.enabled = true; - breakHandler()->setResponse(id, br); + handler->setResponse(id, br); + changeBreakpoint(id); // Maybe there's more to do. +} + +void GdbEngine::handleBreakThreadSpec(const GdbResponse &response) +{ + QTC_ASSERT(response.resultClass == GdbResultDone, /**/) + const BreakpointId id = response.cookie.toInt(); + BreakHandler *handler = breakHandler(); + handler->notifyBreakpointChangeOk(id); + handler->notifyBreakpointNeedsReinsertion(id); + BreakpointResponse br = handler->response(id); + br.threadSpec = handler->threadSpec(id); + handler->setResponse(id, br); + changeBreakpoint(id); // Maybe there's more to do. } void GdbEngine::handleBreakIgnore(const GdbResponse &response) @@ -2340,30 +2359,36 @@ void GdbEngine::handleBreakIgnore(const GdbResponse &response) // gdb 6.3 does not produce any console output QTC_ASSERT(response.resultClass == GdbResultDone, /**/) QString msg = _(response.data.findChild("consolestreamoutput").data()); - BreakpointId id(response.cookie.toInt()); - BreakpointResponse br = breakHandler()->response(id); + BreakpointId id = response.cookie.toInt(); + BreakHandler *handler = breakHandler(); + BreakpointResponse br = handler->response(id); //if (msg.contains(__("Will stop next time breakpoint"))) // response.ignoreCount = _("0"); //else if (msg.contains(__("Will ignore next"))) // response.ignoreCount = data->ignoreCount; // FIXME: this assumes it is doing the right thing... - br.ignoreCount = breakHandler()->ignoreCount(id); - breakHandler()->setResponse(id, br); + br.ignoreCount = handler->ignoreCount(id); + handler->setResponse(id, br); + changeBreakpoint(id); // Maybe there's more to do. } void GdbEngine::handleBreakCondition(const GdbResponse &response) { - QTC_ASSERT(response.resultClass == GdbResultDone, /**/) + // Can happen at invalid condition strings. + //QTC_ASSERT(response.resultClass == GdbResultDone, /**/) const BreakpointId id = response.cookie.toInt(); + qDebug() << "CONDITION FOR" << id; + BreakHandler *handler = breakHandler(); // We just assume it was successful. Otherwise we had to parse // the output stream data. // The following happens on Mac: // QByteArray msg = response.data.findChild("msg").data(); // if (1 || msg.startsWith("Error parsing breakpoint condition. " // " Will try again when we hit the breakpoint.")) { - BreakpointResponse br = breakHandler()->response(id); - br.condition = breakHandler()->condition(id); - breakHandler()->setResponse(id, br); + BreakpointResponse br = handler->response(id); + br.condition = handler->condition(id); + handler->setResponse(id, br); + changeBreakpoint(id); // Maybe there's more to do. } void GdbEngine::extractDataFromInfoBreak(const QString &output, BreakpointId id) @@ -2442,10 +2467,10 @@ void GdbEngine::handleInfoLine(const GdbResponse &response) // at address 0x80526aa <_Z10...+131> and ends at 0x80526b5 // <_Z10testQStackv+142>.\n" QByteArray ba = response.data.findChild("consolestreamoutput").data(); + const BreakpointId id = response.cookie.toInt(); const int pos = ba.indexOf(' ', 5); if (ba.startsWith("Line ") && pos != -1) { const int line = ba.mid(5, pos - 5).toInt(); - const BreakpointId id = response.cookie.toInt(); BreakpointResponse br = breakHandler()->response(id); br.lineNumber = line; breakHandler()->setResponse(id, br); @@ -2620,54 +2645,46 @@ void GdbEngine::changeBreakpoint(BreakpointId id) QTC_ASSERT(response.number > 0, return); const QByteArray bpnr = QByteArray::number(response.number); QTC_ASSERT(response.number > 0, return); - - bool finished = true; - if (data.condition != response.condition - && !data.conditionsMatch(response.condition)) { - // Update conditions if needed. + const BreakpointState state = handler->state(id); + if (state == BreakpointChangeRequested) handler->notifyBreakpointChangeProceeding(id); - finished = false; + const BreakpointState state2 = handler->state(id); + QTC_ASSERT(state2 == BreakpointChangeProceeding, qDebug() << state2); + + if (!data.conditionsMatch(response.condition)) { postCommand("condition " + bpnr + ' ' + data.condition, NeedsStop | RebuildBreakpointModel, CB(handleBreakCondition), id); + return; } if (data.ignoreCount != response.ignoreCount) { - // Update ignorecount if needed. - handler->notifyBreakpointChangeProceeding(id); - finished = false; postCommand("ignore " + bpnr + ' ' + QByteArray::number(data.ignoreCount), NeedsStop | RebuildBreakpointModel, CB(handleBreakIgnore), id); + return; } if (!data.enabled && response.enabled) { - handler->notifyBreakpointChangeProceeding(id); - finished = false; postCommand("-break-disable " + bpnr, NeedsStop | RebuildBreakpointModel, CB(handleBreakDisable), id); + return; } if (data.enabled && !response.enabled) { - handler->notifyBreakpointChangeProceeding(id); - finished = false; postCommand("-break-enable " + bpnr, NeedsStop | RebuildBreakpointModel, CB(handleBreakEnable), id); + return; } - if (data.threadSpec != response.threadSpec) { // The only way to change this seems to be to re-set the bp completely. - qDebug() << "FIXME: THREAD: " << data.threadSpec << response.threadSpec; - //response.threadSpec.clear(); - //postCommand("-break-delete " + bpnr, - // NeedsStop | RebuildBreakpointModel); - //sendInsertBreakpoint(index); - //continue; + postCommand("-break-delete " + bpnr, + NeedsStop | RebuildBreakpointModel, + CB(handleBreakThreadSpec), id); + return; } + handler->notifyBreakpointChangeOk(id); attemptAdjustBreakpointLocation(id); - - if (finished) - handler->notifyBreakpointChangeOk(id); } void GdbEngine::removeBreakpoint(BreakpointId id) diff --git a/src/plugins/debugger/gdb/gdbengine.h b/src/plugins/debugger/gdb/gdbengine.h index a641859e2e..d4e512c7d6 100644 --- a/src/plugins/debugger/gdb/gdbengine.h +++ b/src/plugins/debugger/gdb/gdbengine.h @@ -355,6 +355,7 @@ private: ////////// View & Data Stuff ////////// void handleBreakInsert2(const GdbResponse &response); void handleBreakCondition(const GdbResponse &response); void handleBreakInfo(const GdbResponse &response); + void handleBreakThreadSpec(const GdbResponse &response); void handleWatchInsert(const GdbResponse &response); void handleInfoLine(const GdbResponse &response); void extractDataFromInfoBreak(const QString &output, BreakpointId); -- 2.11.0