From 8266c366a38279975e005355e43cdfdd2d4c1210 Mon Sep 17 00:00:00 2001
From: Friedemann Kleint <Friedemann.Kleint@nokia.com>
Date: Fri, 9 Oct 2009 14:11:05 +0200
Subject: [PATCH] CDB: Make use of the improved debugger expression syntax

CDB can now handle sizeof(Class) and even expressions that
determine the offset of map node values (to some extent), may still
fail with templates). Format expressions accordingly, adapt
cached expressions.
Use gdbQuoteType depending on debugger type only.
Most important, do not evaluate each expression separately before
issuing the call, as ".call" can now do it on its own. Check for syntax
errors there and cache failing types as before.
---
 share/qtcreator/gdbmacros/gdbmacros.cpp      | 13 ++-
 src/plugins/debugger/cdb/cdbdumperhelper.cpp | 80 +++++++-----------
 src/plugins/debugger/cdb/cdbdumperhelper.h   |  9 +--
 src/plugins/debugger/watchutils.cpp          | 85 +++++++++++++++-----
 src/plugins/debugger/watchutils.h            |  6 +-
 5 files changed, 111 insertions(+), 82 deletions(-)

diff --git a/share/qtcreator/gdbmacros/gdbmacros.cpp b/share/qtcreator/gdbmacros/gdbmacros.cpp
index 072a41c6b14..3407b3645c1 100644
--- a/share/qtcreator/gdbmacros/gdbmacros.cpp
+++ b/share/qtcreator/gdbmacros/gdbmacros.cpp
@@ -3561,8 +3561,7 @@ void *watchPoint(int x, int y)
 // Helpers to write out common expression values for CDB
 #ifdef Q_CC_MSVC
 // Offsets of a map node value which looks like
