CDB: Fix stepping and a crash on dumping expanded pointer items.

Simplify & Generalize stepping code, introduce stubs for reverse
debugging. When user switches to another thread than the
event-triggering one in a stopped inferior, explicitly tell
the engine to continue that one.

Improve fixDumperResult to fix missing types (out of scope items)
and call it from expandPointerDumpable.

src/plugins/debugger/cdb/cdbdebugengine.cpp

@@ -120,6 +120,11 @@ QString msgComFailed(const char *func, HRESULT hr)
     return QString::fromLatin1("%1 failed: %2").arg(QLatin1String(func), msgDebugEngineComResult(hr));
 }
 
+QString msgDebuggerCommandFailed(const QString &command, HRESULT hr)
+{
+    return QString::fromLatin1("Unable to execute '%1': %2").arg(command, msgDebugEngineComResult(hr));
+}
+
 static const char *msgNoStackTraceC = "Internal error: no stack trace present.";
 
 static inline QString msgLibLoadFailed(const QString &lib, const QString &why)
@@ -295,6 +300,8 @@ CdbDebugEnginePrivate::CdbDebugEnginePrivate(DebuggerManager *manager,
     m_hDebuggeeThread(0),
     m_breakEventMode(BreakEventHandle),
     m_dumper(new CdbDumperHelper(manager, &m_cif)),
+    m_currentThreadId(-1),
+    m_eventThreadId(-1),
     m_watchTimer(-1),
     m_debugEventCallBack(engine),
     m_engine(engine),
@@ -437,9 +444,8 @@ void CdbDebugEnginePrivate::clearForRun()
         qDebug() << Q_FUNC_INFO;
 
     m_breakEventMode = BreakEventHandle;
-    m_firstActivatedFrame = false;
+    m_eventThreadId = -1;
     cleanStackTrace();
-    m_editorToolTipCache.clear();
 }
 
 void CdbDebugEnginePrivate::cleanStackTrace()
@@ -448,6 +454,8 @@ void CdbDebugEnginePrivate::cleanStackTrace()
         delete m_currentStackTrace;
         m_currentStackTrace = 0;
     }
+    m_firstActivatedFrame = false;
+    m_editorToolTipCache.clear();
 }
 
 CdbDebugEngine::CdbDebugEngine(DebuggerManager *manager, const QSharedPointer<CdbOptions> &options) :
@@ -965,24 +973,85 @@ bool CdbDebugEnginePrivate::executeContinueCommand(const QString &command)
     return success;
 }
 
