[llvm] dcf376a - [DebugInfo] Make DISubprogram's hashing always produce the same result (#90770)
via llvm-commits
llvm-commits at lists.llvm.org
Thu May 2 12:15:01 PDT 2024
Author: Augusto Noronha
Date: 2024-05-02T12:14:57-07:00
New Revision: dcf376aae738252fb52a73bcf7f58fd030e15ee2
URL: https://github.com/llvm/llvm-project/commit/dcf376aae738252fb52a73bcf7f58fd030e15ee2
DIFF: https://github.com/llvm/llvm-project/commit/dcf376aae738252fb52a73bcf7f58fd030e15ee2.diff
LOG: [DebugInfo] Make DISubprogram's hashing always produce the same result (#90770)
A DISubprogram's hashing algorithm takes into account its Scope. A Scope
can be a temporary though which can be replaced later on during
compilation. This means that the hashing algorithm for a DISubprogram
could produce a different hash before/after the Scope has changed. Fix
this by checking the Scope's linkage name instead, which should always
be the same.
rdar://127004707
Added:
Modified:
llvm/lib/IR/LLVMContextImpl.h
llvm/unittests/IR/DebugInfoTest.cpp
Removed:
################################################################################
diff --git a/llvm/lib/IR/LLVMContextImpl.h b/llvm/lib/IR/LLVMContextImpl.h
index 2713015c266c7e..399fe0dad26c73 100644
--- a/llvm/lib/IR/LLVMContextImpl.h
+++ b/llvm/lib/IR/LLVMContextImpl.h
@@ -825,19 +825,26 @@ template <> struct MDNodeKeyImpl<DISubprogram> {
bool isDefinition() const { return SPFlags & DISubprogram::SPFlagDefinition; }
unsigned getHashValue() const {
+ // Use the Scope's linkage name instead of using the scope directly, as the
+ // scope may be a temporary one which can replaced, which would produce a
+ //
diff erent hash for the same DISubprogram.
+ llvm::StringRef ScopeLinkageName;
+ if (auto *CT = dyn_cast_or_null<DICompositeType>(Scope))
+ if (auto *ID = CT->getRawIdentifier())
+ ScopeLinkageName = ID->getString();
+
// If this is a declaration inside an ODR type, only hash the type and the
// name. Otherwise the hash will be stronger than
// MDNodeSubsetEqualImpl::isDeclarationOfODRMember().
- if (!isDefinition() && LinkageName)
- if (auto *CT = dyn_cast_or_null<DICompositeType>(Scope))
- if (CT->getRawIdentifier())
- return hash_combine(LinkageName, Scope);
+ if (!isDefinition() && LinkageName &&
+ isa_and_nonnull<DICompositeType>(Scope))
+ return hash_combine(LinkageName, ScopeLinkageName);
// Intentionally computes the hash on a subset of the operands for
// performance reason. The subset has to be significant enough to avoid
// collision "most of the time". There is no correctness issue in case of
// collision because of the full check above.
- return hash_combine(Name, Scope, File, Type, Line);
+ return hash_combine(Name, ScopeLinkageName, File, Type, Line);
}
};
diff --git a/llvm/unittests/IR/DebugInfoTest.cpp b/llvm/unittests/IR/DebugInfoTest.cpp
index d06b979bf4a1c4..85372874f1a78e 100644
--- a/llvm/unittests/IR/DebugInfoTest.cpp
+++ b/llvm/unittests/IR/DebugInfoTest.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "llvm/IR/DebugInfo.h"
+#include "../lib/IR/LLVMContextImpl.h"
#include "llvm/ADT/APSInt.h"
#include "llvm/AsmParser/Parser.h"
#include "llvm/IR/DIBuilder.h"
@@ -20,6 +21,7 @@
#include "llvm/IR/Verifier.h"
#include "llvm/Support/SourceMgr.h"
#include "llvm/Transforms/Utils/Local.h"
+
#include "gtest/gtest.h"
using namespace llvm;
@@ -349,7 +351,7 @@ TEST(MetadataTest, OrderingOfDbgVariableRecords) {
UseNewDbgInfoFormat = OldDbgValueMode;
}
-TEST(DIBuiler, CreateFile) {
+TEST(DIBuilder, CreateFile) {
LLVMContext Ctx;
std::unique_ptr<Module> M(new Module("MyModule", Ctx));
DIBuilder DIB(*M);
@@ -1184,4 +1186,52 @@ TEST(MetadataTest, DbgVariableRecordConversionRoutines) {
UseNewDbgInfoFormat = false;
}
+// Test that the hashing function for DISubprograms produce the same result
+// after replacing the temporary scope.
+TEST(DIBuilder, HashingDISubprogram) {
+ LLVMContext Ctx;
+ std::unique_ptr<Module> M = std::make_unique<Module>("MyModule", Ctx);
+ DIBuilder DIB(*M);
+
+ DIFile *F = DIB.createFile("main.c", "/");
+ DICompileUnit *CU =
+ DIB.createCompileUnit(dwarf::DW_LANG_C, F, "Test", false, "", 0);
+
+ llvm::TempDIType ForwardDeclaredType =
+ llvm::TempDIType(DIB.createReplaceableCompositeType(
+ llvm::dwarf::DW_TAG_structure_type, "MyType", CU, F, 0, 0, 8, 8, {},
+ "UniqueIdentifier"));
+
+ // The hashing function is
diff erent for declarations and definitions, so
+ // create one of each.
+ DISubprogram *Declaration =
+ DIB.createMethod(ForwardDeclaredType.get(), "MethodName", "LinkageName",
+ F, 0, DIB.createSubroutineType({}));
+
+ DISubprogram *Definition = DIB.createFunction(
+ ForwardDeclaredType.get(), "MethodName", "LinkageName", F, 0,
+ DIB.createSubroutineType({}), 0, DINode::FlagZero,
+ llvm::DISubprogram::SPFlagDefinition, nullptr, Declaration);
+
+ // Produce the hash with the temporary scope.
+ unsigned HashDeclaration =
+ MDNodeKeyImpl<DISubprogram>(Declaration).getHashValue();
+ unsigned HashDefinition =
+ MDNodeKeyImpl<DISubprogram>(Definition).getHashValue();
+
+ // Instantiate the real scope and replace the temporary one with it.
+ DICompositeType *Type = DIB.createStructType(CU, "MyType", F, 0, 8, 8, {}, {},
+ {}, 0, {}, "UniqueIdentifier");
+ DIB.replaceTemporary(std::move(ForwardDeclaredType), Type);
+
+ // Now make sure the hashing is consistent.
+ unsigned HashDeclarationAfter =
+ MDNodeKeyImpl<DISubprogram>(Declaration).getHashValue();
+ unsigned HashDefinitionAfter =
+ MDNodeKeyImpl<DISubprogram>(Definition).getHashValue();
+
+ EXPECT_EQ(HashDeclaration, HashDeclarationAfter);
+ EXPECT_EQ(HashDefinition, HashDefinitionAfter);
+}
+
} // end namespace
More information about the llvm-commits
mailing list