Re-land LLDB dap module symbol tables (#155021)

Re-land the symbol table feature in lldb-dap after it was
[reverted](2b8e806)
because of a crash in the `aarch64` tests, which was caused by
dereferencing `SBSymbol::GetName` which might return `nullptr` for an
invalid symbol.

This patch reapplies the original commits and adds the missing null
check.

Also adding `skipIfWindows` for the module symbols tests, since LLDB
doesn't recognize the symbols from a.out there.

Author: eronnen

lldb/include/lldb/API/SBSymbol.h

@@ -85,6 +85,12 @@ class LLDB_API SBSymbol {
 
   SymbolType GetType();
 
+  /// Get the ID of this symbol, usually the original symbol table index.
+  ///
+  /// \returns
+  ///   Returns the ID of this symbol.
+  uint32_t GetID();
+
   bool operator==(const lldb::SBSymbol &rhs) const;
 
   bool operator!=(const lldb::SBSymbol &rhs) const;
@@ -99,6 +105,15 @@ class LLDB_API SBSymbol {
   // other than the actual symbol table itself in the object file.
   bool IsSynthetic();
 
+  /// Returns true if the symbol is a debug symbol.
+  bool IsDebug();
+
+  /// Get the string representation of a symbol type.
+  static const char *GetTypeAsString(lldb::SymbolType symbol_type);
+
+  /// Get the symbol type from a string representation.
+  static lldb::SymbolType GetTypeFromString(const char *str);
+
 protected:
   lldb_private::Symbol *get();
 

lldb/include/lldb/API/SBTarget.h

@@ -324,6 +324,16 @@ class LLDB_API SBTarget {
 
   lldb::SBModule FindModule(const lldb::SBFileSpec &file_spec);
 
+  /// Find a module with the given module specification.
+  ///
+  /// \param[in] module_spec
+  ///     A lldb::SBModuleSpec object that contains module specification.
+  ///
+  /// \return
+  ///     A lldb::SBModule object that represents the found module, or an
+  ///     invalid SBModule object if no module was found.
+  lldb::SBModule FindModule(const lldb::SBModuleSpec &module_spec);
+
   /// Find compile units related to *this target and passed source
   /// file.
   ///

lldb/include/lldb/Symbol/Symbol.h

@@ -15,6 +15,7 @@
 #include "lldb/Symbol/SymbolContextScope.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/UserID.h"
+#include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-private.h"
 #include "llvm/Support/JSON.h"
 
@@ -301,6 +302,10 @@ class Symbol : public SymbolContextScope {
 
   bool operator==(const Symbol &rhs) const;
 
+  static const char *GetTypeAsString(lldb::SymbolType symbol_type);
+
+  static lldb::SymbolType GetTypeFromString(const char *str);
+
 protected:
   // This is the internal guts of ResolveReExportedSymbol, it assumes
   // reexport_name is not null, and that module_spec is valid.  We track the

lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py

@@ -1299,6 +1299,25 @@ def request_modules(
             {"command": "modules", "type": "request", "arguments": args_dict}
         )
 
+    def request_moduleSymbols(
+        self,
+        moduleId: str = "",
+        moduleName: str = "",
+        startIndex: int = 0,
+        count: int = 0,
+    ):
+        command_dict = {
+            "command": "__lldb_moduleSymbols",
+            "type": "request",
+            "arguments": {
+                "moduleId": moduleId,
+                "moduleName": moduleName,
+                "startIndex": startIndex,
+                "count": count,
+            },
+        }
+        return self._send_recv(command_dict)
+
     def request_stackTrace(
         self, threadId=None, startFrame=None, levels=None, format=None, dump=False
     ):

lldb/source/API/SBSymbol.cpp

@@ -193,6 +193,14 @@ SymbolType SBSymbol::GetType() {
   return eSymbolTypeInvalid;
 }
 
+uint32_t SBSymbol::GetID() {
+  LLDB_INSTRUMENT_VA(this);
+
+  if (m_opaque_ptr)
+    return m_opaque_ptr->GetID();
+  return 0;
+}
+
 bool SBSymbol::IsExternal() {
   LLDB_INSTRUMENT_VA(this);
 
@@ -208,3 +216,23 @@ bool SBSymbol::IsSynthetic() {
     return m_opaque_ptr->IsSynthetic();
   return false;
 }
+
+bool SBSymbol::IsDebug() {
+  LLDB_INSTRUMENT_VA(this);
+
+  if (m_opaque_ptr)
+    return m_opaque_ptr->IsDebug();
+  return false;
+}
+
+const char *SBSymbol::GetTypeAsString(lldb::SymbolType symbol_type) {
+  LLDB_INSTRUMENT_VA(symbol_type);
+
+  return Symbol::GetTypeAsString(symbol_type);
+}
+
+lldb::SymbolType SBSymbol::GetTypeFromString(const char *str) {
+  LLDB_INSTRUMENT_VA(str);
+
+  return Symbol::GetTypeFromString(str);
+}

lldb/source/API/SBTarget.cpp

@@ -1570,6 +1570,18 @@ SBModule SBTarget::FindModule(const SBFileSpec &sb_file_spec) {
   return sb_module;
 }
 
+SBModule SBTarget::FindModule(const SBModuleSpec &sb_module_spec) {
+  LLDB_INSTRUMENT_VA(this, sb_module_spec);
+
+  SBModule sb_module;
+  if (TargetSP target_sp = GetSP(); target_sp && sb_module_spec.IsValid()) {
+    // The module list is thread safe, no need to lock.
+    sb_module.SetSP(
+        target_sp->GetImages().FindFirstModule(*sb_module_spec.m_opaque_up));
+  }
+  return sb_module;
+}
+
 SBSymbolContextList SBTarget::FindCompileUnits(const SBFileSpec &sb_file_spec) {
   LLDB_INSTRUMENT_VA(this, sb_file_spec);
 

lldb/source/Symbol/Symbol.cpp

@@ -392,45 +392,8 @@ bool Symbol::Compare(ConstString name, SymbolType type) const {
   return false;
 }
 
-#define ENUM_TO_CSTRING(x)                                                     \
-  case eSymbolType##x:                                                         \
-    return #x;
-
 const char *Symbol::GetTypeAsString() const {
-  switch (m_type) {
-    ENUM_TO_CSTRING(Invalid);
-    ENUM_TO_CSTRING(Absolute);
-    ENUM_TO_CSTRING(Code);
-    ENUM_TO_CSTRING(Resolver);
-    ENUM_TO_CSTRING(Data);
-    ENUM_TO_CSTRING(Trampoline);
-    ENUM_TO_CSTRING(Runtime);
-    ENUM_TO_CSTRING(Exception);
-    ENUM_TO_CSTRING(SourceFile);
-    ENUM_TO_CSTRING(HeaderFile);
-    ENUM_TO_CSTRING(ObjectFile);
-    ENUM_TO_CSTRING(CommonBlock);
-    ENUM_TO_CSTRING(Block);
-    ENUM_TO_CSTRING(Local);
-    ENUM_TO_CSTRING(Param);
-    ENUM_TO_CSTRING(Variable);
-    ENUM_TO_CSTRING(VariableType);
-    ENUM_TO_CSTRING(LineEntry);
-    ENUM_TO_CSTRING(LineHeader);
-    ENUM_TO_CSTRING(ScopeBegin);
-    ENUM_TO_CSTRING(ScopeEnd);
-    ENUM_TO_CSTRING(Additional);
-    ENUM_TO_CSTRING(Compiler);
-    ENUM_TO_CSTRING(Instrumentation);
-    ENUM_TO_CSTRING(Undefined);
-    ENUM_TO_CSTRING(ObjCClass);
-    ENUM_TO_CSTRING(ObjCMetaClass);
-    ENUM_TO_CSTRING(ObjCIVar);
-    ENUM_TO_CSTRING(ReExported);
-  default:
-    break;
-  }
-  return "<unknown SymbolType>";
+  return GetTypeAsString(static_cast<lldb::SymbolType>(m_type));
 }
 
 void Symbol::CalculateSymbolContext(SymbolContext *sc) {
@@ -774,6 +737,79 @@ bool Symbol::operator==(const Symbol &rhs) const {
   return true;
 }
 
+#define ENUM_TO_CSTRING(x)                                                     \
+  case eSymbolType##x:                                                         \
+    return #x;
+
+const char *Symbol::GetTypeAsString(lldb::SymbolType symbol_type) {
+  switch (symbol_type) {
+    ENUM_TO_CSTRING(Invalid);
+    ENUM_TO_CSTRING(Absolute);
+    ENUM_TO_CSTRING(Code);
+    ENUM_TO_CSTRING(Resolver);
+    ENUM_TO_CSTRING(Data);
+    ENUM_TO_CSTRING(Trampoline);
+    ENUM_TO_CSTRING(Runtime);
+    ENUM_TO_CSTRING(Exception);
+    ENUM_TO_CSTRING(SourceFile);
+    ENUM_TO_CSTRING(HeaderFile);
+    ENUM_TO_CSTRING(ObjectFile);
+    ENUM_TO_CSTRING(CommonBlock);
+    ENUM_TO_CSTRING(Block);
+    ENUM_TO_CSTRING(Local);
+    ENUM_TO_CSTRING(Param);
+    ENUM_TO_CSTRING(Variable);
+    ENUM_TO_CSTRING(VariableType);
+    ENUM_TO_CSTRING(LineEntry);
+    ENUM_TO_CSTRING(LineHeader);
+    ENUM_TO_CSTRING(ScopeBegin);
+    ENUM_TO_CSTRING(ScopeEnd);
+    ENUM_TO_CSTRING(Additional);
+    ENUM_TO_CSTRING(Compiler);
+    ENUM_TO_CSTRING(Instrumentation);
+    ENUM_TO_CSTRING(Undefined);
+    ENUM_TO_CSTRING(ObjCClass);
+    ENUM_TO_CSTRING(ObjCMetaClass);
+    ENUM_TO_CSTRING(ObjCIVar);
+    ENUM_TO_CSTRING(ReExported);
+  }
+  return "<unknown SymbolType>";
+}
+
+lldb::SymbolType Symbol::GetTypeFromString(const char *str) {
+  std::string str_lower = llvm::StringRef(str).lower();
+  return llvm::StringSwitch<lldb::SymbolType>(str_lower)
+      .Case("absolute", eSymbolTypeAbsolute)
+      .Case("code", eSymbolTypeCode)
+      .Case("resolver", eSymbolTypeResolver)
+      .Case("data", eSymbolTypeData)
+      .Case("trampoline", eSymbolTypeTrampoline)
+      .Case("runtime", eSymbolTypeRuntime)
+      .Case("exception", eSymbolTypeException)
+      .Case("sourcefile", eSymbolTypeSourceFile)
+      .Case("headerfile", eSymbolTypeHeaderFile)
+      .Case("objectfile", eSymbolTypeObjectFile)
+      .Case("commonblock", eSymbolTypeCommonBlock)
+      .Case("block", eSymbolTypeBlock)
+      .Case("local", eSymbolTypeLocal)
+      .Case("param", eSymbolTypeParam)
+      .Case("variable", eSymbolTypeVariable)
+      .Case("variableType", eSymbolTypeVariableType)
+      .Case("lineentry", eSymbolTypeLineEntry)
+      .Case("lineheader", eSymbolTypeLineHeader)
+      .Case("scopebegin", eSymbolTypeScopeBegin)
+      .Case("scopeend", eSymbolTypeScopeEnd)
+      .Case("additional,", eSymbolTypeAdditional)
+      .Case("compiler", eSymbolTypeCompiler)
+      .Case("instrumentation", eSymbolTypeInstrumentation)
+      .Case("undefined", eSymbolTypeUndefined)
+      .Case("objcclass", eSymbolTypeObjCClass)
+      .Case("objcmetaclass", eSymbolTypeObjCMetaClass)
+      .Case("objcivar", eSymbolTypeObjCIVar)
+      .Case("reexported", eSymbolTypeReExported)
+      .Default(eSymbolTypeInvalid);
+}
+
 namespace llvm {
 namespace json {
 
@@ -804,36 +840,8 @@ bool fromJSON(const llvm::json::Value &value, lldb_private::JSONSymbol &symbol,
 bool fromJSON(const llvm::json::Value &value, lldb::SymbolType &type,
               llvm::json::Path path) {
   if (auto str = value.getAsString()) {
-    type = llvm::StringSwitch<lldb::SymbolType>(*str)
-               .Case("absolute", eSymbolTypeAbsolute)
-               .Case("code", eSymbolTypeCode)
-               .Case("resolver", eSymbolTypeResolver)
-               .Case("data", eSymbolTypeData)
-               .Case("trampoline", eSymbolTypeTrampoline)
-               .Case("runtime", eSymbolTypeRuntime)
-               .Case("exception", eSymbolTypeException)
-               .Case("sourcefile", eSymbolTypeSourceFile)
-               .Case("headerfile", eSymbolTypeHeaderFile)
-               .Case("objectfile", eSymbolTypeObjectFile)
-               .Case("commonblock", eSymbolTypeCommonBlock)
-               .Case("block", eSymbolTypeBlock)
-               .Case("local", eSymbolTypeLocal)
-               .Case("param", eSymbolTypeParam)
-               .Case("variable", eSymbolTypeVariable)
-               .Case("variableType", eSymbolTypeVariableType)
-               .Case("lineentry", eSymbolTypeLineEntry)
-               .Case("lineheader", eSymbolTypeLineHeader)
-               .Case("scopebegin", eSymbolTypeScopeBegin)
-               .Case("scopeend", eSymbolTypeScopeEnd)
-               .Case("additional,", eSymbolTypeAdditional)
-               .Case("compiler", eSymbolTypeCompiler)
-               .Case("instrumentation", eSymbolTypeInstrumentation)
-               .Case("undefined", eSymbolTypeUndefined)
-               .Case("objcclass", eSymbolTypeObjCClass)
-               .Case("objcmetaClass", eSymbolTypeObjCMetaClass)
-               .Case("objcivar", eSymbolTypeObjCIVar)
-               .Case("reexporte", eSymbolTypeReExported)
-               .Default(eSymbolTypeInvalid);
+    llvm::StringRef str_ref = str.value_or("");
+    type = Symbol::GetTypeFromString(str_ref.data());
 
     if (type == eSymbolTypeInvalid) {
       path.report("invalid symbol type");

lldb/test/API/tools/lldb-dap/moduleSymbols/Makefile

@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules

lldb/test/API/tools/lldb-dap/moduleSymbols/TestDAP_moduleSymbols.py

@@ -0,0 +1,40 @@
+"""
+Test lldb-dap moduleSymbols request
+"""
+
+import lldbdap_testcase
+from lldbsuite.test.decorators import *
+
+
+class TestDAP_moduleSymbols(lldbdap_testcase.DAPTestCaseBase):
+    # On windows LLDB doesn't recognize symbols in a.out.
+    @skipIfWindows
+    def test_moduleSymbols(self):
+        """
+        Test that the moduleSymbols request returns correct symbols from the module.
+        """
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+
+        symbol_names = []
+        i = 0
+        while True:
+            next_symbol = self.dap_server.request_moduleSymbols(
+                moduleName="a.out", startIndex=i, count=1
+            )
+            self.assertIn("symbols", next_symbol["body"])
+            result_symbols = next_symbol["body"]["symbols"]
+            self.assertLessEqual(len(result_symbols), 1)
+            if len(result_symbols) == 0:
+                break
+
+            self.assertIn("name", result_symbols[0])
+            symbol_names.append(result_symbols[0]["name"])
+            i += 1
+            if i >= 1000:
+                break
+
+        self.assertGreater(len(symbol_names), 0)
+        self.assertIn("main", symbol_names)
+        self.assertIn("func1", symbol_names)
+        self.assertIn("func2", symbol_names)

lldb/test/API/tools/lldb-dap/moduleSymbols/main.c

@@ -0,0 +1,9 @@
+int func1() { return 42; }
+
+int func2() { return 84; }
+
+int main() {
+  func1();
+  func2();
+  return 0;
+}