-void CdbDebugEngine::stepExec()
+static inline QString msgStepFailed(unsigned long executionStatus, int threadId, const QString &why)
+{
+    if (executionStatus ==  DEBUG_STATUS_STEP_OVER)
+        return QString::fromLatin1("Thread %1: Unable to step over: %2").arg(threadId).arg(why);
+    return QString::fromLatin1("Thread %1: Unable to step into: %2").arg(threadId).arg(why);
+}
+
+// Step with  DEBUG_STATUS_STEP_OVER ('p'-command) or
+// DEBUG_STATUS_STEP_INTO ('t'-trace-command) or
+// its reverse equivalents in the case of single threads.
+bool CdbDebugEngine::step(unsigned long executionStatus)
 {
-    // Step into
     if (debugCDB)
-        qDebug() << Q_FUNC_INFO;
+        qDebug() << Q_FUNC_INFO << executionStatus << "curr " << m_d->m_currentThreadId << " evt " << m_d->m_eventThreadId;
 
-    m_d->clearForRun();
-    m_d->setCodeLevel();
+    // State of reverse stepping as of 10/2009 (Debugging tools 6.11@404):
+    // The constants exist, but invoking the calls leads to E_NOINTERFACE.
+    // Also there is no CDB command for it.
+    if (executionStatus == DEBUG_STATUS_REVERSE_STEP_OVER || executionStatus == DEBUG_STATUS_REVERSE_STEP_INTO) {
+        warning(tr("Reverse stepping is not implemented."));
+        return false;
+    }
+
+    // SetExecutionStatus() continues the thread that triggered the
+    // stop event (~# p). This can be confusing if the user is looking
+    // at the stack trace of another thread and wants to step that one. If that
+    // is the case, explicitly tell it to step the current thread using a command.
+    const int triggeringEventThread = m_d->m_eventThreadId;
+    const bool sameThread = triggeringEventThread == -1
+                            || m_d->m_currentThreadId == triggeringEventThread
+                            || manager()->threadsHandler()->threads().size() == 1;
+    m_d->clearForRun(); // clears thread ids
+    m_d->setCodeLevel(); // Step by instruction or source line
     setState(InferiorRunningRequested, Q_FUNC_INFO, __LINE__);
-    const HRESULT hr = m_d->m_cif.debugControl->SetExecutionStatus(DEBUG_STATUS_STEP_INTO);
-    if (SUCCEEDED(hr)) {
+    bool success = false;
+    if (sameThread) { // Step event-triggering thread, use fast API
+        const HRESULT hr = m_d->m_cif.debugControl->SetExecutionStatus(executionStatus);
+        success = SUCCEEDED(hr);
+        if (!success)
+            warning(msgStepFailed(executionStatus, m_d->m_currentThreadId, msgComFailed("SetExecutionStatus", hr)));
+    } else {
+        // Need to use a command to explicitly specify the current thread
+        QString command;
+        QTextStream str(&command);
+        str << '~' << m_d->m_currentThreadId << ' '
+                << (executionStatus == DEBUG_STATUS_STEP_OVER ? 'p' : 't');
+        manager()->showDebuggerOutput(tr("Stepping %1").arg(command));
+        const HRESULT hr = m_d->m_cif.debugControl->Execute(DEBUG_OUTCTL_THIS_CLIENT, command.toLatin1().constData(), DEBUG_EXECUTE_ECHO);
+        success = SUCCEEDED(hr);
+        if (!success)
+            warning(msgStepFailed(executionStatus, m_d->m_currentThreadId, msgDebuggerCommandFailed(command, hr)));
+    }
+    if (success) {
+        startWatchTimer();
         setState(InferiorRunning, Q_FUNC_INFO, __LINE__);
     } else {
         setState(InferiorStopped, Q_FUNC_INFO, __LINE__);
-        warning(msgFunctionFailed(Q_FUNC_INFO, msgComFailed("SetExecutionStatus", hr)));
     }
-    m_d->m_breakEventMode = CdbDebugEnginePrivate::BreakEventIgnoreOnce;
-    startWatchTimer();
+    return success;
+}
+
+void CdbDebugEngine::stepExec()
+{
+    step(manager()->isReverseDebugging() ? DEBUG_STATUS_REVERSE_STEP_INTO : DEBUG_STATUS_STEP_INTO);
+}
+
+void CdbDebugEngine::nextExec()
+{
+    step(manager()->isReverseDebugging() ? DEBUG_STATUS_REVERSE_STEP_OVER : DEBUG_STATUS_STEP_OVER);
+}
+
+void CdbDebugEngine::stepIExec()
+{      
+    stepExec(); // Step into by instruction (figured out by step)
+}
+
+void CdbDebugEngine::nextIExec()
+{
+    nextExec(); // Step over by instruction (figured out by step)
 }
 
 void CdbDebugEngine::stepOutExec()
@@ -1027,37 +1096,6 @@ void CdbDebugEngine::stepOutExec()
        warning(msgFunctionFailed(Q_FUNC_INFO, errorMessage));
 }
 
