[Lldb-commits] [lldb] 1cbe003 - [-gmodules] Let LLDB log a warning if the Clang module hash mismatches.

Adrian Prantl via lldb-commits lldb-commits at lists.llvm.org
Fri Nov 15 11:52:25 PST 2019


Author: Adrian Prantl
Date: 2019-11-15T11:52:13-08:00
New Revision: 1cbe0038944a39ba79078997f9c65ba8abf6fbdd

URL: https://github.com/llvm/llvm-project/commit/1cbe0038944a39ba79078997f9c65ba8abf6fbdd
DIFF: https://github.com/llvm/llvm-project/commit/1cbe0038944a39ba79078997f9c65ba8abf6fbdd.diff

LOG: [-gmodules] Let LLDB log a warning if the Clang module hash mismatches.

This feature is mostly there to aid debugging of Clang module issues,
since the only useful actual the end-user can to is to recompile their
program.

Differential Revision: https://reviews.llvm.org/D70272

Added: 
    lldb/packages/Python/lldbsuite/test/lang/objc/modules-hash-mismatch/Makefile
    lldb/packages/Python/lldbsuite/test/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py
    lldb/packages/Python/lldbsuite/test/lang/objc/modules-hash-mismatch/main.m
    lldb/packages/Python/lldbsuite/test/lang/objc/modules-hash-mismatch/other.m

Modified: 
    lldb/include/lldb/Utility/Log.h
    lldb/packages/Python/lldbsuite/test/make/Makefile.rules
    lldb/source/Host/common/Host.cpp
    lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
    lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Utility/Log.h b/lldb/include/lldb/Utility/Log.h
