Fixed invalid debug metadata for LexicalBlock in include files. (#6373)

No longer normalizing paths that are baked into `DIFile` (as well as the
debug module file list and PDB source info).

The previous path normalization change introduced a bug in debug info
where instead of generating a normal `DILexicalBlock`, we generated a
`DILexicalBlockFile`. This was because the file paths written into the
`DIFile`'s were normalized, but the file list in `SourceManager` was
left unchanged. `CGDebugInfo::setLocation` checks whether the `DIFile`
path matches the current file, and due to the normalization differences,
assume the file has changes and compensates by generating a new
`DILexicalBlockFile` on the scope stack.

An alternative to this PR would be to change `CGDebugInfo::setLocation`
to normalize the `SourceManager` path before comparing; however, there
may be other unknown code paths that could have similar issues. Another
alternative is to normalize the path list in `SourceManager`; however,
this has other ramifications and require many more drastic changes.

By no longer normalizing the paths written into `DIFile`, we revert to
the behaviour where the paths in `DIFile` come directly from
`SourceManager`, so there is no chance of a mismatch. Furthermore, by
avoiding normalizing any paths that are written to PDBs, we do not bake
any potential normalization bugs into any test content.
This commit is contained in:
Adam Yang 2024-03-04 23:17:21 -08:00 коммит произвёл GitHub
Родитель 37c0487e9f
Коммит 3da718cd61
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: B5690EEEBB952194
7 изменённых файлов: 80 добавлений и 45 удалений

Просмотреть файл

@ -238,17 +238,6 @@ StringRef CGDebugInfo::getClassName(const RecordDecl *RD) {
return internString(Name);
}
// HLSL Change - begin
std::string CGDebugInfo::HLSLNormalizeDbgFileName(StringRef Str) {
// For HLSL, we want to keep the main file name exactly as is. Everything
// else should be formatted in a standard way.
if (CGM.getLangOpts().HLSL) {
return hlsl::NormalizePath(Str);
}
return Str;
}
// HLSL Change - end
llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation Loc) {
if (!Loc.isValid())
// If Location is not valid then use main input file.
@ -272,8 +261,7 @@ llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation Loc) {
}
llvm::DIFile *F =
DBuilder.createFile(HLSLNormalizeDbgFileName(PLoc.getFilename()),
getCurrentDirname()); // HLSL Change
DBuilder.createFile(PLoc.getFilename(), getCurrentDirname());
DIFileCache[fname].reset(F);
return F;

Просмотреть файл

