From a4c3ed42ba5625af54254584d762ebf96cc06942 Mon Sep 17 00:00:00 2001 From: Raphael Isemann Date: Fri, 21 Aug 2020 14:21:21 +0200 Subject: [PATCH] Correctly emit dwoIDs after ASTFileSignature refactoring (D81347) 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` instead of `std::array`). ``` 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 --- clang/include/clang/Basic/Module.h | 9 +++++++++ clang/lib/CodeGen/CGDebugInfo.cpp | 9 ++++----- .../lib/CodeGen/ObjectFilePCHContainerOperations.cpp | 6 +++--- clang/test/Modules/Inputs/DebugDwoId.h | 4 ++++ clang/test/Modules/Inputs/module.map | 4 ++++ clang/test/Modules/ModuleDebugInfoDwoId.cpp | 20 ++++++++++++++++++++ 6 files changed, 44 insertions(+), 8 deletions(-) create mode 100644 clang/test/Modules/Inputs/DebugDwoId.h create mode 100644 clang/test/Modules/ModuleDebugInfoDwoId.cpp diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h index 94dd215..ac33c75 100644 --- a/clang/include/clang/Basic/Module.h +++ b/clang/include/clang/Basic/Module.h @@ -62,6 +62,15 @@ struct ASTFileSignature : std::array { 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((*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 2faf944..e3442ecd 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 0c7e5f4..04bd668 100644 --- a/clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp +++ b/clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp @@ -250,10 +250,10 @@ public: // 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 0000000..242e4c7 --- /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 ed220e6..e7cb4b2 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 0000000..566db04 --- /dev/null +++ b/clang/test/Modules/ModuleDebugInfoDwoId.cpp @@ -0,0 +1,20 @@ +// 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 &> %t.mod-out +// RUN: cat %t.ll %t.mod-out | FileCheck %s +// RUN: cat %t.ll | FileCheck --check-prefix=CHECK-REALIDS %s +// RUN: cat %t.mod-out | FileCheck --check-prefix=CHECK-REALIDS %s + +@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 -- 2.7.4