[Lldb-commits] [lldb] DebugInfoD issues, take 2 (PR #86812)

Kevin Frei via lldb-commits lldb-commits at lists.llvm.org
Wed Mar 27 08:37:34 PDT 2024


https://github.com/kevinfrei updated https://github.com/llvm/llvm-project/pull/86812

>From 5e3a35bb69b0bd6c3950deaba35a78c085bc5728 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Mon, 25 Mar 2024 08:23:47 -0700
Subject: [PATCH 1/4] Trying to deal with Linux AArch64 test failures :/

---
 .../SymbolVendor/ELF/SymbolVendorELF.cpp      |  18 ++
 .../API/debuginfod/Normal/TestDebuginfod.py   | 187 +++++++++++++++++
 .../SplitDWARF/TestDebuginfodDWP.py           | 196 ++++++++++++++++++
 3 files changed, 401 insertions(+)
 create mode 100644 lldb/test/API/debuginfod/Normal/TestDebuginfod.py
 create mode 100644 lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py

diff --git a/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp b/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
index b5fe35d71032a8..a881218a56cef3 100644
--- a/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
+++ b/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
@@ -44,6 +44,24 @@ llvm::StringRef SymbolVendorELF::GetPluginDescriptionStatic() {
          "executables.";
 }
 
+// If this is needed elsewhere, it can be exported/moved.
+static bool IsDwpSymbolFile(const lldb::ModuleSP &module_sp,
+                            const FileSpec &file_spec) {
+  DataBufferSP dwp_file_data_sp;
+  lldb::offset_t dwp_file_data_offset = 0;
+  // Try to create an ObjectFile from the file_spec.
+  ObjectFileSP dwp_obj_file = ObjectFile::FindPlugin(
+      module_sp, &file_spec, 0, FileSystem::Instance().GetByteSize(file_spec),
+      dwp_file_data_sp, dwp_file_data_offset);
+  // The presence of a debug_cu_index section is the key identifying feature of
+  // a DWP file. Make sure we don't fill in the section list on dwp_obj_file
+  // (by calling GetSectionList(false)) as this is invoked before we may have
+  // all the symbol files collected and available.
+  return dwp_obj_file && ObjectFileELF::classof(dwp_obj_file.get()) &&
+         dwp_obj_file->GetSectionList(false)->FindSectionByType(
+             eSectionTypeDWARFDebugCuIndex, false);
+}
+
 // CreateInstance
 //
 // Platforms can register a callback to use when creating symbol vendors to
diff --git a/lldb/test/API/debuginfod/Normal/TestDebuginfod.py b/lldb/test/API/debuginfod/Normal/TestDebuginfod.py
new file mode 100644
index 00000000000000..a662fb9fc6e686
--- /dev/null
+++ b/lldb/test/API/debuginfod/Normal/TestDebuginfod.py
@@ -0,0 +1,187 @@
+import os
+import shutil
+import tempfile
+import struct
+
+import lldb
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+def getUUID(aoutuuid):
+    """
+    Pull the 20 byte UUID out of the .note.gnu.build-id section that was dumped
+    to a file already, as part of the build.
+    """
+    with open(aoutuuid, "rb") as f:
+        data = f.read(36)
+        if len(data) != 36:
+            return None
+        header = struct.unpack_from("<4I", data)
+        if len(header) != 4:
+            return None
+        # 4 element 'prefix', 20 bytes of uuid, 3 byte long string: 'GNU':
+        if header[0] != 4 or header[1] != 20 or header[2] != 3 or header[3] != 0x554E47:
+            return None
+        return data[16:].hex()
+
+
+"""
+Test support for the DebugInfoD network symbol acquisition protocol.
+This one is for simple / no split-dwarf scenarios.
+
+For no-split-dwarf scenarios, there are 2 variations:
+1 - A stripped binary with it's corresponding unstripped binary:
+2 - A stripped binary with a corresponding --only-keep-debug symbols file
+"""
+
+
+ at skipUnlessPlatform(["linux", "freebsd"])
+class DebugInfodTests(TestBase):
+    # No need to try every flavor of debug inf.
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def test_normal_no_symbols(self):
+        """
+        Validate behavior with no symbols or symbol locator.
+        ('baseline negative' behavior)
+        """
+        test_root = self.config_test(["a.out"])
+        self.try_breakpoint(False)
+
+    def test_normal_default(self):
+        """
+        Validate behavior with symbols, but no symbol locator.
+        ('baseline positive' behavior)
+        """
+        test_root = self.config_test(["a.out", "a.out.debug"])
+        self.try_breakpoint(True)
+
+    def test_debuginfod_symbols(self):
+        """
+        Test behavior with the full binary available from Debuginfod as
+        'debuginfo' from the plug-in.
+        """
+        test_root = self.config_test(["a.out"], "a.out.full")
+        self.try_breakpoint(True)
+
+    def test_debuginfod_executable(self):
+        """
+        Test behavior with the full binary available from Debuginfod as
+        'executable' from the plug-in.
+        """
+        test_root = self.config_test(["a.out"], None, "a.out.full")
+        self.try_breakpoint(True)
+
+    def test_debuginfod_okd_symbols(self):
+        """
+        Test behavior with the 'only-keep-debug' symbols available from Debuginfod.
+        """
+        test_root = self.config_test(["a.out"], "a.out.debug")
+        self.try_breakpoint(True)
+
+    def try_breakpoint(self, should_have_loc):
+        """
+        This function creates a target from self.aout, sets a function-name
+        breakpoint, and checks to see if we have a file/line location,
+        as a way to validate that the symbols have been loaded.
+        should_have_loc specifies if we're testing that symbols have or
+        haven't been loaded.
+        """
+        target = self.dbg.CreateTarget(self.aout)
+        self.assertTrue(target and target.IsValid(), "Target is valid")
+
+        bp = target.BreakpointCreateByName("func")
+        self.assertTrue(bp and bp.IsValid(), "Breakpoint is valid")
+        self.assertEqual(bp.GetNumLocations(), 1)
+
+        loc = bp.GetLocationAtIndex(0)
+        self.assertTrue(loc and loc.IsValid(), "Location is valid")
+        addr = loc.GetAddress()
+        self.assertTrue(addr and addr.IsValid(), "Loc address is valid")
+        line_entry = addr.GetLineEntry()
+        self.assertEqual(
+            should_have_loc,
+            line_entry != None and line_entry.IsValid(),
+            "Loc line entry is valid",
+        )
+        if should_have_loc:
+            self.assertEqual(line_entry.GetLine(), 4)
+            self.assertEqual(
+                line_entry.GetFileSpec().GetFilename(),
+                self.main_source_file.GetFilename(),
+            )
+        self.dbg.DeleteTarget(target)
+        shutil.rmtree(self.tmp_dir)
+
+    def config_test(self, local_files, debuginfo=None, executable=None):
+        """
+        Set up a test with local_files[] copied to a different location
+        so that we control which files are, or are not, found in the file system.
+        Also, create a stand-alone file-system 'hosted' debuginfod server with the
+        provided debuginfo and executable files (if they exist)
+
+        Make the filesystem look like:
+
+        /tmp/<tmpdir>/test/[local_files]
+
+        /tmp/<tmpdir>/cache (for lldb to use as a temp cache)
+
+        /tmp/<tmpdir>/buildid/<uuid>/executable -> <executable>
+        /tmp/<tmpdir>/buildid/<uuid>/debuginfo -> <debuginfo>
+        Returns the /tmp/<tmpdir> path
+        """
+
+        self.build()
+
+        uuid = getUUID(self.getBuildArtifact("a.out.uuid"))
+        if !uuid:
+            self.fail("Could not get UUID for a.out")
+            return
+        self.main_source_file = lldb.SBFileSpec("main.c")
+        self.tmp_dir = tempfile.mkdtemp()
+        test_dir = os.path.join(self.tmp_dir, "test")
+        os.makedirs(test_dir)
+
+        self.aout = ""
+        # Copy the files used by the test:
+        for f in local_files:
+            shutil.copy(self.getBuildArtifact(f), test_dir)
+            # The first item is the binary to be used for the test
+            if self.aout == "":
+                self.aout = os.path.join(test_dir, f)
+
+        use_debuginfod = debuginfo != None or executable != None
+
+        # Populated the 'file://... mocked' Debuginfod server:
+        if use_debuginfod:
+            os.makedirs(os.path.join(self.tmp_dir, "cache"))
+            uuid_dir = os.path.join(self.tmp_dir, "buildid", uuid)
+            os.makedirs(uuid_dir)
+            if debuginfo:
+                shutil.copy(
+                    self.getBuildArtifact(debuginfo),
+                    os.path.join(uuid_dir, "debuginfo"),
+                )
+            if executable:
+                shutil.copy(
+                    self.getBuildArtifact(executable),
+                    os.path.join(uuid_dir, "executable"),
+                )
+
+        # Configure LLDB for the test:
+        self.runCmd(
+            "settings set symbols.enable-external-lookup %s"
+            % str(use_debuginfod).lower()
+        )
+        self.runCmd("settings clear plugin.symbol-locator.debuginfod.server-urls")
+        if use_debuginfod:
+            self.runCmd(
+                "settings set plugin.symbol-locator.debuginfod.cache-path %s/cache"
+                % self.tmp_dir
+            )
+            self.runCmd(
+                "settings insert-before plugin.symbol-locator.debuginfod.server-urls 0 file://%s"
+                % self.tmp_dir
+            )
diff --git a/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py b/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py
new file mode 100644
index 00000000000000..fda54e15d8a847
--- /dev/null
+++ b/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py
@@ -0,0 +1,196 @@
+"""
+Test support for the DebugInfoD network symbol acquisition protocol.
+"""
+import os
+import shutil
+import tempfile
+import struct
+
+import lldb
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+def getUUID(aoutuuid):
+    """
+    Pull the 20 byte UUID out of the .note.gnu.build-id section that was dumped
+    to a file already, as part of the build.
+    """
+    with open(aoutuuid, "rb") as f:
+        data = f.read(36)
+        if len(data) != 36:
+            return None
+        header = struct.unpack_from("<4I", data)
+        if len(header) != 4:
+            return None
+        # 4 element 'prefix', 20 bytes of uuid, 3 byte long string: 'GNU':
+        if header[0] != 4 or header[1] != 20 or header[2] != 3 or header[3] != 0x554E47:
+            return None
+        return data[16:].hex()
+
+
+"""
+Test support for the DebugInfoD network symbol acquisition protocol.
+This file is for split-dwarf (dwp) scenarios.
+
+1 - A split binary target with it's corresponding DWP file
+2 - A stripped, split binary target with an unstripped binary and a DWP file
+3 - A stripped, split binary target with an --only-keep-debug symbols file and a DWP file
+"""
+
+
+ at skipUnlessPlatform(["linux", "freebsd"])
+class DebugInfodDWPTests(TestBase):
+    # No need to try every flavor of debug inf.
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def test_normal_stripped(self):
+        """
+        Validate behavior with a stripped binary, no symbols or symbol locator.
+        """
+        self.config_test(["a.out"])
+        self.try_breakpoint(False)
+
+    def test_normal_stripped_split_with_dwp(self):
+        """
+        Validate behavior with symbols, but no symbol locator.
+        """
+        self.config_test(["a.out", "a.out.debug", "a.out.dwp"])
+        self.try_breakpoint(True)
+
+    def test_normal_stripped_only_dwp(self):
+        """
+        Validate behavior *with* dwp symbols only, but missing other symbols,
+        but no symbol locator. This shouldn't work: without the other symbols
+        DWO's appear mostly useless.
+        """
+        self.config_test(["a.out", "a.out.dwp"])
+        self.try_breakpoint(False)
+
+    def test_debuginfod_dwp_from_service(self):
+        """
+        Test behavior with the unstripped binary, and DWP from the service.
+        """
+        self.config_test(["a.out.debug"], "a.out.dwp")
+        self.try_breakpoint(True)
+
+    def test_debuginfod_both_symfiles_from_service(self):
+        """
+        Test behavior with a stripped binary, with the unstripped binary and
+        dwp symbols from Debuginfod.
+        """
+        self.config_test(["a.out"], "a.out.dwp", "a.out.full")
+        self.try_breakpoint(True)
+
+    def test_debuginfod_both_okd_symfiles_from_service(self):
+        """
+        Test behavior with both the only-keep-debug symbols and the dwp symbols
+        from Debuginfod.
+        """
+        self.config_test(["a.out"], "a.out.dwp", "a.out.debug")
+        self.try_breakpoint(True)
+
+    def try_breakpoint(self, should_have_loc):
+        """
+        This function creates a target from self.aout, sets a function-name
+        breakpoint, and checks to see if we have a file/line location,
+        as a way to validate that the symbols have been loaded.
+        should_have_loc specifies if we're testing that symbols have or
+        haven't been loaded.
+        """
+        target = self.dbg.CreateTarget(self.aout)
+        self.assertTrue(target and target.IsValid(), "Target is valid")
+
+        bp = target.BreakpointCreateByName("func")
+        self.assertTrue(bp and bp.IsValid(), "Breakpoint is valid")
+        self.assertEqual(bp.GetNumLocations(), 1)
+
+        loc = bp.GetLocationAtIndex(0)
+        self.assertTrue(loc and loc.IsValid(), "Location is valid")
+        addr = loc.GetAddress()
+        self.assertTrue(addr and addr.IsValid(), "Loc address is valid")
+        line_entry = addr.GetLineEntry()
+        self.assertEqual(
+            should_have_loc,
+            line_entry != None and line_entry.IsValid(),
+            "Loc line entry is valid",
+        )
+        if should_have_loc:
+            self.assertEqual(line_entry.GetLine(), 4)
+            self.assertEqual(
+                line_entry.GetFileSpec().GetFilename(),
+                self.main_source_file.GetFilename(),
+            )
+        self.dbg.DeleteTarget(target)
+        shutil.rmtree(self.tmp_dir)
+
+    def config_test(self, local_files, debuginfo=None, executable=None):
+        """
+        Set up a test with local_files[] copied to a different location
+        so that we control which files are, or are not, found in the file system.
+        Also, create a stand-alone file-system 'hosted' debuginfod server with the
+        provided debuginfo and executable files (if they exist)
+
+        Make the filesystem look like:
+
+        /tmp/<tmpdir>/test/[local_files]
+
+        /tmp/<tmpdir>/cache (for lldb to use as a temp cache)
+
+        /tmp/<tmpdir>/buildid/<uuid>/executable -> <executable>
+        /tmp/<tmpdir>/buildid/<uuid>/debuginfo -> <debuginfo>
+        Returns the /tmp/<tmpdir> path
+        """
+
+        self.build()
+
+        uuid = getUUID(self.getBuildArtifact("a.out.uuid"))
+        if !uuid:
+            self.fail("Could not get UUID for a.out")
+            return
+        self.main_source_file = lldb.SBFileSpec("main.c")
+        self.tmp_dir = tempfile.mkdtemp()
+        self.test_dir = os.path.join(self.tmp_dir, "test")
+        os.makedirs(self.test_dir)
+
+        self.aout = ""
+        # Copy the files used by the test:
+        for f in local_files:
+            shutil.copy(self.getBuildArtifact(f), self.test_dir)
+            if self.aout == "":
+                self.aout = os.path.join(self.test_dir, f)
+
+        use_debuginfod = debuginfo != None or executable != None
+
+        # Populated the 'file://... mocked' Debuginfod server:
+        if use_debuginfod:
+            os.makedirs(os.path.join(self.tmp_dir, "cache"))
+            uuid_dir = os.path.join(self.tmp_dir, "buildid", uuid)
+            os.makedirs(uuid_dir)
+            if debuginfo:
+                shutil.copy(
+                    self.getBuildArtifact(debuginfo),
+                    os.path.join(uuid_dir, "debuginfo"),
+                )
+            if executable:
+                shutil.copy(
+                    self.getBuildArtifact(executable),
+                    os.path.join(uuid_dir, "executable"),
+                )
+        os.remove(self.getBuildArtifact("main.dwo"))
+        # Configure LLDB for the test:
+        self.runCmd(
+            "settings set symbols.enable-external-lookup %s"
+            % str(use_debuginfod).lower()
+        )
+        self.runCmd("settings clear plugin.symbol-locator.debuginfod.server-urls")
+        if use_debuginfod:
+            self.runCmd(
+                "settings set plugin.symbol-locator.debuginfod.cache-path %s/cache"
+                % self.tmp_dir
+            )
+            self.runCmd(
+                "settings insert-before plugin.symbol-locator.debuginfod.server-urls 0 file://%s"
+                % self.tmp_dir
+            )

>From feadde51c4c1452a61732dbe245706538c5cd1e4 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Mon, 25 Mar 2024 08:33:58 -0700
Subject: [PATCH 2/4] Reapply "DebugInfoD tests + fixing issues exposed by
 tests (#85693)"

This reverts commit 7fc2fbb3f1961e0ad0722c2d749ddd6264195a1c.
---
 .../Python/lldbsuite/test/make/Makefile.rules | 33 +++++++++++++++-
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp      | 38 ++++++++++++-------
 .../Plugins/SymbolLocator/CMakeLists.txt      |  7 +++-
 .../SymbolVendor/ELF/SymbolVendorELF.cpp      | 15 ++++++--
 lldb/test/API/debuginfod/Normal/Makefile      | 25 ++++++++++++
 .../API/debuginfod/Normal/TestDebuginfod.py   |  4 +-
 lldb/test/API/debuginfod/Normal/main.c        |  7 ++++
 lldb/test/API/debuginfod/SplitDWARF/Makefile  | 28 ++++++++++++++
 .../SplitDWARF/TestDebuginfodDWP.py           |  2 +-
 lldb/test/API/debuginfod/SplitDWARF/main.c    |  7 ++++
 10 files changed, 144 insertions(+), 22 deletions(-)
 create mode 100644 lldb/test/API/debuginfod/Normal/Makefile
 create mode 100644 lldb/test/API/debuginfod/Normal/main.c
 create mode 100644 lldb/test/API/debuginfod/SplitDWARF/Makefile
 create mode 100644 lldb/test/API/debuginfod/SplitDWARF/main.c

diff --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
index bfd249ccd43f2e..9fce4b8cf07791 100644
--- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -51,7 +51,7 @@ LLDB_BASE_DIR := $(THIS_FILE_DIR)/../../../../../
 #
 # GNUWin32 uname gives "windows32" or "server version windows32" while
 # some versions of MSYS uname return "MSYS_NT*", but most environments
-# standardize on "Windows_NT", so we'll make it consistent here. 
+# standardize on "Windows_NT", so we'll make it consistent here.
 # When running tests from Visual Studio, the environment variable isn't
 # inherited all the way down to the process spawned for make.
 #----------------------------------------------------------------------
@@ -210,6 +210,12 @@ else
 	ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES"
 		DSYM = $(EXE).debug
 	endif
+
+	ifeq "$(MAKE_DWP)" "YES"
+		MAKE_DWO := YES
+		DWP_NAME = $(EXE).dwp
+		DYLIB_DWP_NAME = $(DYLIB_NAME).dwp
+	endif
 endif
 
 LIMIT_DEBUG_INFO_FLAGS =
@@ -357,6 +363,7 @@ ifneq "$(OS)" "Darwin"
 
 	OBJCOPY ?= $(call replace_cc_with,objcopy)
 	ARCHIVER ?= $(call replace_cc_with,ar)
+	DWP ?= $(call replace_cc_with,dwp)
 	override AR = $(ARCHIVER)
 endif
 
@@ -527,6 +534,10 @@ ifneq "$(CXX)" ""
 	endif
 endif
 
+ifeq "$(GEN_GNU_BUILD_ID)" "YES"
+	LDFLAGS += -Wl,--build-id
+endif
+
 #----------------------------------------------------------------------
 # DYLIB_ONLY variable can be used to skip the building of a.out.
 # See the sections below regarding dSYM file as well as the building of
@@ -565,11 +576,25 @@ else
 endif
 else
 ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES"
+ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES"
+	cp "$(EXE)" "$(EXE).unstriped""
+endif
 	$(OBJCOPY) --only-keep-debug "$(EXE)" "$(DSYM)"
 	$(OBJCOPY) --strip-debug --add-gnu-debuglink="$(DSYM)" "$(EXE)" "$(EXE)"
 endif
+ifeq "$(MAKE_DWP)" "YES"
+	$(DWP) -o "$(DWP_NAME)" $(DWOS)
+endif
 endif
 
+
+#----------------------------------------------------------------------
+# Support emitting the content of the GNU build-id into a file
+# This needs to be used in conjunction with GEN_GNU_BUILD_ID := YES
+#----------------------------------------------------------------------
+$(EXE).uuid : $(EXE)
+	$(OBJCOPY) --dump-section=.note.gnu.build-id=$@ $<
+
 #----------------------------------------------------------------------
 # Make the dylib
 #----------------------------------------------------------------------
@@ -610,9 +635,15 @@ endif
 else
 	$(LD) $(DYLIB_OBJECTS) $(LDFLAGS) -shared -o "$(DYLIB_FILENAME)"
 ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES"
+	ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES"
+	cp "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME).unstripped"
+	endif
 	$(OBJCOPY) --only-keep-debug "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME).debug"
 	$(OBJCOPY) --strip-debug --add-gnu-debuglink="$(DYLIB_FILENAME).debug" "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME)"
 endif
+ifeq "$(MAKE_DWP)" "YES"
+	$(DWP) -o $(DYLIB_DWP_FILE) $(DYLIB_DWOS)
+endif
 endif
 
 #----------------------------------------------------------------------
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 1164bc62682a9a..0f459706c05643 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -4378,26 +4378,38 @@ const std::shared_ptr<SymbolFileDWARFDwo> &SymbolFileDWARF::GetDwpSymbolFile() {
     FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
     ModuleSpec module_spec;
     module_spec.GetFileSpec() = m_objfile_sp->GetFileSpec();
+    FileSpec dwp_filespec;
     for (const auto &symfile : symfiles.files()) {
       module_spec.GetSymbolFileSpec() =
           FileSpec(symfile.GetPath() + ".dwp", symfile.GetPathStyle());
       LLDB_LOG(log, "Searching for DWP using: \"{0}\"",
                module_spec.GetSymbolFileSpec());
-      FileSpec dwp_filespec =
+      dwp_filespec =
           PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
       if (FileSystem::Instance().Exists(dwp_filespec)) {
-        LLDB_LOG(log, "Found DWP file: \"{0}\"", dwp_filespec);
-        DataBufferSP dwp_file_data_sp;
-        lldb::offset_t dwp_file_data_offset = 0;
-        ObjectFileSP dwp_obj_file = ObjectFile::FindPlugin(
-            GetObjectFile()->GetModule(), &dwp_filespec, 0,
-            FileSystem::Instance().GetByteSize(dwp_filespec), dwp_file_data_sp,
-            dwp_file_data_offset);
-        if (dwp_obj_file) {
-          m_dwp_symfile = std::make_shared<SymbolFileDWARFDwo>(
-              *this, dwp_obj_file, DIERef::k_file_index_mask);
-          break;
-        }
+        break;
+      }
+    }
+    if (!FileSystem::Instance().Exists(dwp_filespec)) {
+      LLDB_LOG(log, "No DWP file found locally");
+      // Fill in the UUID for the module we're trying to match for, so we can
+      // find the correct DWP file, as the Debuginfod plugin uses *only* this
+      // data to correctly match the DWP file with the binary.
+      module_spec.GetUUID() = m_objfile_sp->GetUUID();
+      dwp_filespec =
+          PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
+    }
+    if (FileSystem::Instance().Exists(dwp_filespec)) {
+      LLDB_LOG(log, "Found DWP file: \"{0}\"", dwp_filespec);
+      DataBufferSP dwp_file_data_sp;
+      lldb::offset_t dwp_file_data_offset = 0;
+      ObjectFileSP dwp_obj_file = ObjectFile::FindPlugin(
+          GetObjectFile()->GetModule(), &dwp_filespec, 0,
+          FileSystem::Instance().GetByteSize(dwp_filespec), dwp_file_data_sp,
+          dwp_file_data_offset);
+      if (dwp_obj_file) {
+        m_dwp_symfile = std::make_shared<SymbolFileDWARFDwo>(
+            *this, dwp_obj_file, DIERef::k_file_index_mask);
       }
     }
     if (!m_dwp_symfile) {
diff --git a/lldb/source/Plugins/SymbolLocator/CMakeLists.txt b/lldb/source/Plugins/SymbolLocator/CMakeLists.txt
index ca969626f4ffc4..3367022639ab85 100644
--- a/lldb/source/Plugins/SymbolLocator/CMakeLists.txt
+++ b/lldb/source/Plugins/SymbolLocator/CMakeLists.txt
@@ -1,5 +1,10 @@
+# Order matters here: the first symbol locator prevents further searching.
+# For DWARF binaries that are both stripped and split, the Default plugin
+# will return the stripped binary when asked for the ObjectFile, which then
+# prevents an unstripped binary from being requested from the Debuginfod
+# provider.
+add_subdirectory(Debuginfod)
 add_subdirectory(Default)
 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   add_subdirectory(DebugSymbols)
 endif()
-add_subdirectory(Debuginfod)
diff --git a/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp b/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
index a881218a56cef3..f296e655cc466d 100644
--- a/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
+++ b/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
@@ -55,8 +55,8 @@ static bool IsDwpSymbolFile(const lldb::ModuleSP &module_sp,
       dwp_file_data_sp, dwp_file_data_offset);
   // The presence of a debug_cu_index section is the key identifying feature of
   // a DWP file. Make sure we don't fill in the section list on dwp_obj_file
-  // (by calling GetSectionList(false)) as this is invoked before we may have
-  // all the symbol files collected and available.
+  // (by calling GetSectionList(false)) as this function could be called before
+  // we may have all the symbol files collected and available.
   return dwp_obj_file && ObjectFileELF::classof(dwp_obj_file.get()) &&
          dwp_obj_file->GetSectionList(false)->FindSectionByType(
              eSectionTypeDWARFDebugCuIndex, false);
@@ -105,8 +105,15 @@ SymbolVendorELF::CreateInstance(const lldb::ModuleSP &module_sp,
   FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
   FileSpec dsym_fspec =
       PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
-  if (!dsym_fspec)
-    return nullptr;
+  if (!dsym_fspec || IsDwpSymbolFile(module_sp, dsym_fspec)) {
+    // If we have a stripped binary or if we got a DWP file, we should prefer
+    // symbols in the executable acquired through a plugin.
+    ModuleSpec unstripped_spec =
+        PluginManager::LocateExecutableObjectFile(module_spec);
+    if (!unstripped_spec)
+      return nullptr;
+    dsym_fspec = unstripped_spec.GetFileSpec();
+  }
 
   DataBufferSP dsym_file_data_sp;
   lldb::offset_t dsym_file_data_offset = 0;
diff --git a/lldb/test/API/debuginfod/Normal/Makefile b/lldb/test/API/debuginfod/Normal/Makefile
new file mode 100644
index 00000000000000..bd2fa623df473d
--- /dev/null
+++ b/lldb/test/API/debuginfod/Normal/Makefile
@@ -0,0 +1,25 @@
+C_SOURCES := main.c
+
+# For normal (non DWP) Debuginfod tests, we need:
+
+# * The "full" binary: a.out.debug
+#   Produced by Makefile.rules with KEEP_FULL_DEBUG_BINARY set to YES and
+#   SPLIT_DEBUG_SYMBOLS set to YES
+
+# * The stripped binary (a.out)
+#   Produced by Makefile.rules with SPLIT_DEBUG_SYMBOLS set to YES
+
+# * The 'only-keep-debug' binary (a.out.dbg)
+#   Produced below
+
+# * The .uuid file (for a little easier testing code)
+#   Produced below
+
+# Don't strip the debug info from a.out:
+SPLIT_DEBUG_SYMBOLS := YES
+SAVE_FULL_DEBUG_BINARY := YES
+GEN_GNU_BUILD_ID := YES
+
+all: a.out.uuid a.out
+
+include Makefile.rules
diff --git a/lldb/test/API/debuginfod/Normal/TestDebuginfod.py b/lldb/test/API/debuginfod/Normal/TestDebuginfod.py
index a662fb9fc6e686..4bbb43782118c1 100644
--- a/lldb/test/API/debuginfod/Normal/TestDebuginfod.py
+++ b/lldb/test/API/debuginfod/Normal/TestDebuginfod.py
@@ -63,7 +63,7 @@ def test_debuginfod_symbols(self):
         Test behavior with the full binary available from Debuginfod as
         'debuginfo' from the plug-in.
         """
-        test_root = self.config_test(["a.out"], "a.out.full")
+        test_root = self.config_test(["a.out"], "a.out.unstripped")
         self.try_breakpoint(True)
 
     def test_debuginfod_executable(self):
@@ -71,7 +71,7 @@ def test_debuginfod_executable(self):
         Test behavior with the full binary available from Debuginfod as
         'executable' from the plug-in.
         """
-        test_root = self.config_test(["a.out"], None, "a.out.full")
+        test_root = self.config_test(["a.out"], None, "a.out.unstripped")
         self.try_breakpoint(True)
 
     def test_debuginfod_okd_symbols(self):
diff --git a/lldb/test/API/debuginfod/Normal/main.c b/lldb/test/API/debuginfod/Normal/main.c
new file mode 100644
index 00000000000000..4c7184609b4536
--- /dev/null
+++ b/lldb/test/API/debuginfod/Normal/main.c
@@ -0,0 +1,7 @@
+// This is a dump little pair of test files
+
+int func(int argc, const char *argv[]) {
+  return (argc + 1) * (argv[argc][0] + 2);
+}
+
+int main(int argc, const char *argv[]) { return func(0, argv); }
diff --git a/lldb/test/API/debuginfod/SplitDWARF/Makefile b/lldb/test/API/debuginfod/SplitDWARF/Makefile
new file mode 100644
index 00000000000000..266d74cf906298
--- /dev/null
+++ b/lldb/test/API/debuginfod/SplitDWARF/Makefile
@@ -0,0 +1,28 @@
+C_SOURCES := main.c
+
+# For split-dwarf Debuginfod tests, we need:
+
+# * A .DWP file (a.out.dwp)
+#   Produced by Makefile.rules with MAKE_DWO and MERGE_DWOS both set to YES
+
+# * The "full" binary: it's missing things that live in .dwo's (a.out.debug)
+#   Produced by Makefile.rules with KEEP_FULL_DEBUG_BINARY set to YES and
+#   SPLIT_DEBUG_SYMBOLS set to YES
+
+# * The stripped binary (a.out)
+#   Produced by Makefile.rules
+
+# * The 'only-keep-debug' binary (a.out.dbg)
+#   Produced below
+
+# * The .uuid file (for a little easier testing code)
+#   Produced here in the rule below
+
+MAKE_DWP := YES
+SPLIT_DEBUG_SYMBOLS := YES
+SAVE_FULL_DEBUG_BINARY := YES
+GEN_GNU_BUILD_ID := YES
+
+all: a.out.uuid a.out
+
+include Makefile.rules
diff --git a/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py b/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py
index fda54e15d8a847..53a19c0a8454d1 100644
--- a/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py
+++ b/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py
@@ -80,7 +80,7 @@ def test_debuginfod_both_symfiles_from_service(self):
         Test behavior with a stripped binary, with the unstripped binary and
         dwp symbols from Debuginfod.
         """
-        self.config_test(["a.out"], "a.out.dwp", "a.out.full")
+        self.config_test(["a.out"], "a.out.dwp", "a.out.unstripped")
         self.try_breakpoint(True)
 
     def test_debuginfod_both_okd_symfiles_from_service(self):
diff --git a/lldb/test/API/debuginfod/SplitDWARF/main.c b/lldb/test/API/debuginfod/SplitDWARF/main.c
new file mode 100644
index 00000000000000..4c7184609b4536
--- /dev/null
+++ b/lldb/test/API/debuginfod/SplitDWARF/main.c
@@ -0,0 +1,7 @@
+// This is a dump little pair of test files
+
+int func(int argc, const char *argv[]) {
+  return (argc + 1) * (argv[argc][0] + 2);
+}
+
+int main(int argc, const char *argv[]) { return func(0, argv); }

>From eedc7cc2b984ec6a7a8e1d2acea5efd7432d1dc8 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Mon, 25 Mar 2024 15:11:32 -0700
Subject: [PATCH 3/4] Switched to using LLDB to get UUID at @clayborg's
 suggestion

---
 .../Python/lldbsuite/test/make/Makefile.rules |  9 +----
 lldb/test/API/debuginfod/Normal/Makefile      | 12 ++-----
 .../API/debuginfod/Normal/TestDebuginfod.py   | 33 +++++++------------
 lldb/test/API/debuginfod/SplitDWARF/Makefile  | 13 +++-----
 .../SplitDWARF/TestDebuginfodDWP.py           | 33 +++++++------------
 5 files changed, 32 insertions(+), 68 deletions(-)

diff --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
index 9fce4b8cf07791..ee8793fa1b3029 100644
--- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -577,7 +577,7 @@ endif
 else
 ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES"
 ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES"
-	cp "$(EXE)" "$(EXE).unstriped""
+	cp "$(EXE)" "$(EXE).unstripped"
 endif
 	$(OBJCOPY) --only-keep-debug "$(EXE)" "$(DSYM)"
 	$(OBJCOPY) --strip-debug --add-gnu-debuglink="$(DSYM)" "$(EXE)" "$(EXE)"
@@ -588,13 +588,6 @@ endif
 endif
 
 
-#----------------------------------------------------------------------
-# Support emitting the content of the GNU build-id into a file
-# This needs to be used in conjunction with GEN_GNU_BUILD_ID := YES
-#----------------------------------------------------------------------
-$(EXE).uuid : $(EXE)
-	$(OBJCOPY) --dump-section=.note.gnu.build-id=$@ $<
-
 #----------------------------------------------------------------------
 # Make the dylib
 #----------------------------------------------------------------------
diff --git a/lldb/test/API/debuginfod/Normal/Makefile b/lldb/test/API/debuginfod/Normal/Makefile
index bd2fa623df473d..54bd7adae241f5 100644
--- a/lldb/test/API/debuginfod/Normal/Makefile
+++ b/lldb/test/API/debuginfod/Normal/Makefile
@@ -2,24 +2,18 @@ C_SOURCES := main.c
 
 # For normal (non DWP) Debuginfod tests, we need:
 
-# * The "full" binary: a.out.debug
-#   Produced by Makefile.rules with KEEP_FULL_DEBUG_BINARY set to YES and
+# * The full binary: a.out.unstripped
+#   Produced by Makefile.rules with SAVE_FULL_DEBUG_BINARY set to YES and
 #   SPLIT_DEBUG_SYMBOLS set to YES
 
 # * The stripped binary (a.out)
 #   Produced by Makefile.rules with SPLIT_DEBUG_SYMBOLS set to YES
 
-# * The 'only-keep-debug' binary (a.out.dbg)
+# * The 'only-keep-debug' binary (a.out.debug)
 #   Produced below
 
-# * The .uuid file (for a little easier testing code)
-#   Produced below
-
-# Don't strip the debug info from a.out:
 SPLIT_DEBUG_SYMBOLS := YES
 SAVE_FULL_DEBUG_BINARY := YES
 GEN_GNU_BUILD_ID := YES
 
-all: a.out.uuid a.out
-
 include Makefile.rules
diff --git a/lldb/test/API/debuginfod/Normal/TestDebuginfod.py b/lldb/test/API/debuginfod/Normal/TestDebuginfod.py
index 4bbb43782118c1..815e0a7768ad22 100644
--- a/lldb/test/API/debuginfod/Normal/TestDebuginfod.py
+++ b/lldb/test/API/debuginfod/Normal/TestDebuginfod.py
@@ -1,7 +1,6 @@
 import os
 import shutil
 import tempfile
-import struct
 
 import lldb
 from lldbsuite.test.decorators import *
@@ -9,24 +8,6 @@
 from lldbsuite.test.lldbtest import *
 
 
-def getUUID(aoutuuid):
-    """
-    Pull the 20 byte UUID out of the .note.gnu.build-id section that was dumped
-    to a file already, as part of the build.
-    """
-    with open(aoutuuid, "rb") as f:
-        data = f.read(36)
-        if len(data) != 36:
-            return None
-        header = struct.unpack_from("<4I", data)
-        if len(header) != 4:
-            return None
-        # 4 element 'prefix', 20 bytes of uuid, 3 byte long string: 'GNU':
-        if header[0] != 4 or header[1] != 20 or header[2] != 3 or header[3] != 0x554E47:
-            return None
-        return data[16:].hex()
-
-
 """
 Test support for the DebugInfoD network symbol acquisition protocol.
 This one is for simple / no split-dwarf scenarios.
@@ -135,8 +116,8 @@ def config_test(self, local_files, debuginfo=None, executable=None):
 
         self.build()
 
-        uuid = getUUID(self.getBuildArtifact("a.out.uuid"))
-        if !uuid:
+        uuid = self.getUUID("a.out")
+        if not uuid:
             self.fail("Could not get UUID for a.out")
             return
         self.main_source_file = lldb.SBFileSpec("main.c")
@@ -185,3 +166,13 @@ def config_test(self, local_files, debuginfo=None, executable=None):
                 "settings insert-before plugin.symbol-locator.debuginfod.server-urls 0 file://%s"
                 % self.tmp_dir
             )
+
+    def getUUID(self, filename):
+        try:
+            target = self.dbg.CreateTarget(self.getBuildArtifact(filename))
+            module = target.GetModuleAtIndex(0)
+            uuid = module.GetUUIDString().replace("-", "").lower()
+            self.dbg.DeleteTarget(target)
+            return uuid if len(uuid) == 40 else None
+        except:
+            return None
diff --git a/lldb/test/API/debuginfod/SplitDWARF/Makefile b/lldb/test/API/debuginfod/SplitDWARF/Makefile
index 266d74cf906298..3ab9a969e5a447 100644
--- a/lldb/test/API/debuginfod/SplitDWARF/Makefile
+++ b/lldb/test/API/debuginfod/SplitDWARF/Makefile
@@ -3,26 +3,21 @@ C_SOURCES := main.c
 # For split-dwarf Debuginfod tests, we need:
 
 # * A .DWP file (a.out.dwp)
-#   Produced by Makefile.rules with MAKE_DWO and MERGE_DWOS both set to YES
+#   Produced by Makefile.rules with MAKE_DWP set to YES
 
-# * The "full" binary: it's missing things that live in .dwo's (a.out.debug)
-#   Produced by Makefile.rules with KEEP_FULL_DEBUG_BINARY set to YES and
+# * The "full" binary (missing things that live in .dwo's) (a.out.unstripped)
+#   Produced by Makefile.rules with SAVE_FULL_DEBUG_BINARY set to YES and
 #   SPLIT_DEBUG_SYMBOLS set to YES
 
 # * The stripped binary (a.out)
 #   Produced by Makefile.rules
 
-# * The 'only-keep-debug' binary (a.out.dbg)
+# * The 'only-keep-debug' binary (a.out.debug)
 #   Produced below
 
-# * The .uuid file (for a little easier testing code)
-#   Produced here in the rule below
-
 MAKE_DWP := YES
 SPLIT_DEBUG_SYMBOLS := YES
 SAVE_FULL_DEBUG_BINARY := YES
 GEN_GNU_BUILD_ID := YES
 
-all: a.out.uuid a.out
-
 include Makefile.rules
diff --git a/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py b/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py
index 53a19c0a8454d1..4e31ba515242e7 100644
--- a/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py
+++ b/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py
@@ -4,7 +4,6 @@
 import os
 import shutil
 import tempfile
-import struct
 
 import lldb
 from lldbsuite.test.decorators import *
@@ -12,24 +11,6 @@
 from lldbsuite.test.lldbtest import *
 
 
-def getUUID(aoutuuid):
-    """
-    Pull the 20 byte UUID out of the .note.gnu.build-id section that was dumped
-    to a file already, as part of the build.
-    """
-    with open(aoutuuid, "rb") as f:
-        data = f.read(36)
-        if len(data) != 36:
-            return None
-        header = struct.unpack_from("<4I", data)
-        if len(header) != 4:
-            return None
-        # 4 element 'prefix', 20 bytes of uuid, 3 byte long string: 'GNU':
-        if header[0] != 4 or header[1] != 20 or header[2] != 3 or header[3] != 0x554E47:
-            return None
-        return data[16:].hex()
-
-
 """
 Test support for the DebugInfoD network symbol acquisition protocol.
 This file is for split-dwarf (dwp) scenarios.
@@ -145,8 +126,8 @@ def config_test(self, local_files, debuginfo=None, executable=None):
 
         self.build()
 
-        uuid = getUUID(self.getBuildArtifact("a.out.uuid"))
-        if !uuid:
+        uuid = self.getUUID("a.out")
+        if not uuid:
             self.fail("Could not get UUID for a.out")
             return
         self.main_source_file = lldb.SBFileSpec("main.c")
@@ -194,3 +175,13 @@ def config_test(self, local_files, debuginfo=None, executable=None):
                 "settings insert-before plugin.symbol-locator.debuginfod.server-urls 0 file://%s"
                 % self.tmp_dir
             )
+
+    def getUUID(self, filename):
+        try:
+            target = self.dbg.CreateTarget(self.getBuildArtifact(filename))
+            module = target.GetModuleAtIndex(0)
+            uuid = module.GetUUIDString().replace("-", "").lower()
+            self.dbg.DeleteTarget(target)
+            return uuid if len(uuid) == 40 else None
+        except:
+            return None

>From 11224b84a60bc018cd4872e7f07e4bc81eb7bf1f Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Wed, 27 Mar 2024 08:06:50 -0700
Subject: [PATCH 4/4] Disable tests on non-x86 Linux platforms, as they appear
 to fail on AArch64/Arm buildbots

---
 lldb/test/API/debuginfod/Normal/TestDebuginfod.py        | 3 ++-
 lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/lldb/test/API/debuginfod/Normal/TestDebuginfod.py b/lldb/test/API/debuginfod/Normal/TestDebuginfod.py
index 815e0a7768ad22..2e87228c3142f1 100644
--- a/lldb/test/API/debuginfod/Normal/TestDebuginfod.py
+++ b/lldb/test/API/debuginfod/Normal/TestDebuginfod.py
@@ -18,7 +18,8 @@
 """
 
 
- at skipUnlessPlatform(["linux", "freebsd"])
+# It looks like Linux-AArch64 doesn't support build-id's on the LLDB builtbots
+ at skipIf(oslist=no_match(["linux"]), archs=no_match(["i386", "x86_64"]))
 class DebugInfodTests(TestBase):
     # No need to try every flavor of debug inf.
     NO_DEBUG_INFO_TESTCASE = True
diff --git a/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py b/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py
index 4e31ba515242e7..90db3525725cbe 100644
--- a/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py
+++ b/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py
@@ -21,7 +21,8 @@
 """
 
 
- at skipUnlessPlatform(["linux", "freebsd"])
+# It looks like Linux-AArch64 doesn't support build-id's on the LLDB builtbots
+ at skipIf(oslist=no_match(["linux"]), archs=no_match(["i386", "x86_64"]))
 class DebugInfodDWPTests(TestBase):
     # No need to try every flavor of debug inf.
     NO_DEBUG_INFO_TESTCASE = True



More information about the lldb-commits mailing list