Debugger: Provider Qt version externally to bridges

Extracting within the bridges is expensive.

Change-Id: Icf69db4b112230cc23e331abc0b3eb0de1323f46
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
Reviewed-by: Christian Stenger <christian.stenger@qt.io>
Reviewed-by: David Schulz <david.schulz@qt.io>

9 files changed, 89 insertions, 182 deletions

diff --git a/share/qtcreator/debugger/dumper.py b/share/qtcreator/debugger/dumper.py
index 488f1e362e..ec7b83d331 100644
--- a/share/qtcreator/debugger/dumper.py
+++ b/share/qtcreator/debugger/dumper.py
@@ -172,9 +172,11 @@ class DumperBase():
         self.qtCustomEventFunc = 0
         self.qtCustomEventPltFunc = 0
         self.qtPropertyFunc = 0
-        self.fallbackQtVersion = 0x60200
+        self.qtversion = None
+        self.qtns = None
         self.passExceptions = False
         self.isTesting = False
+        self.qtLoaded = False
 
         self.isBigEndian = False
         self.packCode = '<'
@@ -231,6 +233,8 @@ class DumperBase():
         self.useTimeStamps = int(args.get('timestamps', '0'))
         self.partialVariable = args.get('partialvar', '')
         self.uninitialized = args.get('uninitialized', [])
+        self.qtversion = args.get('qtversion', 0x060602)
+        self.qtnamespace = args.get('qtnamespace', '')
         self.uninitialized = list(map(lambda x: self.hexdecode(x), self.uninitialized))
         #self.warn('NAMESPACE: "%s"' % self.qtNamespace())
         #self.warn('EXPANDED INAMES: %s' % self.expandedINames)
@@ -252,9 +256,11 @@ class DumperBase():
         args['partialvar'] = ''
         self.fetchVariables(args)
 
-    def setFallbackQtVersion(self, args):
-        version = int(args.get('version', self.fallbackQtVersion))
-        self.fallbackQtVersion = version
+    def qtVersion(self):
+        return self.qtversion
+
+    def qtNamespace(self):
+        return self.qtnamespace
 
     def resetPerStepCaches(self):
         self.perStepCache = {}
@@ -1566,7 +1572,16 @@ class DumperBase():
         if address is not None:
             self.put('origaddr="0x%x",' % address)
 
+    def wantQObjectNames(self):
+        return self.showQObjectNames and self.qtLoaded
+
+    def fetchInternalFunctions(self):
+        # Overrridden
+        pass
+
     def putQObjectNameValue(self, value):
+        self.fetchInternalFunctions()
+
         try:
             # dd = value['d_ptr']['d'] is just behind the vtable.
             (vtable, dd) = self.split('pp', value)
@@ -3022,7 +3037,7 @@ typename))
         self.putExpandable()
         self.putEmptyValue()
         #self.warn('STRUCT GUTS: %s  ADDRESS: 0x%x ' % (value.name, value.address()))
-        if self.showQObjectNames:
+        if self.wantQObjectNames():
             #with self.timer(self.currentIName):
             self.putQObjectNameValue(value)
         if self.isExpanded():
@@ -3030,7 +3045,7 @@ typename))
                 self.putField('sortable', 1)
             with Children(self, 1, childType=None):
                 self.putFields(value)
-                if self.showQObjectNames:
+                if self.wantQObjectNames():
                     self.tryPutQObjectGuts(value)
 
     def symbolAddress(self, symbolName):
diff --git a/share/qtcreator/debugger/gdbbridge.py b/share/qtcreator/debugger/gdbbridge.py
index 744d246142..941c9b8312 100644
--- a/share/qtcreator/debugger/gdbbridge.py
+++ b/share/qtcreator/debugger/gdbbridge.py
@@ -852,37 +852,6 @@ class Dumper(DumperBase):
         except:
             return '0x%x' % address
 