index 09c0f6954478..01edec044565 100644
--- a/lldb/include/lldb/Utility/Log.h
+++ b/lldb/include/lldb/Utility/Log.h
@@ -166,10 +166,10 @@ class Log final {
 
   bool GetVerbose() const;
 
-private:
   void VAPrintf(const char *format, va_list args);
   void VAError(const char *format, va_list args);
 
+private:
   Channel &m_channel;
 
   // The mutex makes sure enable/disable operations are thread-safe. The

diff  --git a/lldb/packages/Python/lldbsuite/test/lang/objc/modules-hash-mismatch/Makefile b/lldb/packages/Python/lldbsuite/test/lang/objc/modules-hash-mismatch/Makefile
new file mode 100644
index 000000000000..59bf009f6867
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/lang/objc/modules-hash-mismatch/Makefile
@@ -0,0 +1,16 @@
+OBJC_SOURCES := main.m
+CFLAGS_EXTRAS = -I$(BUILDDIR)
+USE_PRIVATE_MODULE_CACHE = YES
+
+.PHONY: update-module
+
+all: $(EXE)
+	$(MAKE) -f $(SRCDIR)/Makefile update-module
+
+include Makefile.rules
+
+update-module:
+	echo "forcing an update of f.pcm"
+	echo "typedef int something_other;" > $(BUILDDIR)/f.h
+	$(CC) $(CFLAGS) $(MANDATORY_MODULE_BUILD_CFLAGS) \
+		-c $(SRCDIR)/other.m -o $(BUILDDIR)/other.o

diff  --git a/lldb/packages/Python/lldbsuite/test/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py b/lldb/packages/Python/lldbsuite/test/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py
new file mode 100644
index 000000000000..298e26ee400d
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py
@@ -0,0 +1,49 @@
+from __future__ import print_function
+
+import unittest2
+import os
+import shutil
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestClangModuleHashMismatch(TestBase):
+    mydir = TestBase.compute_mydir(__file__)
+
+    def setUp(self):
+        TestBase.setUp(self)
+
+    @skipUnlessDarwin
+    @skipIf(debug_info=no_match(["gmodules"]))
+    def test_expr(self):
+        with open(self.getBuildArtifact("module.modulemap"), "w") as f:
+            f.write("""
+                    module Foo { header "f.h" }
+                    """)
+        with open(self.getBuildArtifact("f.h"), "w") as f:
+            f.write("""
+                    typedef int my_int;
+                    void f() {}
+                    """)
+
+        mod_cache = self.getBuildArtifact("private-module-cache")
+        if os.path.isdir(mod_cache):
+          shutil.rmtree(mod_cache)
+        self.build()
+        self.assertTrue(os.path.isdir(mod_cache), "module cache exists")
+
+        logfile = self.getBuildArtifact("host.log")
+        self.runCmd("log enable -v -f %s lldb host" % logfile)
+        target, _, _, _ = lldbutil.run_to_source_breakpoint(
+            self, "break here", lldb.SBFileSpec("main.m"))
+        target.GetModuleAtIndex(0).FindTypes('my_int') 
+
+        found = False
+        with open(logfile, 'r') as f:
+            for line in f:
+                if "hash mismatch" in line and "Foo" in line:
+                    found = True
+        self.assertTrue(found)

diff  --git a/lldb/packages/Python/lldbsuite/test/lang/objc/modules-hash-mismatch/main.m b/lldb/packages/Python/lldbsuite/test/lang/objc/modules-hash-mismatch/main.m
new file mode 100644
index 000000000000..5065222731ea
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/lang/objc/modules-hash-mismatch/main.m
@@ -0,0 +1,6 @@
+#include "f.h"
+int main(int argc, char **argv) {
+  my_int i = argc;
+  f(); // break here
+  return 0;
+}

diff  --git a/lldb/packages/Python/lldbsuite/test/lang/objc/modules-hash-mismatch/other.m b/lldb/packages/Python/lldbsuite/test/lang/objc/modules-hash-mismatch/other.m
new file mode 100644
index 000000000000..0afd3eb078db
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/lang/objc/modules-hash-mismatch/other.m
@@ -0,0 +1,4 @@
+#include "f.h"
+something_other f() {
+  return 0;
+}

diff  --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
index bde94d2c462b..f25d062ca43f 100644
--- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -25,6 +25,7 @@
 # LD_EXTRAS :=
 # SPLIT_DEBUG_SYMBOLS := YES
 # CROSS_COMPILE :=
+# USE_PRIVATE_MODULE_CACHE := YES
 #
 # And test/functionalities/archives/Makefile:
 # MAKE_DSYM := NO
@@ -306,7 +307,13 @@ ifeq "$(MAKE_DWO)" "YES"
 	CFLAGS += -gsplit-dwarf
 endif
 
-MODULE_BASE_FLAGS := -fmodules -gmodules -fmodules-cache-path=$(CLANG_MODULE_CACHE_DIR)
+ifeq "$(USE_PRIVATE_MODULE_CACHE)" "YES"
+THE_CLANG_MODULE_CACHE_DIR := $(BUILDDIR)/private-module-cache
+else
+THE_CLANG_MODULE_CACHE_DIR := $(CLANG_MODULE_CACHE_DIR)
+endif
+
+MODULE_BASE_FLAGS := -fmodules -gmodules -fmodules-cache-path=$(THE_CLANG_MODULE_CACHE_DIR)
 MANDATORY_MODULE_BUILD_CFLAGS := $(MODULE_BASE_FLAGS) -gmodules
 # Build flags for building with C++ modules.
 # -glldb is necessary for emitting information about what modules were imported.

diff  --git a/lldb/source/Host/common/Host.cpp b/lldb/source/Host/common/Host.cpp
index 8e210c7e5fa5..5fbb655fc793 100644
--- a/lldb/source/Host/common/Host.cpp
+++ b/lldb/source/Host/common/Host.cpp
@@ -293,10 +293,21 @@ void Host::SystemLog(SystemLogType type, const char *format, va_list args) {
 #endif
 
 void Host::SystemLog(SystemLogType type, const char *format, ...) {
-  va_list args;
-  va_start(args, format);
-  SystemLog(type, format, args);
-  va_end(args);
+  {
+    va_list args;
+    va_start(args, format);
+    SystemLog(type, format, args);
+    va_end(args);
+  }
+
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST));
+  if (log && log->GetVerbose()) {
+    // Log to log channel. This allows testcases to grep for log output.
+    va_list args;
+    va_start(args, format);
+    log->VAPrintf(format, args);
+    va_end(args);
+  }
 }
 
 lldb::pid_t Host::GetCurrentProcessID() { return ::getpid(); }

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 5b508181406d..08dcfa57ffee 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1555,6 +1555,40 @@ SymbolFileDWARF::GetDIE(const DIERef &die_ref) {
     return DWARFDIE();
 }
 