@ -257,11 +257,10 @@ class CGDebugInfo {
// HLSL Change Begins
private:
bool TryCollectHLSLRecordElements(const RecordType *Ty,
llvm::DICompositeType *DITy,
SmallVectorImpl<llvm::Metadata *> &Elements);
std::string HLSLNormalizeDbgFileName(StringRef Str);
bool
TryCollectHLSLRecordElements(const RecordType *Ty,
llvm::DICompositeType *DITy,
SmallVectorImpl<llvm::Metadata *> &Elements);
// HLSL Change Ends
public:

Просмотреть файл

@ -276,7 +276,9 @@ namespace {
end = Ctx.getSourceManager().fileinfo_end();
it != end; ++it) {
if (it->first->isValid() && !it->second->IsSystemFile) {
std::string path = hlsl::NormalizePath(it->first->getName());
llvm::SmallString<256> path = StringRef(it->first->getName());
llvm::sys::path::native(path);
StringRef contentBuffer = it->second->getRawBuffer()->getBuffer();
// If main file, write that to metadata first.
// Add the rest to filesMap to sort by name.
@ -288,7 +290,7 @@ namespace {
// We want the include file paths to match the values passed into
// the include handlers exactly. The SourceManager entries should
// match it except the call to MakeAbsoluteOrCurDirRelative.
filesMap[path] = contentBuffer;
filesMap[path.str()] = contentBuffer;
}
}
}

Просмотреть файл

@ -593,8 +593,6 @@ public:
// Formerly API values.
const char *pUtf8SourceName =
opts.InputFile.empty() ? "hlsl.hlsl" : opts.InputFile.data();
std::string NormalizedSourceName = hlsl::NormalizePath(pUtf8SourceName);
pUtf8SourceName = NormalizedSourceName.c_str();
CA2W pWideSourceName(pUtf8SourceName, CP_UTF8);
const char *pUtf8EntryPoint =

Просмотреть файл

@ -312,19 +312,20 @@ static std::vector<SourceFile> ComputeFileList(clang::CodeGenOptions &cgOpts,
for (auto it = srcMgr.fileinfo_begin(), end = srcMgr.fileinfo_end();
it != end; ++it) {
if (it->first->isValid() && !it->second->IsSystemFile) {
llvm::SmallString<256> Path = llvm::StringRef(it->first->getName());
llvm::sys::path::native(Path);
// If main file, write that to metadata first.
// Add the rest to filesMap to sort by name.
if (cgOpts.MainFileName.compare(it->first->getName()) == 0) {
SourceFile file;
file.Name = hlsl::NormalizePath(it->first->getName());
file.Name = Path.str();
file.Content = it->second->getRawBuffer()->getBuffer();
ret.push_back(file);
assert(!bFoundMainFile &&
"otherwise, more than one file matches main filename");
bFoundMainFile = true;
} else {
std::string NormalizedBuf = hlsl::NormalizePath(it->first->getName());
filesMap[NormalizedBuf] = it->second->getRawBuffer()->getBuffer();
filesMap[Path.str()] = it->second->getRawBuffer()->getBuffer();
}
}
}

Просмотреть файл

@ -68,13 +68,9 @@
#ifdef _WIN32
#define SLASH_W L"\\"
#define SLASH "\\"
#define PP_SLASH_W L"\\\\"
#define PP_SLASH "\\\\"
#else
#define SLASH_W L"/"
#define SLASH "/"
#define PP_SLASH_W L"/"
#define PP_SLASH "/"
#endif
using namespace std;
@ -193,6 +189,7 @@ public:
TEST_METHOD(CompileWhenIncludeSystemThenLoadNotRelative)
TEST_METHOD(CompileWhenAllIncludeCombinations)
TEST_METHOD(TestPdbUtilsPathNormalizations)
TEST_METHOD(CompileWithIncludeThenTestNoLexicalBlockFile)
TEST_METHOD(CompileWhenIncludeSystemMissingThenLoadAttempt)
TEST_METHOD(CompileWhenIncludeFlagsThenIncludeUsed)
TEST_METHOD(CompileThenCheckDisplayIncludeProcess)
@ -2929,6 +2926,57 @@ public:
}
};
TEST_F(CompilerTest, CompileWithIncludeThenTestNoLexicalBlockFile) {
std::string includeFile = R"x(
[RootSignature("")]
float main(uint x : X) : SV_Target {
float ret = 0;
if (x) {
float other_ret = 1;
ret = other_ret;
}
return ret;
}
)x";
std::string mainFile = R"(
#include "include.h"
)";
CComPtr<IDxcCompiler> pCompiler;
CComPtr<IDxcOperationResult> pResult;
CComPtr<IDxcBlobEncoding> pSource;
CComPtr<SimpleIncludeHanlder> pInclude;
VERIFY_SUCCEEDED(CreateCompiler(&pCompiler));
CreateBlobFromText(mainFile.c_str(), &pSource);
pInclude = new SimpleIncludeHanlder(m_dllSupport);
pInclude->Content = includeFile;
pInclude->Path = L"." SLASH_W "include.h";
std::vector<const WCHAR *> args = {L"/Zi", L"/Od"};
VERIFY_SUCCEEDED(pCompiler->Compile(pSource, L"MyShader.hlsl", L"main",
L"ps_6_0", args.data(), args.size(),
nullptr, 0, pInclude, &pResult));
CComPtr<IDxcResult> pRealResult;
VERIFY_SUCCEEDED(pResult.QueryInterface(&pRealResult));
CComPtr<IDxcBlob> pDxil;
VERIFY_SUCCEEDED(
pRealResult->GetOutput(DXC_OUT_OBJECT, IID_PPV_ARGS(&pDxil), nullptr));
CComPtr<IDxcBlobEncoding> pDisasm;
VERIFY_SUCCEEDED(pCompiler->Disassemble(pDxil, &pDisasm));
CComPtr<IDxcBlobUtf8> pDisasmUtf8;
VERIFY_SUCCEEDED(pDisasm.QueryInterface(&pDisasmUtf8));
std::string disasm(pDisasmUtf8->GetStringPointer(),
pDisasmUtf8->GetStringLength());
VERIFY_IS_TRUE(disasm.find("!DILexicalBlock") != std::string::npos);
VERIFY_IS_TRUE(disasm.find("!DILexicalBlockFile") == std::string::npos);
}
TEST_F(CompilerTest, TestPdbUtilsPathNormalizations) {
#include "TestHeaders/TestPdbUtilsPathNormalizations.h"
struct TestCase {
@ -4521,7 +4569,7 @@ TEST_F(CompilerTest, PreprocessWhenValidThenOK) {
CComPtr<IDxcBlob> pOutText;
VERIFY_SUCCEEDED(pResult->GetResult(&pOutText));
std::string text(BlobToUtf8(pOutText));
VERIFY_ARE_EQUAL_STR("#line 1 \"." PP_SLASH "file.hlsl\"\n"
VERIFY_ARE_EQUAL_STR("#line 1 \"file.hlsl\"\n"
"\n"
"int g_int = 123;\n"
"\n"
@ -4568,8 +4616,8 @@ TEST_F(CompilerTest, PreprocessWhenExpandTokenPastingOperandThenAccept) {
CComPtr<IDxcBlob> pOutText;
VERIFY_SUCCEEDED(pResult->GetResult(&pOutText));
std::string text(BlobToUtf8(pOutText));
VERIFY_ARE_EQUAL_STR("#line 1 \"." PP_SLASH "file.hlsl\"\n"
"#line 12 \"." PP_SLASH "file.hlsl\"\n"
VERIFY_ARE_EQUAL_STR("#line 1 \"file.hlsl\"\n"
"#line 12 \"file.hlsl\"\n"
" Texture2D<float4> resource_set_10_bind_5_tex;\n"
"\n"
" float4 main() : SV_Target{\n"
@ -4609,7 +4657,7 @@ TEST_F(CompilerTest, PreprocessWithDebugOptsThenOk) {
CComPtr<IDxcBlob> pOutText;
VERIFY_SUCCEEDED(pResult->GetResult(&pOutText));
std::string text(BlobToUtf8(pOutText));
VERIFY_ARE_EQUAL_STR("#line 1 \"." PP_SLASH "file.hlsl\"\n"
VERIFY_ARE_EQUAL_STR("#line 1 \"file.hlsl\"\n"
"\n"
"int g_int = 123;\n"
"\n"
@ -4636,7 +4684,7 @@ TEST_F(CompilerTest, PreprocessCheckBuiltinIsOk) {
CComPtr<IDxcBlob> pOutText;
VERIFY_SUCCEEDED(pResult->GetResult(&pOutText));
std::string text(BlobToUtf8(pOutText));
VERIFY_ARE_EQUAL_STR("#line 1 \"." PP_SLASH "file.hlsl\"\n\n", text.c_str());
VERIFY_ARE_EQUAL_STR("#line 1 \"file.hlsl\"\n\n", text.c_str());
}
TEST_F(CompilerTest, CompileOtherModesWithDebugOptsThenOk) {

Просмотреть файл

@ -832,8 +832,7 @@ PixDiaTest::GetLiveVariablesAt(const char *hlsl,
++InterestingLine) {
CComPtr<IDxcPixDxilInstructionOffsets> instructionOffsets;
if (SUCCEEDED(dxilDebugger->InstructionOffsetsFromSourceLocation(
(std::wstring(L".\\") + defaultFilename).c_str(), InterestingLine,
0, &instructionOffsets))) {
defaultFilename, InterestingLine, 0, &instructionOffsets))) {
if (instructionOffsets->GetCount() > 0) {
auto instructionOffset = instructionOffsets->GetOffsetByIndex(0);
if (SUCCEEDED(dxilDebugger->GetLiveVariablesAt(instructionOffset,
@ -926,7 +925,7 @@ TEST_F(PixDiaTest, CompileWhenDebugThenDIPresent) {
L"lexicalParent: id=2, value: ps_6_0"));
VERIFY_IS_NOT_NULL(wcsstr(diaDump.c_str(), L"lineNumber: 2"));
VERIFY_IS_NOT_NULL(
wcsstr(diaDump.c_str(), L"length: 99, filename: .\\source.hlsl"));
wcsstr(diaDump.c_str(), L"length: 99, filename: source.hlsl"));
std::wstring diaFileContent = GetDebugFileContent(pDiaSource).c_str();
VERIFY_IS_NOT_NULL(
wcsstr(diaFileContent.c_str(),
@ -1512,8 +1511,7 @@ TEST_F(PixDiaTest, PixDebugCompileInfo) {
CComBSTR entryPointFile;
VERIFY_SUCCEEDED(compilationInfo->GetEntryPointFile(&entryPointFile));
VERIFY_ARE_EQUAL(std::wstring(L".\\source.hlsl"),
std::wstring(entryPointFile));
VERIFY_ARE_EQUAL(std::wstring(L"source.hlsl"), std::wstring(entryPointFile));
CComBSTR entryPointFunction;
VERIFY_SUCCEEDED(compilationInfo->GetEntryPoint(&entryPointFunction));
@ -2029,7 +2027,7 @@ void ASMain()
CComPtr<IDxcPixDxilInstructionOffsets> instructionOffsets;
VERIFY_SUCCEEDED(dxilDebugger->InstructionOffsetsFromSourceLocation(
L".\\source.hlsl", DispatchMeshLine, 0, &instructionOffsets));
L"source.hlsl", DispatchMeshLine, 0, &instructionOffsets));
VERIFY_IS_TRUE(instructionOffsets->GetCount() > 0);
DWORD InstructionOrdinal = instructionOffsets->GetOffsetByIndex(0);
CComPtr<IDxcPixDxilLiveVariables> liveVariables;
@ -2758,21 +2756,22 @@ float4 fn2( float3 f3, float d, bool sanitize = true )
auto it = sourceLocations.begin();
VERIFY_IS_FALSE(it == sourceLocations.end());
const WCHAR *mainFileName = L"source.hlsl";
// The list of source locations should start with the containing file:
while (it != sourceLocations.end() && it->Filename == L".\\source.hlsl")
while (it != sourceLocations.end() && it->Filename == mainFileName)
it++;
VERIFY_IS_FALSE(it == sourceLocations.end());
// Then have a bunch of "../include2/samefilename.h"
VERIFY_ARE_EQUAL_WSTR(L".\\..\\include2\\samefilename.h", it->Filename);
VERIFY_ARE_EQUAL_WSTR(L"./../include2/samefilename.h", it->Filename);
while (it != sourceLocations.end() &&
it->Filename == L".\\..\\include2\\samefilename.h")
it->Filename == L"./../include2/samefilename.h")
it++;
VERIFY_IS_FALSE(it == sourceLocations.end());
// Then some more main file:
VERIFY_ARE_EQUAL_WSTR(L".\\source.hlsl", it->Filename);
while (it != sourceLocations.end() && it->Filename == L".\\source.hlsl")
VERIFY_ARE_EQUAL_WSTR(mainFileName, it->Filename);
while (it != sourceLocations.end() && it->Filename == mainFileName)
it++;
// And that should be the end: