Skip to content

Commit af0b809

Browse files
authored
Error out if two blobs compiled with different compiler versions are linked. (#5361)
This PR aims to accomplish the final step of the task to prevent blobs compiled with different compiler versions from being linked together. The same compiler is used to compile both blobs in the test, but the compilerversion part in the first blob is forcefully changed to conflict with the compilerversion part within the second blob. The test detects the correct error message, which mentions the names of the two libraries that differ, and what their differing versions are exactly. The PR aims to resolve this issue: #5310
1 parent 1174c88 commit af0b809

2 files changed

Lines changed: 221 additions & 4 deletions

File tree

tools/clang/tools/dxcompiler/dxclinker.cpp

Lines changed: 126 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,48 @@ using namespace llvm;
4444
// This declaration is used for the locally-linked validator.
4545
HRESULT CreateDxcValidator(_In_ REFIID riid, _Out_ LPVOID *ppv);
4646

47+
struct DeserializedDxilCompilerVersion {
48+
const hlsl::DxilCompilerVersion DCV;
49+
std::string commitHashStr;
50+
std::string versionStr;
51+
52+
DeserializedDxilCompilerVersion(const DxilCompilerVersion *pDCV)
53+
: DCV(*pDCV) {
54+
// Assumes pDCV has been checked for safe parsing
55+
if (pDCV->VersionStringListSizeInBytes) {
56+
const char *pStr = (const char *)(pDCV + 1);
57+
commitHashStr.assign(pStr);
58+
if (commitHashStr.size() + 1 < pDCV->VersionStringListSizeInBytes)
59+
versionStr.assign(pStr + commitHashStr.size() + 1);
60+
}
61+
}
62+
63+
DeserializedDxilCompilerVersion(DeserializedDxilCompilerVersion &&other)
64+
: DCV(other.DCV), commitHashStr(std::move(other.commitHashStr)),
65+
versionStr(std::move(other.versionStr)) {}
66+
67+
bool operator<(const DeserializedDxilCompilerVersion &rhs) const {
68+
return std::tie(DCV.Major, DCV.Minor, DCV.CommitCount,
69+
DCV.VersionStringListSizeInBytes, commitHashStr,
70+
versionStr) < std::tie(rhs.DCV.Major, rhs.DCV.Minor,
71+
rhs.DCV.CommitCount,
72+
rhs.DCV.VersionStringListSizeInBytes,
73+
rhs.commitHashStr, rhs.versionStr);
74+
}
75+
76+
std::string display() const {
77+
std::string ret;
78+
ret += "Version(" + std::to_string(DCV.Major) + "." +
79+
std::to_string(DCV.Minor) + ") ";
80+
ret += "commits(" + std::to_string(DCV.CommitCount) + ") ";
81+
ret += "sha(" + commitHashStr + ") ";
82+
ret += "version string: \"" + versionStr + "\"";
83+
ret += "\n";
84+
85+
return ret;
86+
}
87+
};
88+
4789
class DxcLinker : public IDxcLinker, public IDxcContainerEvent {
4890
public:
4991
DXC_MICROCOM_TM_ADDREF_RELEASE_IMPL()
@@ -98,6 +140,30 @@ class DxcLinker : public IDxcLinker, public IDxcContainerEvent {
98140
m_pLinker.reset(DxilLinker::CreateLinker(m_Ctx, valMajor, valMinor));
99141
}
100142

143+
bool AddCompilerVersionMapEntry(LPCWSTR libName,
144+
const hlsl::DxilCompilerVersion *pDCV,
145+
uint32_t partSize) {
146+
// Make sure it's safe to parse pDCV
147+
bool valid = pDCV && partSize >= sizeof(hlsl::DxilCompilerVersion) &&
148+
partSize - sizeof(hlsl::DxilCompilerVersion) >=
149+
pDCV->VersionStringListSizeInBytes;
150+
if (valid && pDCV->VersionStringListSizeInBytes) {
151+
const char *vStr = (const char *)(pDCV + 1);
152+
valid = vStr[pDCV->VersionStringListSizeInBytes - 1] == 0;
153+
}
154+
155+
DXASSERT(valid, "DxilCompilerVersion part malformed");
156+
if (!valid)
157+
return false;
158+
159+
CW2A pUtf8LibName(libName, CP_UTF8);
160+
std::string libNameStr = std::string(pUtf8LibName);
161+
auto result = m_uniqueCompilerVersions.insert(DeserializedDxilCompilerVersion(pDCV));
162+
m_libNameToCompilerVersionPart[libNameStr] = &(*result.first);
163+
164+
return true;
165+
}
166+
101167
~DxcLinker() {
102168
// Make sure DxilLinker is released before LLVMContext.
103169
m_pLinker.reset();
@@ -109,6 +175,8 @@ class DxcLinker : public IDxcLinker, public IDxcContainerEvent {
109175
std::unique_ptr<DxilLinker> m_pLinker;
110176
CComPtr<IDxcContainerEventsHandler> m_pDxcContainerEventsHandler;
111177
std::vector<CComPtr<IDxcBlob>> m_blobs; // Keep blobs live for lazy load.
178+
std::map<std::string, const DeserializedDxilCompilerVersion*> m_libNameToCompilerVersionPart;
179+
std::set<DeserializedDxilCompilerVersion> m_uniqueCompilerVersions;
112180
};
113181

114182
HRESULT
@@ -137,6 +205,22 @@ DxcLinker::RegisterLibrary(_In_opt_ LPCWSTR pLibName, // Name of the library.
137205
pBlob->GetBufferPointer(), pBlob->GetBufferSize(), pModule,
138206
pDebugModule, m_Ctx, m_Ctx, DiagStream));
139207

208+
209+
// add an entry into the library to compiler version part map
210+
const hlsl::DxilContainerHeader *pHeader = hlsl::IsDxilContainerLike(
211+
pBlob->GetBufferPointer(), pBlob->GetBufferSize());
212+
const DxilPartHeader *pDPH = hlsl::GetDxilPartByType(
213+
pHeader, hlsl::DxilFourCC::DFCC_CompilerVersion);
214+
if (pDPH) {
215+
const hlsl::DxilCompilerVersion *pDCV =
216+
(const hlsl::DxilCompilerVersion *)(pDPH + 1);
217+
// If the compiler version string is non-empty, add the struct to the
218+
// map
219+
if (!AddCompilerVersionMapEntry(pLibName, pDCV, pDPH->PartSize)) {
220+
return E_INVALIDARG;
221+
}
222+
}
223+
140224
if (m_pLinker->RegisterLib(pUtf8LibName.m_psz, std::move(pModule),
141225
std::move(pDebugModule))) {
142226
m_blobs.emplace_back(pBlob);
@@ -224,13 +308,53 @@ HRESULT STDMETHODCALLTYPE DxcLinker::Link(
224308

225309
// Attach libraries.
226310
bool bSuccess = true;
227-
for (unsigned i = 0; i < libCount; i++) {
311+
const DeserializedDxilCompilerVersion *cur_version = nullptr;
312+
const DeserializedDxilCompilerVersion *first_version = nullptr;
313+
314+
std::string cur_lib_name;
315+
std::string first_lib_name;
316+
317+
for (UINT32 i = 0; i < libCount; i++) {
228318
CW2A pUtf8LibName(pLibNames[i], CP_UTF8);
229319
bSuccess &= m_pLinker->AttachLib(pUtf8LibName.m_psz);
320+
321+
cur_lib_name = std::string(pUtf8LibName);
322+
323+
// only libraries with compiler version parts are in the map
324+
auto result = m_libNameToCompilerVersionPart.find(cur_lib_name);
325+
326+
if (result != m_libNameToCompilerVersionPart.end()) {
327+
cur_version = result->second;
328+
}
329+
330+
if (i == 0) {
331+
first_lib_name = cur_lib_name;
332+
first_version = cur_version;
333+
}
334+
335+
if (cur_version != first_version) {
336+
337+
std::string errorMsg = "error: Cannot link libraries with "
338+
"conflicting compiler versions.\n";
339+
340+
std::string firstErrorStr =
341+
"note: library \"" + first_lib_name + "\" version: ";
342+
firstErrorStr += first_version ? first_version->display()
343+
: "No version info available";
344+
345+
std::string secondErrorStr =
346+
"note: library \"" + cur_lib_name + "\" version: ";
347+
secondErrorStr +=
348+
cur_version ? cur_version->display() : "No version info available";
349+
350+
errorMsg += firstErrorStr + secondErrorStr;
351+
DiagStream << errorMsg;
352+
bSuccess = false;
353+
}
230354
}
231355

232356
dxilutil::ExportMap exportMap;
233-
bSuccess = exportMap.ParseExports(opts.Exports, DiagStream);
357+
bSuccess &= exportMap.ParseExports(opts.Exports, DiagStream);
234358

235359
if (opts.ExportShadersOnly)
236360
exportMap.setExportShadersOnly(true);

tools/clang/unittests/HLSL/LinkerTest.cpp

Lines changed: 95 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "WexTestClass.h"
2222
#include "dxc/Test/HlslTestUtils.h"
2323
#include "dxc/Test/DxcTestUtils.h"
24+
#include "dxc/Support/Global.h" // for IFT macro
2425
#include "dxc/dxcapi.h"
2526
#include "dxc/DxilContainer/DxilContainer.h"
2627

@@ -40,6 +41,7 @@ class LinkerTest
4041
TEST_CLASS_SETUP(InitSupport);
4142

4243
TEST_METHOD(RunLinkResource);
44+
TEST_METHOD(RunLinkModulesDifferentVersions);
4345
TEST_METHOD(RunLinkResourceWithBinding);
4446
TEST_METHOD(RunLinkAllProfiles);
4547
TEST_METHOD(RunLinkFailNoDefine);
@@ -158,14 +160,14 @@ class LinkerTest
158160

159161
void LinkCheckMsg(LPCWSTR pEntryName, LPCWSTR pShaderModel, IDxcLinker *pLinker,
160162
ArrayRef<LPCWSTR> libNames, llvm::ArrayRef<LPCSTR> pErrorMsgs,
161-
llvm::ArrayRef<LPCWSTR> pArguments = {}) {
163+
llvm::ArrayRef<LPCWSTR> pArguments = {}, bool bRegex=false) {
162164
CComPtr<IDxcOperationResult> pResult;
163165
VERIFY_SUCCEEDED(pLinker->Link(pEntryName, pShaderModel,
164166
libNames.data(), libNames.size(),
165167
pArguments.data(), pArguments.size(),
166168
&pResult));
167169
CheckOperationResultMsgs(pResult, pErrorMsgs.data(), pErrorMsgs.size(),
168-
false, false);
170+
false, bRegex);
169171
}
170172
};
171173

@@ -274,6 +276,97 @@ TEST_F(LinkerTest, RunLinkAllProfiles) {
274276
Link(L"cs_main", L"cs_6_0", pLinker, {libName, libResName}, {},{});
275277
}
276278

279+
TEST_F(LinkerTest, RunLinkModulesDifferentVersions) {
280+
CComPtr<IDxcLinker> pLinker1, pLinker2, pLinker3;
281+
CreateLinker(&pLinker1);
282+
CreateLinker(&pLinker2);
283+
CreateLinker(&pLinker3);
284+
285+
LPCWSTR libName = L"entry";
286+
LPCWSTR option[] = {L"-Zi", L"-Qembed_debug"};
287+
CComPtr<IDxcBlob> pEntryLib;
288+
CompileLib(L"..\\CodeGenHLSL\\lib_entries2.hlsl", &pEntryLib, option);
289+
290+
// the 2nd blob is the "good" blob that stays constant
291+
CComPtr<IDxcBlob> pResLib;
292+
CompileLib(L"..\\CodeGenHLSL\\lib_resource2.hlsl", &pResLib);
293+
LPCWSTR libResName = L"res";
294+
295+
// modify the first compiled blob to force it to have a different compiler
296+
// version
297+
CComPtr<IDxcContainerReflection> containerReflection;
298+
IFT(m_dllSupport.CreateInstance(CLSID_DxcContainerReflection,
299+
&containerReflection));
300+
IFT(containerReflection->Load(pEntryLib));
301+
UINT part_index;
302+
VERIFY_SUCCEEDED(containerReflection->FindFirstPartKind(
303+
hlsl::DFCC_CompilerVersion, &part_index));
304+
CComPtr<IDxcBlob> pBlob;
305+
IFT(containerReflection->GetPartContent(part_index, &pBlob));
306+
void *pBlobPtr = pBlob->GetBufferPointer();
307+
308+
std::string commonMismatchStr =
309+
"error: Cannot link libraries with conflicting compiler versions.";
310+
311+
hlsl::DxilCompilerVersion *pDCV = (hlsl::DxilCompilerVersion *)pBlobPtr;
312+
if (pDCV->VersionStringListSizeInBytes) {
313+
// first just get both strings, the compiler version and the commit hash
314+
char *pVersionStringListPtr =
315+
(char *)pBlobPtr + sizeof(hlsl::DxilCompilerVersion);
316+
std::string commitHashStr(pVersionStringListPtr);
317+
std::string oldCommitHashStr = commitHashStr;
318+
std::string compilerVersionStr(pVersionStringListPtr +
319+
commitHashStr.size() + 1);
320+
std::string oldCompilerVersionStr = compilerVersionStr;
321+
322+
// flip a character to change the commit hash
323+
*pVersionStringListPtr = commitHashStr[0] == 'a' ? 'b' : 'a';
324+
// store the modified compiler version part.
325+
RegisterDxcModule(libName, pEntryLib, pLinker1);
326+
// and the "good" version part
327+
RegisterDxcModule(libResName, pResLib, pLinker1);
328+
// 2 blobs with different compiler versions should not be linkable
329+
LinkCheckMsg(L"hs_main", L"hs_6_1", pLinker1, {libName, libResName},
330+
{commonMismatchStr.c_str()});
331+
332+
// reset the modified part back to normal
333+
*pVersionStringListPtr = oldCommitHashStr[0];
334+
335+
// flip a character to change the compiler version
336+
*(pVersionStringListPtr + commitHashStr.size() + 1) =
337+
compilerVersionStr[0] == '1' ? '2' : '1';
338+
// store the modified compiler version part.
339+
RegisterDxcModule(libName, pEntryLib, pLinker2);
340+
// and the "good" version part
341+
RegisterDxcModule(libResName, pResLib, pLinker2);
342+
// 2 blobs with different compiler versions should not be linkable
343+
LinkCheckMsg(L"hs_main", L"hs_6_1", pLinker2, {libName, libResName},
344+
{commonMismatchStr.c_str()});
345+
346+
// reset the modified part back to normal
347+
*(pVersionStringListPtr + commitHashStr.size() + 1) =
348+
oldCompilerVersionStr[0];
349+
} else {
350+
// The blob can't be changed by adding data, since the
351+
// the data after this header could be a subsequent header.
352+
// The test should announce that it can't make any version string
353+
// modifications
354+
WEX::Logging::Log::Warning(
355+
L"Cannot modify compiler version string for test");
356+
}
357+
358+
// finally, test that a difference is detected if a member of the struct, say
359+
// the major member, is different.
360+
pDCV->Major = pDCV->Major == 2 ? 1 : 2;
361+
// store the modified compiler version part.
362+
RegisterDxcModule(libName, pEntryLib, pLinker3);
363+
// and the "good" version part
364+
RegisterDxcModule(libResName, pResLib, pLinker3);
365+
// 2 blobs with different compiler versions should not be linkable
366+
LinkCheckMsg(L"hs_main", L"hs_6_1", pLinker3, {libName, libResName},
367+
{commonMismatchStr.c_str()});
368+
}
369+
277370
TEST_F(LinkerTest, RunLinkFailNoDefine) {
278371
CComPtr<IDxcBlob> pEntryLib;
279372
CompileLib(L"..\\CodeGenHLSL\\lib_cs_entry.hlsl", &pEntryLib);

0 commit comments

Comments
 (0)