-void CdbDebugEngine::nextExec()
-{
-    // Step over
-    if (debugCDB)
-        qDebug() << Q_FUNC_INFO;
-    m_d->clearForRun();
-    m_d->setCodeLevel(); // Step by instruction
-    setState(InferiorRunningRequested, Q_FUNC_INFO, __LINE__);
-    const HRESULT hr = m_d->m_cif.debugControl->SetExecutionStatus(DEBUG_STATUS_STEP_OVER);
-    // To control threads, use:
-    // -- const HRESULT hr = m_d->m_cif.debugControl->Execute(DEBUG_OUTCTL_THIS_CLIENT, "~* p", 0);
-    if (SUCCEEDED(hr)) {
-        setState(InferiorRunning, Q_FUNC_INFO, __LINE__);
-        startWatchTimer();        
-    } else {
-        setState(InferiorStopped, Q_FUNC_INFO, __LINE__);
-        warning(msgFunctionFailed(Q_FUNC_INFO, msgComFailed("SetExecutionStatus", hr)));
-    }
-}
-
-void CdbDebugEngine::stepIExec()
-{   
-    // Step by instruction
-    nextExec();
-}
-
-void CdbDebugEngine::nextIExec()
-{
-    nextExec();
-}
-
 void CdbDebugEngine::continueInferior()
 {
     QString errorMessage;    
@@ -1603,7 +1641,7 @@ void CdbDebugEnginePrivate::handleDebugEvent()
         if (m_engine->state() != InferiorStopping)
             m_engine->setState(InferiorStopping, Q_FUNC_INFO, __LINE__);
         m_engine->setState(InferiorStopped, Q_FUNC_INFO, __LINE__);
-        m_currentThreadId = updateThreadList();
+        m_eventThreadId = m_currentThreadId = updateThreadList();
         manager()->showDebuggerOutput(LogMisc, CdbDebugEngine::tr("Stopped, current thread: %1").arg(m_currentThreadId));
         ThreadsHandler *threadsHandler = manager()->threadsHandler();
         const int threadIndex = threadIndexById(threadsHandler, m_currentThreadId);
@@ -1701,7 +1739,7 @@ void CdbDebugEnginePrivate::updateStackTrace()
     if (debugCDB)
         qDebug() << Q_FUNC_INFO;
     // Create a new context
-    clearForRun();
+    cleanStackTrace();
     QString errorMessage;
     m_engine->reloadRegisters();
     if (!setCDBThreadId(m_currentThreadId, &errorMessage)) {
@@ -1727,6 +1765,15 @@ void CdbDebugEnginePrivate::updateStackTrace()
             current = i;
             break;
         }
+    // Visibly warn the users about missing top frames/all frames, as they otherwise
+    // might think stepping is broken.    
+    if (!stackFrames.at(0).isUsable()) {
+        const QString topFunction = count ? stackFrames.at(0).function : QString();
+        const QString msg = current >= 0 ?
+                            CdbDebugEngine::tr("Thread %1: Missing debug information for top stack frame (%2).").arg(m_currentThreadId).arg(topFunction) :
+                            CdbDebugEngine::tr("Thread %1: No debug information available (%2).").arg(m_currentThreadId).arg(topFunction);
+        m_engine->warning(msg);
+    }
 
     manager()->stackHandler()->setFrames(stackFrames);
     m_firstActivatedFrame = true;

src/plugins/debugger/cdb/cdbdebugengine.h

@@ -124,6 +124,7 @@ private:
     bool evaluateExpression(const QString &expression, QString *value, QString *type, QString *errorMessage);
     void evaluateWatcher(WatchData *wd);
     QString editorToolTip(const QString &exp, const QString &function);
+    bool step(unsigned long executionStatus);
 
     CdbDebugEnginePrivate *m_d;
 

src/plugins/debugger/cdb/cdbdebugengine_p.h

@@ -157,6 +157,7 @@ struct CdbDebugEnginePrivate
     HANDLE                  m_hDebuggeeProcess;
     HANDLE                  m_hDebuggeeThread;
     int                     m_currentThreadId;
+    int                     m_eventThreadId;
     HandleBreakEventMode    m_breakEventMode;
 
     int                     m_watchTimer;

src/plugins/debugger/cdb/cdbstackframecontext.cpp

@@ -128,50 +128,21 @@ WatchHandleDumperInserter::WatchHandleDumperInserter(WatchHandler *wh,
     Q_ASSERT(m_hexNullPattern.isValid());
 }
 
-// Is this a non-null pointer to a dumpeable item with a value
-// "0x4343 class QString *" ? - Insert a fake '*' dereferenced item
-// and run dumpers on it. If that succeeds, insert the fake items owned by dumpers,
-// which will trigger the ignore predicate.
-// Note that the symbol context does not create '*' dereferenced items for
-// classes (see note in its header documentation).
-bool WatchHandleDumperInserter::expandPointerToDumpable(const WatchData &wd, QString *errorMessage)
-{
-    if (debugCDBWatchHandling)
-        qDebug() << ">expandPointerToDumpable" << wd.iname;
 
-    bool handled = false;
-    do {
-        if (!isPointerType(wd.type))
-            break;
-        const int classPos = wd.value.indexOf(" class ");
-        if (classPos == -1)
-            break;
-        const QString hexAddrS = wd.value.mid(0, classPos);
-        if (m_hexNullPattern.exactMatch(hexAddrS))
-            break;
-        const QString type = stripPointerType(wd.value.mid(classPos + 7));
-        WatchData derefedWd;
-        derefedWd.setType(type);
-        derefedWd.setAddress(hexAddrS);
-        derefedWd.name = QString(QLatin1Char('*'));
-        derefedWd.iname = wd.iname + QLatin1String(".*");
-        derefedWd.source = OwnerDumper | CdbStackFrameContext::ChildrenKnownBit;
-        const CdbDumperHelper::DumpResult dr = m_dumper->dumpType(derefedWd, true, &m_dumperResult, errorMessage);
-        if (dr != CdbDumperHelper::DumpOk)
-            break;
-        // Insert the pointer item with 1 additional child + its dumper results
-        // Note: formal arguments might already be expanded in the symbol group.
-        WatchData ptrWd = wd;
-        ptrWd.source = OwnerDumper | CdbStackFrameContext::ChildrenKnownBit;
-        ptrWd.setHasChildren(true);
-        ptrWd.setChildrenUnneeded();
-        m_wh->insertData(ptrWd);
-        m_wh->insertBulkData(m_dumperResult);
-        handled = true;
-    } while (false);
-    if (debugCDBWatchHandling)
-        qDebug() << "<expandPointerToDumpable returns " << handled << *errorMessage;
-    return handled;
+// Prevent recursion of the model by setting value and type
+static inline void fixDumperValueAndType(WatchData *wd, const WatchData *source = 0)
+{
+    static const QString unknown = QCoreApplication::translate("CdbStackFrameContext", "<Unknown>");
+    if (wd->isTypeNeeded() || wd->type.isEmpty()) {
+        wd->setType(source ? source->type : unknown);
+    }
+    if (wd->isValueNeeded()) {
+        if (source && source->isValueKnown()) {
+            wd->setValue(source->value);
+        } else {
+            wd->setValue(unknown);
+        }
+    }
 }
 
 // When querying an item, the queried item is sometimes returned in incomplete form.
@@ -180,6 +151,7 @@ static inline void fixDumperResult(const WatchData &source,
                                    QList<WatchData> *result,
                                    bool suppressGrandChildren)
 {
+
     const int size = result->size();
     if (!size)
         return;
@@ -188,16 +160,7 @@ static inline void fixDumperResult(const WatchData &source,
     WatchData &returned = result->front();
     if (returned.iname != source.iname)
         return;
-    if (returned.type.isEmpty())
-        returned.setType(source.type);
-    if (returned.isValueNeeded()) {
-        if (source.isValueKnown()) {
-            returned.setValue(source.value);
-        } else {
-            // Should not happen
-            returned.setValue(QCoreApplication::translate("CdbStackFrameContext", "<Unknown>"));
-        }
-    }
+    fixDumperValueAndType(&returned, &source);
     // Indicate owner and known children
     returned.source = OwnerDumper;
     if (returned.isChildrenKnown() && returned.isHasChildrenKnown() && returned.hasChildren)
@@ -225,11 +188,60 @@ static inline void fixDumperResult(const WatchData &source,
             if (suppressGrandChildren && (wd.isChildrenNeeded() || wd.isHasChildrenNeeded()))
                 wd.setHasChildren(false);
         }
+        //   <Out of scope value> have sometimes missing types. Kill recursion
+        fixDumperValueAndType(&wd);
     }
     if (debugCDBWatchHandling)
         debugWatchDataList(*result, "<fixDumperResult");
 }
 
+// Is this a non-null pointer to a dumpeable item with a value
+// "0x4343 class QString *" ? - Insert a fake '*' dereferenced item
+// and run dumpers on it. If that succeeds, insert the fake items owned by dumpers,
+// which will trigger the ignore predicate.
+// Note that the symbol context does not create '*' dereferenced items for
+// classes (see note in its header documentation).
+bool WatchHandleDumperInserter::expandPointerToDumpable(const WatchData &wd, QString *errorMessage)
+{
+    if (debugCDBWatchHandling)
+        qDebug() << ">expandPointerToDumpable" << wd.toString();
+
+    bool handled = false;
+    do {
+        if (!isPointerType(wd.type))
+            break;
+        const int classPos = wd.value.indexOf(" class ");
+        if (classPos == -1)
+            break;
+        const QString hexAddrS = wd.value.mid(0, classPos);
+        if (m_hexNullPattern.exactMatch(hexAddrS))
+            break;
+        const QString type = stripPointerType(wd.value.mid(classPos + 7));
+        WatchData derefedWd;
+        derefedWd.setType(type);
+        derefedWd.setAddress(hexAddrS);
+        derefedWd.name = QString(QLatin1Char('*'));
+        derefedWd.iname = wd.iname + QLatin1String(".*");
+        derefedWd.source = OwnerDumper | CdbStackFrameContext::ChildrenKnownBit;
+        const CdbDumperHelper::DumpResult dr = m_dumper->dumpType(derefedWd, true, &m_dumperResult, errorMessage);
+        if (dr != CdbDumperHelper::DumpOk)
+            break;
+        fixDumperResult(derefedWd, &m_dumperResult, true);
+        // Insert the pointer item with 1 additional child + its dumper results
+        // Note: formal arguments might already be expanded in the symbol group.
+        WatchData ptrWd = wd;
+        ptrWd.source = OwnerDumper | CdbStackFrameContext::ChildrenKnownBit;
+        ptrWd.setHasChildren(true);
+        ptrWd.setChildrenUnneeded();
+        m_wh->insertData(ptrWd);
+        m_wh->insertBulkData(m_dumperResult);
+        handled = true;
+    } while (false);
+    if (debugCDBWatchHandling)
+        qDebug() << "<expandPointerToDumpable returns " << handled << *errorMessage;
+    return handled;
+}
+
 WatchHandleDumperInserter &WatchHandleDumperInserter::operator=(WatchData &wd)
 {
     if (debugCDBWatchHandling)

src/plugins/debugger/watchhandler.cpp

@@ -257,6 +257,8 @@ QString WatchData::toString() const
 
     if (isChildrenNeeded())
         str << "children=<needed>,";
+    if (source)
+        str << "source=" << source;
     str.flush();
     if (res.endsWith(QLatin1Char(',')))
         res.truncate(res.size() - 1);