-    def qtVersionString(self):
-        try:
-            return str(gdb.lookup_symbol('qVersion')[0].value()())
-        except:
-            pass
-        try:
-            ns = self.qtNamespace()
-            return str(gdb.parse_and_eval("((const char*(*)())'%sqVersion')()" % ns))
-        except:
-            pass
-        return None
-
-    def qtVersion(self):
-        try:
-            # Only available with Qt 5.3+
-            qtversion = int(str(gdb.parse_and_eval('((void**)&qtHookData)[2]')), 16)
-            self.qtVersion = lambda: qtversion
-            return qtversion
-        except:
-            pass
-
-        try:
-            version = self.qtVersionString()
-            (major, minor, patch) = version[version.find('"') + 1:version.rfind('"')].split('.')
-            qtversion = 0x10000 * int(major) + 0x100 * int(minor) + int(patch)
-            self.qtVersion = lambda: qtversion
-            return qtversion
-        except:
-            # Use fallback until we have a better answer.
-            return self.fallbackQtVersion
-
     def createSpecialBreakpoints(self, args):
         self.specialBreakpoints = []
 
@@ -1008,10 +977,6 @@ class Dumper(DumperBase):
         for obj in gdb.objfiles():
             self.importPlainDumpersForObj(obj)
 
-    def qtNamespace(self):
-        # This function is replaced by handleQtCoreLoaded()
-        return ''
-
     def findSymbol(self, symbolName):
         try:
             return int(gdb.parse_and_eval("(size_t)&'%s'" % symbolName))
@@ -1044,41 +1009,38 @@ class Dumper(DumperBase):
             pass
 
     def handleQtCoreLoaded(self, objfile):
-        fd, tmppath = tempfile.mkstemp()
-        os.close(fd)
-        cmd = 'maint print msymbols -objfile "%s" -- %s' % (objfile.filename, tmppath)
-        symbols = gdb.execute(cmd, to_string=True)
-        ns = ''
-        with open(tmppath) as f:
-            ns1re = re.compile(r'_ZN?(\d*)(\w*)L17msgHandlerGrabbedE? ')
-            ns2re = re.compile(r'_ZN?(\d*)(\w*)L17currentThreadDataE? ')
-            for line in f:
-                if 'msgHandlerGrabbed ' in line:
-                    # [11] b 0x7ffff683c000 _ZN4MynsL17msgHandlerGrabbedE
-                    # section .tbss Myns::msgHandlerGrabbed  qlogging.cpp
-                    ns = ns1re.split(line)[2]
-                    if len(ns):
-                        ns += '::'
-                    break
-                if 'currentThreadData ' in line:
-                    # [ 0] b 0x7ffff67d3000 _ZN2UUL17currentThreadDataE
-                    # section .tbss  UU::currentThreadData qthread_unix.cpp\\n
-                    ns = ns2re.split(line)[2]
-                    if len(ns):
-                        ns += '::'
-                    break
-        os.remove(tmppath)
-
+        self.qtLoaded = True
+        # FIXME: Namespace auto-detection. Is it worth the price?
+        #       fd, tmppath = tempfile.mkstemp()
+        #       os.close(fd)
+        #       cmd = 'maint print msymbols -objfile "%s" -- %s' % (objfile.filename, tmppath)
+        #       symbols = gdb.execute(cmd, to_string=True)
+        #       ns = ''
+        #       with open(tmppath) as f:
+        #           ns1re = re.compile(r'_ZN?(\d*)(\w*)L17msgHandlerGrabbedE? ')
+        #           ns2re = re.compile(r'_ZN?(\d*)(\w*)L17currentThreadDataE? ')
+        #           for line in f:
+        #               if 'msgHandlerGrabbed ' in line:
+        #                   # [11] b 0x7ffff683c000 _ZN4MynsL17msgHandlerGrabbedE
+        #                   # section .tbss Myns::msgHandlerGrabbed  qlogging.cpp
+        #                   ns = ns1re.split(line)[2]
+        #                   if len(ns):
+        #                       ns += '::'
+        #                   break
+        #               if 'currentThreadData ' in line:
+        #                   # [ 0] b 0x7ffff67d3000 _ZN2UUL17currentThreadDataE
+        #                   # section .tbss  UU::currentThreadData qthread_unix.cpp\\n
+        #                   ns = ns2re.split(line)[2]
+        #                   if len(ns):
+        #                       ns += '::'
+        #                   break
+        #       os.remove(tmppath)
+
+    def fetchInternalFunctions(self):
+        ns = self.qtNamespace()
         lenns = len(ns)
         strns = ('%d%s' % (lenns - 2, ns[:lenns - 2])) if lenns else ''
