[clang] ada2e8e - Reland "Correctly emit dwoIDs after ASTFileSignature refactoring (D81347)"

Raphael Isemann via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 24 02:54:45 PDT 2020


Author: Raphael Isemann
Date: 2020-08-24T11:51:32+02:00
New Revision: ada2e8ea67393aa8c44fe8e9d46be62df6d1c702

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

LOG: Reland "Correctly emit dwoIDs after ASTFileSignature refactoring (D81347)"

This relands D84013 but with a test that relies on less shell features to
hopefully make the test pass on Fuchsia (where the test from the previous patch
version strangely failed with a plain "Exit code 1").

Original summary:

D81347 changes the ASTFileSignature to be an array of 20 uint8_t instead of 5 uint32_t.
However, it didn't update the code in ObjectFilePCHContainerOperations that creates
the dwoID in the module from the ASTFileSignature (`Buffer->Signature` being the
array subclass that is now `std::array<uint8_t, 20>` instead of `std::array<uint32_t, 5>`).

```
  uint64_t Signature = [..] (uint64_t)Buffer->Signature[1] << 32 | Buffer->Signature[0]
```

This code works with the old ASTFileSignature  (where two uint32_t are enough to
fill the uint64_t), but after the patch this only took two bytes from the ASTFileSignature
and only partly filled the Signature uint64_t.

This caused that the dwoID in the module ref and the dwoID in the actual module no
longer match (which in turns causes that LLDB keeps warning about the dwoID's not
matching when debugging -gmodules-compiled binaries).

This patch just unifies the logic for turning the ASTFileSignature into an uint64_t which
makes the dwoID match again (and should prevent issues like that in the future).

Reviewed By: aprantl, dang

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

Added: 
    clang/test/Modules/Inputs/DebugDwoId.h
    clang/test/Modules/ModuleDebugInfoDwoId.cpp

Modified: 
    clang/include/clang/Basic/Module.h
    clang/lib/CodeGen/CGDebugInfo.cpp
    clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
    clang/test/Modules/Inputs/module.map

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h
index 94dd21537966..ac33c7573f35 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -62,6 +62,15 @@ struct ASTFileSignature : std::array<uint8_t, 20> {
 
   explicit operator bool() const { return *this != BaseT({{0}}); }
 
+  /// Returns the value truncated to the size of an uint64_t.
+  uint64_t truncatedValue() const {
+    uint64_t Value = 0;
+    static_assert(sizeof(*this) >= sizeof(uint64_t), "No need to truncate.");
+    for (unsigned I = 0; I < sizeof(uint64_t); ++I)
+      Value |= static_cast<uint64_t>((*this)[I]) << (I * 8);
+    return Value;
+  }
+
   static ASTFileSignature create(StringRef Bytes) {
     return create(Bytes.bytes_begin(), Bytes.bytes_end());
   }

diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 2faf944d07d1..e3442ecd4bd5 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2545,12 +2545,11 @@ llvm::DIModule *CGDebugInfo::getOrCreateModuleRef(ASTSourceDescriptor Mod,
     // We use the lower 64 bits for debug info.
 
     uint64_t Signature = 0;
-    if (const auto &ModSig = Mod.getSignature()) {
-      for (unsigned I = 0; I != sizeof(Signature); ++I)
-        Signature |= (uint64_t)ModSig[I] << (I * 8);
-    } else {
+    if (const auto &ModSig = Mod.getSignature())
+      Signature = ModSig.truncatedValue();
+    else
       Signature = ~1ULL;
-    }
+
     llvm::DIBuilder DIB(CGM.getModule());
     SmallString<0> PCM;
     if (!llvm::sys::path::is_absolute(Mod.getASTFile()))

diff  --git a/clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp b/clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
index 0c7e5f4598f8..04bd6680e31c 100644
--- a/clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
+++ b/clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
@@ -250,10 +250,10 @@ class PCHContainerGenerator : public ASTConsumer {
     // PCH files don't have a signature field in the control block,
     // but LLVM detects DWO CUs by looking for a non-zero DWO id.
     // We use the lower 64 bits for debug info.
+
     uint64_t Signature =
-        Buffer->Signature
-            ? (uint64_t)Buffer->Signature[1] << 32 | Buffer->Signature[0]
-            : ~1ULL;
+        Buffer->Signature ? Buffer->Signature.truncatedValue() : ~1ULL;
+
     Builder->getModuleDebugInfo()->setDwoId(Signature);
 
     // Finalize the Builder.

diff  --git a/clang/test/Modules/Inputs/DebugDwoId.h b/clang/test/Modules/Inputs/DebugDwoId.h
new file mode 100644
index 000000000000..242e4c7f5116
--- /dev/null
+++ b/clang/test/Modules/Inputs/DebugDwoId.h
@@ -0,0 +1,4 @@
+#ifndef DEBUG_DWO_ID_H
+#define DEBUG_DWO_ID_H
+struct Dummy {};
+#endif

diff  --git a/clang/test/Modules/Inputs/module.map b/clang/test/Modules/Inputs/module.map
index ed220e667f05..e7cb4b27bc08 100644
--- a/clang/test/Modules/Inputs/module.map
+++ b/clang/test/Modules/Inputs/module.map
@@ -357,6 +357,10 @@ module DebugObjCImport {
   }
 }
 
+module DebugDwoId {
+  header "DebugDwoId.h"
+}
+
 module ImportNameInDir {
   header "ImportNameInDir.h"
   export *

diff  --git a/clang/test/Modules/ModuleDebugInfoDwoId.cpp b/clang/test/Modules/ModuleDebugInfoDwoId.cpp
new file mode 100644
index 000000000000..7a450c2580ea
--- /dev/null
+++ b/clang/test/Modules/ModuleDebugInfoDwoId.cpp
@@ -0,0 +1,22 @@
+// Tests that dwoIds in modules match the dwoIDs in the main file.
+
+// RUN: rm -rf %t.cache
+// RUN: %clang_cc1 -triple %itanium_abi_triple -x objective-c++ -std=c++11 -debugger-tuning=lldb -debug-info-kind=limited -fmodules -fmodule-format=obj -dwarf-ext-refs -fimplicit-module-maps -fmodules-cache-path=%t.cache %s -I %S/Inputs -emit-llvm -o %t.ll -mllvm -debug-only=pchcontainer 2> %t.mod-out
+// RUN: cat %t.ll > %t.combined_output
+// RUN: cat %t.mod-out >> %t.combined_output
+// RUN: cat %t.combined_output | FileCheck %s
+// RUN: cat %t.ll | FileCheck --check-prefix=CHECK-REALIDS %s
+// RUN: cat %t.mod-out | FileCheck --check-prefix=CHECK-REALIDS %s
+
+ at import DebugDwoId;
+
+Dummy d;
+
+// Find the emitted dwoID for DebugInfoId and compare it against the one in the PCM.
+// CHECK: DebugDwoId-{{[A-Z0-9]+}}.pcm
+// CHECK-SAME: dwoId: [[DWOID:[0-9]+]]
+// CHECK: dwoId: [[DWOID]]
+// CHECK-NEXT: !DIFile(filename: "DebugDwoId"
+
+// Make sure the dwo IDs are real IDs and not fallback values (~1ULL).
+// CHECK-REALIDS-NOT: dwoId: 18446744073709551615


        


More information about the cfe-commits mailing list