+/// Return the DW_AT_(GNU_)dwo_name.
+static const char *GetDWOName(DWARFCompileUnit &dwarf_cu,
+                              const DWARFDebugInfoEntry &cu_die) {
+  const char *dwo_name =
+      cu_die.GetAttributeValueAsString(&dwarf_cu, DW_AT_GNU_dwo_name, nullptr);
+  if (!dwo_name)
+    dwo_name =
+        cu_die.GetAttributeValueAsString(&dwarf_cu, DW_AT_dwo_name, nullptr);
+  return dwo_name;
+}
+
+/// Return the DW_AT_(GNU_)dwo_id.
+/// FIXME: Technically 0 is a valid hash.
+static uint64_t GetDWOId(DWARFCompileUnit &dwarf_cu,
+                         const DWARFDebugInfoEntry &cu_die) {
+  uint64_t dwo_id =
+      cu_die.GetAttributeValueAsUnsigned(&dwarf_cu, DW_AT_GNU_dwo_id, 0);
+  if (!dwo_id)
+    dwo_id = cu_die.GetAttributeValueAsUnsigned(&dwarf_cu, DW_AT_dwo_id, 0);
+  return dwo_id;
+}
+
+llvm::Optional<uint64_t> SymbolFileDWARF::GetDWOId() {
+  if (GetNumCompileUnits() == 1) {
+    if (auto comp_unit = GetCompileUnitAtIndex(0))
+      if (DWARFCompileUnit *cu = llvm::dyn_cast_or_null<DWARFCompileUnit>(
+              GetDWARFCompileUnit(comp_unit.get())))
+        if (DWARFDebugInfoEntry *cu_die = cu->DIE().GetDIE())
+          if (uint64_t dwo_id = ::GetDWOId(*cu, *cu_die))
+            return dwo_id;
+  }
+  return {};
+}
+
 std::unique_ptr<SymbolFileDWARFDwo>
 SymbolFileDWARF::GetDwoSymbolFileForCompileUnit(
     DWARFUnit &unit, const DWARFDebugInfoEntry &cu_die) {
@@ -1571,15 +1605,13 @@ SymbolFileDWARF::GetDwoSymbolFileForCompileUnit(
   if (!dwarf_cu)
     return nullptr;
 
-  const char *dwo_name =
-      cu_die.GetAttributeValueAsString(dwarf_cu, DW_AT_GNU_dwo_name, nullptr);
+  const char *dwo_name = GetDWOName(*dwarf_cu, cu_die);
   if (!dwo_name)
     return nullptr;
 
   SymbolFileDWARFDwp *dwp_symfile = GetDwpSymbolFile();
   if (dwp_symfile) {
-    uint64_t dwo_id =
-        cu_die.GetAttributeValueAsUnsigned(dwarf_cu, DW_AT_GNU_dwo_id, 0);
+    uint64_t dwo_id = ::GetDWOId(*dwarf_cu, cu_die);
     std::unique_ptr<SymbolFileDWARFDwo> dwo_symfile =
         dwp_symfile->GetSymbolFileForDwoId(*dwarf_cu, dwo_id);
     if (dwo_symfile)
@@ -1619,15 +1651,18 @@ void SymbolFileDWARF::UpdateExternalModuleListIfNeeded() {
   if (m_fetched_external_modules)
     return;
   m_fetched_external_modules = true;
-
   DWARFDebugInfo *debug_info = DebugInfo();
 
   // Follow DWO skeleton unit breadcrumbs.
   const uint32_t num_compile_units = GetNumCompileUnits();
   for (uint32_t cu_idx = 0; cu_idx < num_compile_units; ++cu_idx) {
-    DWARFUnit *dwarf_cu = debug_info->GetUnitAtIndex(cu_idx);
+    auto *dwarf_cu =
+        llvm::dyn_cast<DWARFCompileUnit>(debug_info->GetUnitAtIndex(cu_idx));
+    if (!dwarf_cu)
+      continue;
+
     const DWARFBaseDIE die = dwarf_cu->GetUnitDIEOnly();
-    if (!die || die.HasChildren())
+    if (!die || die.HasChildren() || !die.GetDIE())
       continue;
 
     const char *name = die.GetAttributeValueAsString(DW_AT_name, nullptr);
@@ -1639,10 +1674,7 @@ void SymbolFileDWARF::UpdateExternalModuleListIfNeeded() {
     if (module_sp)
       continue;
 
-    const char *dwo_path =
-        die.GetAttributeValueAsString(DW_AT_GNU_dwo_name, nullptr);
-    if (!dwo_path)
-      dwo_path = die.GetAttributeValueAsString(DW_AT_dwo_name, nullptr);
+    const char *dwo_path = GetDWOName(*dwarf_cu, *die.GetDIE());
     if (!dwo_path)
       continue;
 
@@ -1685,12 +1717,35 @@ void SymbolFileDWARF::UpdateExternalModuleListIfNeeded() {
       GetObjectFile()->GetModule()->ReportWarning(
           "0x%8.8x: unable to locate module needed for external types: "
           "%s\nerror: %s\nDebugging will be degraded due to missing "
-          "types. Rebuilding your project will regenerate the needed "
+          "types. Rebuilding the project will regenerate the needed "
           "module files.",
           die.GetOffset(), dwo_module_spec.GetFileSpec().GetPath().c_str(),
           error.AsCString("unknown error"));
       continue;
     }
+
+    // Verify the DWO hash.
+    // FIXME: Technically "0" is a valid hash.
+    uint64_t dwo_id = ::GetDWOId(*dwarf_cu, *die.GetDIE());
+    if (!dwo_id)
+      continue;
+
+    auto *dwo_symfile =
+        llvm::dyn_cast_or_null<SymbolFileDWARF>(module_sp->GetSymbolFile());
+    if (!dwo_symfile)
+      continue;
+    llvm::Optional<uint64_t> dwo_dwo_id = dwo_symfile->GetDWOId();
+    if (!dwo_dwo_id)
+      continue;
+
+    if (dwo_id != dwo_dwo_id) {
+      GetObjectFile()->GetModule()->ReportWarning(
+          "0x%8.8x: Module %s is out-of-date (hash mismatch). Type information "
+          "from this module may be incomplete or inconsistent with the rest of "
+          "the program. Rebuilding the project will regenerate the needed "
+          "module files.",
+          die.GetOffset(), dwo_module_spec.GetFileSpec().GetPath().c_str());
+    }
   }
 }
 

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
index 1b8633ad1f1f..a86350844ef5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -300,6 +300,9 @@ class SymbolFileDWARF : public lldb_private::SymbolFile,
 
   virtual llvm::Optional<uint32_t> GetDwoNum() { return llvm::None; }
 
+  /// If this is a DWARF object with a single CU, return its DW_AT_dwo_id.
+  llvm::Optional<uint64_t> GetDWOId();
+
   static bool
   DIEInDeclContext(const lldb_private::CompilerDeclContext *parent_decl_ctx,
                    const DWARFDIE &die);


        


More information about the lldb-commits mailing list