-
         if lenns:
-            # This might be wrong, but we can't do better: We found
-            # a libQt5Core and could not extract a namespace.
-            # The best guess is that there isn't any.
-            self.qtNamespaceToReport = ns
-            self.qtNamespace = lambda: ns
-
             sym = '_ZN%s7QObject11customEventEPNS_6QEventE' % strns
         else:
             sym = '_ZN7QObject11customEventEP6QEvent'
@@ -1091,6 +1053,8 @@ class Dumper(DumperBase):
         if not self.isWindowsTarget(): # prevent calling the property function on windows
             self.qtPropertyFunc = self.findSymbol(sym)
 
+        self.fetchInternalFunctions = lambda: None
+
     def assignValue(self, args):
         type_name = self.hexdecode(args['type'])
         expr = self.hexdecode(args['expr'])
diff --git a/share/qtcreator/debugger/lldbbridge.py b/share/qtcreator/debugger/lldbbridge.py
index 6b08ac3d32..d8c0ab6ca6 100644
--- a/share/qtcreator/debugger/lldbbridge.py
+++ b/share/qtcreator/debugger/lldbbridge.py
@@ -768,80 +768,7 @@ class Dumper(DumperBase):
             symbol = funcs[0].GetSymbol()
             self.qtPropertyFunc = symbol.GetStartAddress().GetLoadAddress(self.target)
 
-    def fetchQtVersionAndNamespace(self):
-        for func in self.target.FindFunctions('qVersion'):
-            name = func.GetSymbol().GetName()
-            if name == None:
-                continue
-            if name.endswith('()'):
-                name = name[:-2]
-            if name.count(':') > 2:
-                continue
-
-            qtNamespace = name[:name.find('qVersion')]
-            self.qtNamespace = lambda: qtNamespace
-
-            options = lldb.SBExpressionOptions()
-            res = self.target.EvaluateExpression(name + '()', options)
-
-            if not res.IsValid() or not res.GetType().IsPointerType():
-                exp = '((const char*())%s)()' % name
-                res = self.target.EvaluateExpression(exp, options)
-
-            if not res.IsValid() or not res.GetType().IsPointerType():
-                exp = '((const char*())_Z8qVersionv)()'
-                res = self.target.EvaluateExpression(exp, options)
-
-            if not res.IsValid() or not res.GetType().IsPointerType():
-                continue
-
-            version = str(res)
-            if version.count('.') != 2:
-                continue
-
-            version.replace("'", '"')  # Both seem possible
-            version = version[version.find('"') + 1:version.rfind('"')]
-
-            (major, minor, patch) = version.split('.')
-            qtVersion = 0x10000 * int(major) + 0x100 * int(minor) + int(patch)
-            self.qtVersion = lambda: qtVersion
-
-            return (qtNamespace, qtVersion)
-
-        try:
-            versionValue = self.target.EvaluateExpression('qtHookData[2]').GetNonSyntheticValue()
-            if versionValue.IsValid():
-                return ('', versionValue.unsigned)
-        except:
-            pass
-
-        return ('', self.fallbackQtVersion)
-
-    def qtVersionAndNamespace(self):
-        qtVersionAndNamespace = None
-        try:
-            qtVersionAndNamespace = self.fetchQtVersionAndNamespace()
-            self.report("Detected Qt Version: 0x%0x (namespace='%s')" %
-                        (qtVersionAndNamespace[1], qtVersionAndNamespace[0] or "no namespace"))
-        except Exception as e:
-            DumperBase.warn('[lldb] Error detecting Qt version: %s' % e)
-
-        try:
-            self.fetchInternalFunctions()
-            self.report('Found function QObject::property: 0x%0x' % self.qtPropertyFunc)
-            self.report('Found function QObject::customEvent: 0x%0x' % self.qtCustomEventFunc)
-        except Exception as e:
-            DumperBase.warn('[lldb] Error fetching internal Qt functions: %s' % e)
-
-        # Cache version information by overriding this function.
-        self.qtVersionAndNamespace = lambda: qtVersionAndNamespace
-        return qtVersionAndNamespace
-
-    def qtNamespace(self):
-        return self.qtVersionAndNamespace()[0]
-
-    def qtVersion(self):
-        return self.qtVersionAndNamespace()[1]
+        self.fetchInternalFunctions = lambda: None
 
     def handleCommand(self, command):
         result = lldb.SBCommandReturnObject()
@@ -1360,6 +1287,9 @@ class Dumper(DumperBase):
 
         self.setVariableFetchingOptions(args)
 
+        self.qtLoaded = True # FIXME: Do that elsewhere
+
+
         # Reset certain caches whenever a step over / into / continue
         # happens.
         # FIXME: Caches are currently also cleared if currently
diff --git a/src/plugins/debugger/cdb/cdbengine.cpp b/src/plugins/debugger/cdb/cdbengine.cpp
index 83c5a9459b..24386788db 100644
--- a/src/plugins/debugger/cdb/cdbengine.cpp
+++ b/src/plugins/debugger/cdb/cdbengine.cpp
@@ -1042,6 +1042,8 @@ void CdbEngine::doUpdateLocals(const UpdateParameters &updateParameters)
         cmd.arg("partialvar", updateParameters.partialVariable);
         cmd.arg("qobjectnames", s.showQObjectNames());
         cmd.arg("timestamps", s.logTimeStamps());
+        cmd.arg("qtversion", runParameters().qtVersion);
+        cmd.arg("qtnamespace", runParameters().qtNamespace);
 
         StackFrame frame = stackHandler()->currentFrame();
         cmd.arg("context", frame.context);
@@ -2809,10 +2811,6 @@ void CdbEngine::setupScripting(const DebuggerResponse &response)
             runCommand({command, ScriptCommand});
     }
 
-    DebuggerCommand cmd0("theDumper.setFallbackQtVersion", ScriptCommand);
-    cmd0.arg("version", runParameters().fallbackQtVersion);
-    runCommand(cmd0);
-
     runCommand({"theDumper.loadDumpers(None)", ScriptCommand,
                 [this](const DebuggerResponse &response) {
                     watchHandler()->addDumpers(response.data["result"]["dumpers"]);
diff --git a/src/plugins/debugger/debuggerengine.h b/src/plugins/debugger/debuggerengine.h
index e82a386311..b5e8a3cba8 100644
--- a/src/plugins/debugger/debuggerengine.h
+++ b/src/plugins/debugger/debuggerengine.h
@@ -188,7 +188,8 @@ public:
 
     QStringList validationErrors;
 
-    int fallbackQtVersion = 0x50200;
+    int qtVersion = 0;
+    QString qtNamespace;
 
     // Common debugger constants.
     Utils::FilePath peripheralDescriptionFile;
diff --git a/src/plugins/debugger/debuggerruncontrol.cpp b/src/plugins/debugger/debuggerruncontrol.cpp
index 503bad4bc2..522191e25b 100644
--- a/src/plugins/debugger/debuggerruncontrol.cpp
+++ b/src/plugins/debugger/debuggerruncontrol.cpp
@@ -898,7 +898,7 @@ DebuggerRunTool::DebuggerRunTool(RunControl *runControl, AllowTerminal allowTerm
 
     if (QtSupport::QtVersion *baseQtVersion = QtSupport::QtKitAspect::qtVersion(kit)) {
         const QVersionNumber qtVersion = baseQtVersion->qtVersion();
-        m_runParameters.fallbackQtVersion = 0x10000 * qtVersion.majorVersion()
+        m_runParameters.qtVersion = 0x10000 * qtVersion.majorVersion()
                                             + 0x100 * qtVersion.minorVersion()
                                             + qtVersion.microVersion();
     }
diff --git a/src/plugins/debugger/gdb/gdbengine.cpp b/src/plugins/debugger/gdb/gdbengine.cpp
index e4873de775..3abaea75a4 100644
--- a/src/plugins/debugger/gdb/gdbengine.cpp
+++ b/src/plugins/debugger/gdb/gdbengine.cpp
@@ -4054,10 +4054,6 @@ void GdbEngine::handleGdbStarted()
     if (!commands.isEmpty())
         runCommand({commands});
 
-    DebuggerCommand cmd1("setFallbackQtVersion");
-    cmd1.arg("version", rp.fallbackQtVersion);
-    runCommand(cmd1);
-
     runCommand({"loadDumpers", CB(handlePythonSetup)});
 
     // Reload peripheral register description.
@@ -5133,6 +5129,8 @@ void GdbEngine::doUpdateLocals(const UpdateParameters &params)
     cmd.arg("dyntype", s.useDynamicType());
     cmd.arg("qobjectnames", s.showQObjectNames());
     cmd.arg("timestamps", s.logTimeStamps());
+    cmd.arg("qtversion", runParameters().qtVersion);
+    cmd.arg("qtnamespace", runParameters().qtNamespace);
 
     StackFrame frame = stackHandler()->currentFrame();
     cmd.arg("context", frame.context);
diff --git a/src/plugins/debugger/lldb/lldbengine.cpp b/src/plugins/debugger/lldb/lldbengine.cpp
index 3d21f82ca2..a2b8c4eb3d 100644
--- a/src/plugins/debugger/lldb/lldbengine.cpp
+++ b/src/plugins/debugger/lldb/lldbengine.cpp
@@ -346,10 +346,6 @@ void LldbEngine::handleLldbStarted()
 
     cmd2.flags = Silent;
     runCommand(cmd2);
-
-    DebuggerCommand cmd0("setFallbackQtVersion");
-    cmd0.arg("version", rp.fallbackQtVersion);
-    runCommand(cmd0);
 }
 
 void LldbEngine::runEngine()
@@ -767,6 +763,8 @@ void LldbEngine::doUpdateLocals(const UpdateParameters &params)
     cmd.arg("partialvar", params.partialVariable);
     cmd.arg("qobjectnames", s.showQObjectNames());
     cmd.arg("timestamps", s.logTimeStamps());
+    cmd.arg("qtversion", runParameters().qtVersion);
+    cmd.arg("qtnamespace", runParameters().qtNamespace);
 
     StackFrame frame = stackHandler()->currentFrame();
     cmd.arg("context", frame.context);
diff --git a/tests/auto/debugger/tst_dumpers.cpp b/tests/auto/debugger/tst_dumpers.cpp
index e5763e847e..4385f84084 100644
--- a/tests/auto/debugger/tst_dumpers.cpp
+++ b/tests/auto/debugger/tst_dumpers.cpp
@@ -1200,7 +1200,28 @@ void tst_Dumpers::initTestCase()
     if (qEnvironmentVariableIntValue("QTC_USE_CMAKE_FOR_TEST"))
         m_buildSystem = BuildSystem::CMake;
 
+    QProcess qmake;
+    qmake.start(m_qmakeBinary, {"--version"});
+    QVERIFY(qmake.waitForFinished());
+    QByteArray output = qmake.readAllStandardOutput();
+    QByteArray error = qmake.readAllStandardError();
+    int pos0 = output.indexOf("Qt version");
+    if (pos0 == -1) {
+        qCDebug(lcDumpers).noquote() << "Output: " << output;
+        qCDebug(lcDumpers).noquote() << "Error: " << error;
+        QVERIFY(false);
+    }
+    pos0 += 11;
+    int pos1 = output.indexOf('.', pos0 + 1);
+    int major = output.mid(pos0, pos1++ - pos0).toInt();
+    int pos2 = output.indexOf('.', pos1 + 1);
+    int minor = output.mid(pos1, pos2++ - pos1).toInt();
+    int pos3 = output.indexOf(' ', pos2 + 1);
+    int patch = output.mid(pos2, pos3++ - pos2).toInt();
+    m_qtVersion = 0x10000 * major + 0x100 * minor + patch;
+
     qCDebug(lcDumpers) << "QMake              : " << m_qmakeBinary;
+    qCDebug(lcDumpers) << "Qt Version         : " << m_qtVersion;
     qCDebug(lcDumpers) << "Use CMake          : " << (m_buildSystem == BuildSystem::CMake) << int(m_buildSystem);
 
     m_useGLibCxxDebug = qgetenv("QTC_USE_GLIBCXXDEBUG_FOR_TEST").toInt();
@@ -1381,27 +1402,6 @@ void tst_Dumpers::dumper()
     QByteArray error;
 
     if (data.neededQtVersion.isRestricted) {
-        QProcess qmake;
-        qmake.setWorkingDirectory(t->buildPath);
-        qmake.start(m_qmakeBinary, {"--version"});
-        QVERIFY(qmake.waitForFinished());
-        output = qmake.readAllStandardOutput();
-        error = qmake.readAllStandardError();
-        int pos0 = output.indexOf("Qt version");
-        if (pos0 == -1) {
-            qCDebug(lcDumpers).noquote() << "Output: " << output;
-            qCDebug(lcDumpers).noquote() << "Error: " << error;
-            QVERIFY(false);
-        }
-        pos0 += 11;
-        int pos1 = output.indexOf('.', pos0 + 1);
-        int major = output.mid(pos0, pos1++ - pos0).toInt();
-        int pos2 = output.indexOf('.', pos1 + 1);
-        int minor = output.mid(pos1, pos2++ - pos1).toInt();
-        int pos3 = output.indexOf(' ', pos2 + 1);
-        int patch = output.mid(pos2, pos3++ - pos2).toInt();
-        m_qtVersion = 0x10000 * major + 0x100 * minor + patch;
-
         if (data.neededQtVersion.min > m_qtVersion)
             MSKIP_SINGLE(QByteArray("Need minimum Qt version "
                 + QByteArray::number(data.neededQtVersion.min, 16)));
@@ -1793,6 +1793,7 @@ void tst_Dumpers::dumper()
                 "python theDumper.fetchVariables({" + dumperOptions +
                     "'token':2,'fancy':1,'forcens':1,"
                     "'autoderef':1,'dyntype':1,'passexceptions':1,"
+                    "'qtversion':" + QString::number(m_qtVersion) + ",'qtnamespace':'',"
                     "'testing':1,'qobjectnames':1,"
                     "'expanded':{" + expandedq + "}})\n";
 
@@ -1816,6 +1817,7 @@ void tst_Dumpers::dumper()
                 "'token':2,'fancy':1,'forcens':1,"
                 "'autoderef':1,'dyntype':1,'passexceptions':0,"
                 "'testing':1,'qobjectnames':1,"
+                "'qtversion':" + QString::number(m_qtVersion) + ",'qtnamespace':'',"
                 "'expanded':{" + expandedq + "}})\n"
                 "q\n";
     } else if (m_debuggerEngine == LldbEngine) {
@@ -1840,6 +1842,7 @@ void tst_Dumpers::dumper()
                     "'fancy':1,'forcens':1,"
                     "'autoderef':1,'dyntype':1,'passexceptions':1,"
                     "'testing':1,'qobjectnames':1,"
+                    "'qtversion':" + QString::number(m_qtVersion) + ",'qtnamespace':'',"
                     "'expanded':{" + expandedq + "}})\n"
                "quit\n";