Skip to content

Commit dcf376a

Browse files
authored
[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
1 parent 57216f7 commit dcf376a

File tree

2 files changed

+63
-6
lines changed

2 files changed

+63
-6
lines changed

llvm/lib/IR/LLVMContextImpl.h

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -825,19 +825,26 @@ template <> struct MDNodeKeyImpl<DISubprogram> {
825825
bool isDefinition() const { return SPFlags & DISubprogram::SPFlagDefinition; }
826826

827827
unsigned getHashValue() const {
828+
// Use the Scope's linkage name instead of using the scope directly, as the
829+
// scope may be a temporary one which can replaced, which would produce a
830+
// different hash for the same DISubprogram.
831+
llvm::StringRef ScopeLinkageName;
832+
if (auto *CT = dyn_cast_or_null<DICompositeType>(Scope))
833+
if (auto *ID = CT->getRawIdentifier())
834+
ScopeLinkageName = ID->getString();
835+
828836
// If this is a declaration inside an ODR type, only hash the type and the
829837
// name. Otherwise the hash will be stronger than
830838
// MDNodeSubsetEqualImpl::isDeclarationOfODRMember().
831-
if (!isDefinition() && LinkageName)
832-
if (auto *CT = dyn_cast_or_null<DICompositeType>(Scope))
833-
if (CT->getRawIdentifier())
834-
return hash_combine(LinkageName, Scope);
839+
if (!isDefinition() && LinkageName &&
840+
isa_and_nonnull<DICompositeType>(Scope))
841+
return hash_combine(LinkageName, ScopeLinkageName);
835842

836843
// Intentionally computes the hash on a subset of the operands for
837844
// performance reason. The subset has to be significant enough to avoid
838845
// collision "most of the time". There is no correctness issue in case of
839846
// collision because of the full check above.
840-
return hash_combine(Name, Scope, File, Type, Line);
847+
return hash_combine(Name, ScopeLinkageName, File, Type, Line);
841848
}
842849
};
843850

llvm/unittests/IR/DebugInfoTest.cpp

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "llvm/IR/DebugInfo.h"
10+
#include "../lib/IR/LLVMContextImpl.h"
1011
#include "llvm/ADT/APSInt.h"
1112
#include "llvm/AsmParser/Parser.h"
1213
#include "llvm/IR/DIBuilder.h"
@@ -20,6 +21,7 @@
2021
#include "llvm/IR/Verifier.h"
2122
#include "llvm/Support/SourceMgr.h"
2223
#include "llvm/Transforms/Utils/Local.h"
24+
2325
#include "gtest/gtest.h"
2426

2527
using namespace llvm;
@@ -349,7 +351,7 @@ TEST(MetadataTest, OrderingOfDbgVariableRecords) {
349351
UseNewDbgInfoFormat = OldDbgValueMode;
350352
}
351353

352-
TEST(DIBuiler, CreateFile) {
354+
TEST(DIBuilder, CreateFile) {
353355
LLVMContext Ctx;
354356
std::unique_ptr<Module> M(new Module("MyModule", Ctx));
355357
DIBuilder DIB(*M);
@@ -1184,4 +1186,52 @@ TEST(MetadataTest, DbgVariableRecordConversionRoutines) {
11841186
UseNewDbgInfoFormat = false;
11851187
}
11861188

1189+
// Test that the hashing function for DISubprograms produce the same result
1190+
// after replacing the temporary scope.
1191+
TEST(DIBuilder, HashingDISubprogram) {
1192+
LLVMContext Ctx;
1193+
std::unique_ptr<Module> M = std::make_unique<Module>("MyModule", Ctx);
1194+
DIBuilder DIB(*M);
1195+
1196+
DIFile *F = DIB.createFile("main.c", "/");
1197+
DICompileUnit *CU =
1198+
DIB.createCompileUnit(dwarf::DW_LANG_C, F, "Test", false, "", 0);
1199+
1200+
llvm::TempDIType ForwardDeclaredType =
1201+
llvm::TempDIType(DIB.createReplaceableCompositeType(
1202+
llvm::dwarf::DW_TAG_structure_type, "MyType", CU, F, 0, 0, 8, 8, {},
1203+
"UniqueIdentifier"));
1204+
1205+
// The hashing function is different for declarations and definitions, so
1206+
// create one of each.
1207+
DISubprogram *Declaration =
1208+
DIB.createMethod(ForwardDeclaredType.get(), "MethodName", "LinkageName",
1209+
F, 0, DIB.createSubroutineType({}));
1210+
1211+
DISubprogram *Definition = DIB.createFunction(
1212+
ForwardDeclaredType.get(), "MethodName", "LinkageName", F, 0,
1213+
DIB.createSubroutineType({}), 0, DINode::FlagZero,
1214+
llvm::DISubprogram::SPFlagDefinition, nullptr, Declaration);
1215+
1216+
// Produce the hash with the temporary scope.
1217+
unsigned HashDeclaration =
1218+
MDNodeKeyImpl<DISubprogram>(Declaration).getHashValue();
1219+
unsigned HashDefinition =
1220+
MDNodeKeyImpl<DISubprogram>(Definition).getHashValue();
1221+
1222+
// Instantiate the real scope and replace the temporary one with it.
1223+
DICompositeType *Type = DIB.createStructType(CU, "MyType", F, 0, 8, 8, {}, {},
1224+
{}, 0, {}, "UniqueIdentifier");
1225+
DIB.replaceTemporary(std::move(ForwardDeclaredType), Type);
1226+
1227+
// Now make sure the hashing is consistent.
1228+
unsigned HashDeclarationAfter =
1229+
MDNodeKeyImpl<DISubprogram>(Declaration).getHashValue();
1230+
unsigned HashDefinitionAfter =
1231+
MDNodeKeyImpl<DISubprogram>(Definition).getHashValue();
1232+
1233+
EXPECT_EQ(HashDeclaration, HashDeclarationAfter);
1234+
EXPECT_EQ(HashDefinition, HashDefinitionAfter);
1235+
}
1236+
11871237
} // end namespace

0 commit comments

Comments
 (0)