-// "(size_t)&(('QMapNode<QString,QString >'*)0)->value")" in gdb syntax
-
+// "(size_t)&(((QMapNode<int,int> *)0)->value)-0"
 template <class Key, class Value>
         inline QDumper & putQMapNodeOffsetExpression(const char *keyType,
                                                      const char *valueType,
@@ -3570,13 +3569,13 @@ template <class Key, class Value>
 {
     QMapNode<Key, Value> *mn = 0;
     const int valueOffset = (char *)&(mn->value) - (char*)mn;
-    d.put("[\"(size_t)&(('"NS"QMapNode<");
+    d.put("[\"(size_t)&((("NS"QMapNode<");
     d.put(keyType);
     d.put(',');
     d.put(valueType);
     if (valueType[qstrlen(valueType) - 1] == '>')
         d.put(' ');
-    d.put(">'*)0)->value\",\"");
+    d.put("> *)0)->value)-0\",\"");
     d.put(valueOffset);
     d.put("\"]");
     return d;
@@ -3584,7 +3583,7 @@ template <class Key, class Value>
 
 // Helper to write out common expression values for CDB:
 // Offsets of a std::pair for dumping std::map node value which look like
-// "(size_t)&(('std::pair<int const ,unsigned int>'*)0)->second"
+// "(size_t)&(((std::pair<int const ,int> *)0)->second)-0"
 
 template <class Key, class Value>
         inline QDumper & putStdPairValueOffsetExpression(const char *keyType,
@@ -3593,13 +3592,13 @@ template <class Key, class Value>
 {
     std::pair<Key, Value> *p = 0;
     const int valueOffset = (char *)&(p->second) - (char*)p;
-    d.put("[\"(size_t)&(('std::pair<");
+    d.put("[\"(size_t)&(((std::pair<");
     d.put(keyType);
     d.put(" const ,");
     d.put(valueType);
     if (valueType[qstrlen(valueType) - 1] == '>')
         d.put(' ');
-    d.put(">'*)0)->second\",\"");
+    d.put("> *)0)->second)-0\",\"");
     d.put(valueOffset);
     d.put("\"]");
     return  d;
diff --git a/src/plugins/debugger/cdb/cdbdumperhelper.cpp b/src/plugins/debugger/cdb/cdbdumperhelper.cpp
index 9d1d8c9713e..a188d2c1f2b 100644
--- a/src/plugins/debugger/cdb/cdbdumperhelper.cpp
+++ b/src/plugins/debugger/cdb/cdbdumperhelper.cpp
@@ -457,7 +457,7 @@ bool CdbDumperHelper::initKnownTypes(QString *errorMessage)
     QString callCmd;
     QTextStream(&callCmd) << ".call " << m_dumpObjectSymbol << "(1,0,0,0,0,0,0,0)";
     const char *outData;
-    if (!callDumper(callCmd, QByteArray(), &outData, false, errorMessage)) {
+    if (callDumper(callCmd, QByteArray(), &outData, false, errorMessage) != CallOk) {
         return false;
     }
     if (!m_helper.parseQuery(outData, QtDumperHelper::CdbDebugger)) {
@@ -490,8 +490,9 @@ bool CdbDumperHelper::writeToDebuggee(CIDebugDataSpaces *ds, const QByteArray &b
     return true;
 }
 
-bool CdbDumperHelper::callDumper(const QString &callCmd, const QByteArray &inBuffer, const char **outDataPtr,
-                                 bool ignoreAccessViolation, QString *errorMessage)
+CdbDumperHelper::CallResult
+    CdbDumperHelper::callDumper(const QString &callCmd, const QByteArray &inBuffer, const char **outDataPtr,
+                                bool ignoreAccessViolation, QString *errorMessage)
 {
     *outDataPtr = 0;
     CdbExceptionLoggerEventCallback exLogger(LogWarning, m_manager);
@@ -499,10 +500,10 @@ bool CdbDumperHelper::callDumper(const QString &callCmd, const QByteArray &inBuf
     // write input buffer
     if (!inBuffer.isEmpty()) {
         if (!writeToDebuggee(m_cif->debugDataSpaces, inBuffer, m_inBufferAddress, errorMessage))
-            return false;
+            return CallFailed;
     }
     if (!CdbDebugEnginePrivate::executeDebuggerCommand(m_cif->debugControl, callCmd, errorMessage))
-        return false;
+        return CallSyntaxError;
     // Set up call and a temporary breakpoint after it.
     // Try to skip debuggee crash exceptions and dumper exceptions
     // by using 'gN' (go not handled -> pass handling to dumper __try/__catch block)
@@ -514,11 +515,11 @@ bool CdbDumperHelper::callDumper(const QString &callCmd, const QByteArray &inBuf
         if (i)
             goCmd = QLatin1Char('N');
         if (!CdbDebugEnginePrivate::executeDebuggerCommand(m_cif->debugControl, goCmd, errorMessage))
-            return false;
+            return CallFailed;
         HRESULT hr = m_cif->debugControl->WaitForEvent(0, waitTimeOutMS);
         if (FAILED(hr)) {
             *errorMessage = msgComFailed("WaitForEvent", hr);
-            return false;
+            return CallFailed;
         }
         const int newExceptionCount = exLogger.exceptionCount();
         // no new exceptions? -> break
@@ -535,13 +536,13 @@ bool CdbDumperHelper::callDumper(const QString &callCmd, const QByteArray &inBuf
     if (exLogger.exceptionCount()) {
         const QString exMsgs = exLogger.exceptionMessages().join(QString(QLatin1Char(',')));
         *errorMessage = QString::fromLatin1("Exceptions occurred during the dumper call: %1").arg(exMsgs);
-        return false;
+        return CallFailed;
     }
     // Read output
     const HRESULT hr = m_cif->debugDataSpaces->ReadVirtual(m_outBufferAddress, m_buffer, m_outBufferSize, 0);
     if (FAILED(hr)) {
         *errorMessage = msgComFailed("ReadVirtual", hr);
-        return false;
+        return CallFailed;
     }
     // see QDumper implementation
     const char result = m_buffer[0];
@@ -550,16 +551,16 @@ bool CdbDumperHelper::callDumper(const QString &callCmd, const QByteArray &inBuf
         break;
     case '+':
         *errorMessage = QString::fromLatin1("Dumper call '%1' resulted in output overflow.").arg(callCmd);
-        return false;
+        return CallFailed;
     case 'f':
         *errorMessage = QString::fromLatin1("Dumper call '%1' failed.").arg(callCmd);
-        return false;
+        return CallFailed;
     default:
         *errorMessage = QString::fromLatin1("Dumper call '%1' failed ('%2').").arg(callCmd).arg(QLatin1Char(result));
-        return false;
+        return CallFailed;
     }
     *outDataPtr = m_buffer + 1;
-    return true;
+    return CallOk;
 }
 
 static inline QString msgDumpFailed(const WatchData &wd, const QString *why)
@@ -640,12 +641,11 @@ CdbDumperHelper::DumpResult CdbDumperHelper::dumpTypeI(const WatchData &wd, bool
     const DumpExecuteResult der = executeDump(wd, td, dumpChildren, result, errorMessage);
     if (der == DumpExecuteOk)
         return DumpOk;
-    // Cache types that fail due to complicated template size expressions.
-    // Exceptions OTOH might occur when accessing variables that are not
-    // yet initialized in a particular breakpoint. That should be ignored.
-    // Also fail for complex expression that were not cached/replaced by the helper.
-    if (der == DumpExecuteSizeFailed || der == DumpComplexExpressionEncountered)
+    if (der == CallSyntaxError) {
         m_failedTypes.push_back(wd.type);
+        if (dumpDebug)
+            qDebug() << "Caching failing type/expression evaluation failed for " << wd.type;
+       }
     // log error
     *errorMessage = msgDumpFailed(wd, errorMessage);
     m_manager->showDebuggerOutput(LogWarning, *errorMessage);
@@ -661,37 +661,13 @@ CdbDumperHelper::DumpExecuteResult
     QStringList extraParameters;
     // Build parameter list.
     m_helper.evaluationParameters(wd, td, QtDumperHelper::CdbDebugger, &inBuffer, &extraParameters);
-    // If the parameter list contains sizeof-expressions, execute them separately
-    // and replace them by the resulting numbers
-    const QString sizeOfExpr = QLatin1String("sizeof");
-    const QStringList::iterator eend = extraParameters.end();
-    for (QStringList::iterator it = extraParameters.begin() ; it != eend; ++it) {
-        // Strip 'sizeof(X)' to 'X' and query size
-        QString &ep = *it;
-        if (ep.startsWith(sizeOfExpr)) {
-            int size;
-            ep.truncate(ep.lastIndexOf(QLatin1Char(')')));
-            ep.remove(0, ep.indexOf(QLatin1Char('(')) + 1);
-            const bool sizeOk = getTypeSize(ep, &size, errorMessage);
-            if (loadDebug)
-                qDebug() << "Size" << sizeOk << size << ep;
-            if (!sizeOk)
-                return DumpExecuteSizeFailed;
-            ep = QString::number(size);
-            continue;
-        }
-        // We cannot evaluate any other expressions than 'sizeof()' ;-(
-        if (!ep.isEmpty() && !ep.at(0).isDigit()) {
-            *errorMessage = QString::fromLatin1("Unable to evaluate: '%1'").arg(ep);
-            return DumpComplexExpressionEncountered;
-        }
-    }
-    // Execute call
     QString callCmd;
-    QTextStream(&callCmd) << ".call " << m_dumpObjectSymbol
-            << "(2,0," << wd.addr << ','
-            << (dumpChildren ? 1 : 0) << ',' << extraParameters.join(QString(QLatin1Char(','))) << ')';
-    if (loadDebug)
+    QTextStream str(&callCmd);
+    str << ".call " << m_dumpObjectSymbol << "(2,0," << wd.addr << ',' << (dumpChildren ? 1 : 0);
+    foreach(const QString &e, extraParameters)
+        str << ',' << e;
+    str << ')';
+    if (dumpDebug)
         qDebug() << "Query: " << wd.toString() << "\nwith: " << callCmd << '\n';
     const char *outputData;
     // Completely ignore EXCEPTION_ACCESS_VIOLATION crashes in the dumpers.
@@ -700,8 +676,14 @@ CdbDumperHelper::DumpExecuteResult
         *errorMessage = eb.errorString();
         return DumpExecuteCallFailed;
     }
-    if (!callDumper(callCmd, inBuffer, &outputData, true, errorMessage))
+    switch (callDumper(callCmd, inBuffer, &outputData, true, errorMessage)) {
+    case CallFailed:
         return DumpExecuteCallFailed;
+    case CallSyntaxError:
+        return DumpExpressionFailed;
+    case CallOk:
+        break;
+    }
     if (!QtDumperHelper::parseValue(outputData, result)) {
         *errorMessage = QLatin1String("Parsing of value query output failed.");
         return DumpExecuteCallFailed;
diff --git a/src/plugins/debugger/cdb/cdbdumperhelper.h b/src/plugins/debugger/cdb/cdbdumperhelper.h
index ca95fe47793..28bffdba320 100644
--- a/src/plugins/debugger/cdb/cdbdumperhelper.h
+++ b/src/plugins/debugger/cdb/cdbdumperhelper.h
@@ -122,12 +122,11 @@ private:
 
     bool getTypeSize(const QString &typeName, int *size, QString *errorMessage);
     bool runTypeSizeQuery(const QString &typeName, int *size, QString *errorMessage);
-    bool callDumper(const QString &call, const QByteArray &inBuffer, const char **outputPtr,
-                    bool ignoreAccessViolation, QString *errorMessage);
+    enum CallResult { CallOk, CallSyntaxError, CallFailed };
+    CallResult callDumper(const QString &call, const QByteArray &inBuffer, const char **outputPtr,
+                          bool ignoreAccessViolation, QString *errorMessage);
 
-    enum DumpExecuteResult { DumpExecuteOk, DumpExecuteSizeFailed,
-                             DumpComplexExpressionEncountered,
-                             DumpExecuteCallFailed };
+    enum DumpExecuteResult { DumpExecuteOk, DumpExpressionFailed, DumpExecuteCallFailed };
     DumpExecuteResult executeDump(const WatchData &wd,
                                   const QtDumperHelper::TypeData& td, bool dumpChildren,
                                   QList<WatchData> *result, QString *errorMessage);
diff --git a/src/plugins/debugger/watchutils.cpp b/src/plugins/debugger/watchutils.cpp
index 576f29811e5..c004a25652c 100644
--- a/src/plugins/debugger/watchutils.cpp
+++ b/src/plugins/debugger/watchutils.cpp
@@ -366,11 +366,11 @@ GuessChildrenResult guessChildren(const QString &type)
     return HasPossiblyChildren;
 }
 
-QString sizeofTypeExpression(const QString &type)
+QString sizeofTypeExpression(const QString &type, QtDumperHelper::Debugger debugger)
 {
     if (type.endsWith(QLatin1Char('*')))
         return QLatin1String("sizeof(void*)");
-    if (type.endsWith(QLatin1Char('>')))
+    if (debugger != QtDumperHelper::GdbDebugger || type.endsWith(QLatin1Char('>')))
         return QLatin1String("sizeof(") + type + QLatin1Char(')');
     return QLatin1String("sizeof(") + gdbQuoteTypes(type) + QLatin1Char(')');
 }
@@ -838,7 +838,7 @@ QtDumperHelper::TypeData QtDumperHelper::typeData(const QString &typeName) const
 // Format an expression to have the debugger query the
 // size. Use size cache if possible
 QString QtDumperHelper::evaluationSizeofTypeExpression(const QString &typeName,
-                                                       Debugger /* debugger */) const
+                                                       Debugger debugger) const
 {
     // Look up special size types
     const SpecialSizeType st = specialSizeType(typeName);
@@ -851,7 +851,7 @@ QString QtDumperHelper::evaluationSizeofTypeExpression(const QString &typeName,
     if (sit != m_sizeCache.constEnd())
         return QString::number(sit.value());
     // Finally have the debugger evaluate
-    return sizeofTypeExpression(typeName);
+    return sizeofTypeExpression(typeName, debugger);
 }
 
 QtDumperHelper::SpecialSizeType QtDumperHelper::specialSizeType(const QString &typeName) const
@@ -962,24 +962,17 @@ void QtDumperHelper::evaluationParameters(const WatchData &data,
             //qDebug() << "OUTERTYPE: " << outertype << " NODETYPE: " << nodetype
             //    << "QT VERSION" << m_qtVersion << ((4 << 16) + (5 << 8) + 0);
             extraArgs[2] = evaluationSizeofTypeExpression(nodetype, debugger);
-            extraArgs[3] = QLatin1String("(size_t)&(('");
-            extraArgs[3] += nodetype;
-            extraArgs[3] += QLatin1String("'*)0)->value");
+            extraArgs[3] = qMapNodeValueOffsetExpression(nodetype, data.addr, debugger);
         }
         break;
     case QMapNodeType:
         extraArgs[2] = evaluationSizeofTypeExpression(data.type, debugger);
-        extraArgs[3] = QLatin1String("(size_t)&(('");
-        extraArgs[3] += data.type;
-        extraArgs[3] += QLatin1String("'*)0)->value");
+        extraArgs[3] = qMapNodeValueOffsetExpression(data.type, data.addr, debugger);
         break;
     case StdVectorType:
         //qDebug() << "EXTRACT TEMPLATE: " << outertype << inners;
         if (inners.at(0) == QLatin1String("bool")) {
             outertype = QLatin1String("std::vector::bool");
-        } else {
-            //extraArgs[extraArgCount++] = evaluationSizeofTypeExpression(data.type, debugger);
-            //extraArgs[extraArgCount++] = "(size_t)&(('" + data.type + "'*)0)->value";
         }
         break;
     case StdDequeType:
@@ -995,27 +988,40 @@ void QtDumperHelper::evaluationParameters(const WatchData &data,
         extraArgs[2] = zero;
         break;
     case StdMapType: {
-            // We don't want the comparator and the allocator confuse gdb.
-            // But we need the offset of the second item in the value pair.
+            // We need the offset of the second item in the value pair.
             // We read the type of the pair from the allocator argument because
-            // that gets the constness "right" (in the sense that gdb can
+            // that gets the constness "right" (in the sense that gdb/cdb can
             // read it back: "std::allocator<std::pair<Key,Value> >"
             // -> "std::pair<Key,Value>". Different debuggers have varying
             // amounts of terminating blanks...
+            extraArgs[2].clear();
+            extraArgs[3] = zero;
             QString pairType = inners.at(3);
             int bracketPos = pairType.indexOf(QLatin1Char('<'));
             if (bracketPos != -1)
                 pairType.remove(0, bracketPos + 1);
+            // We don't want the comparator and the allocator confuse gdb.
             const QChar closingBracket = QLatin1Char('>');
             bracketPos = pairType.lastIndexOf(closingBracket);
             if (bracketPos != -1)
                 bracketPos = pairType.lastIndexOf(closingBracket, bracketPos - pairType.size() - 1);
             if (bracketPos != -1)
                 pairType.truncate(bracketPos + 1);
-            extraArgs[2] = QLatin1String("(size_t)&(('");
-            extraArgs[2] += pairType;
-            extraArgs[2] += QLatin1String("'*)0)->second");
-            extraArgs[3] = zero;
+            if (debugger == GdbDebugger) {
+                extraArgs[2] = QLatin1String("(size_t)&(('");
+                extraArgs[2] += pairType;
+                extraArgs[2] += QLatin1String("'*)0)->second");
+            } else {
+                // Cdb: The std::pair is usually in scope. Still, this expression
+                // occasionally fails for complex types (std::string).
+                // We need an address as CDB cannot do the 0-trick.
+                // Use data address or try at least cache if missing.
+                const QString address = data.addr.isEmpty() ? QString::fromLatin1("DUMMY_ADDRESS") : data.addr;
+                QString offsetExpr;
+                QTextStream str(&offsetExpr);
+                str << "(size_t)&(((" << pairType << " *)" << address << ")->second)" << '-' << address;
+                extraArgs[2] = lookupCdbDummyAddressExpression(offsetExpr, address);
+            }
         }
         break;
     case StdStringType:
@@ -1068,6 +1074,45 @@ void QtDumperHelper::evaluationParameters(const WatchData &data,
         qDebug() << '\n' << Q_FUNC_INFO << '\n' << data.toString() << "\n-->" << outertype << td.type << extraArgs;
 }
 
+// Return debugger expression to get the offset of a map node.
+QString QtDumperHelper::qMapNodeValueOffsetExpression(const QString &type,
+                                                      const QString &addressIn,
+                                                      Debugger debugger) const
+{
+    switch (debugger) {
+    case GdbDebugger:
+        return QLatin1String("(size_t)&(('") + type + QLatin1String("'*)0)->value");
+    case CdbDebugger: {
+            // Cdb: This will only work if a QMapNode is in scope.
+            // We need an address as CDB cannot do the 0-trick.
+            // Use data address or try at least cache if missing.
+            const QString address = addressIn.isEmpty() ? QString::fromLatin1("DUMMY_ADDRESS") : addressIn;
+            QString offsetExpression;
+            QTextStream(&offsetExpression) << "(size_t)&(((" << type
+                    << " *)" << address << ")->value)-" << address;
+            return lookupCdbDummyAddressExpression(offsetExpression, address);
+        }
+    }
+    return QString();
+}
+
+/* Cdb cannot do tricks like ( "&(std::pair<int,int>*)(0)->second)",
+ * that is, use a null pointer to determine the offset of a member.
+ * It tries to dereference the address at some point and fails with
+ * "memory access error". As a trick, use the address of the watch item
+ * to do this. However, in the expression cache, 0 is still used, so,
+ * for cache lookups,  use '0' as address. */
+QString QtDumperHelper::lookupCdbDummyAddressExpression(const QString &expr,
+                                                        const QString &address) const
+{
+    QString nullExpr = expr;
+    nullExpr.replace(address, QString(QLatin1Char('0')));
+    const QString rc = m_expressionCache.value(nullExpr, expr);
+    if (debug)
+        qDebug() << "lookupCdbDummyAddressExpression" << expr << rc;
+    return rc;
+}
+
 // GdbMi parsing helpers for parsing dumper value results
 
 static bool gdbMiGetIntValue(int *target,
diff --git a/src/plugins/debugger/watchutils.h b/src/plugins/debugger/watchutils.h
index 9c4280b0dfe..7811c98e75c 100644
--- a/src/plugins/debugger/watchutils.h
+++ b/src/plugins/debugger/watchutils.h
@@ -79,7 +79,6 @@ bool isSymbianIntType(const QString &type);
 enum GuessChildrenResult { HasChildren, HasNoChildren, HasPossiblyChildren };
 GuessChildrenResult guessChildren(const QString &type);
 
-QString sizeofTypeExpression(const QString &type);
 QString quoteUnprintableLatin1(const QByteArray &ba);
 
 // Editor tooltip support
@@ -193,6 +192,11 @@ private:
     static Type specialType(QString s);
     QString evaluationSizeofTypeExpression(const QString &typeName, Debugger d) const;
     void parseQueryTypes(const QStringList &l, Debugger debugger);
+    QString qMapNodeValueOffsetExpression(const QString &type,
+                                          const QString &addressIn,
+                                          Debugger debugger) const;
+
+    inline QString lookupCdbDummyAddressExpression(const QString &expr, const QString &address) const;
 
     NameTypeMap m_nameTypeMap;
     SizeCache m_sizeCache;
-- 
GitLab