Skip to content

Commit 9cf29ef

Browse files
committed
linker: detect duplicate exported definitions without imports
The duplicate-export check in GetImportExportPairs() only triggers when iterating over imports. If no module imports a symbol, the linker never looks up the export table for that name, so multiple modules can each export the same function and spvtools::Link() silently accepts them instead of reporting an error. Add a standalone pass over the exports map before the import-matching loop to reject links where the same symbol name is exported by more than one module. This correctly handles LinkOnceODR symbols because they are already deduplicated into a single export entry by the preceding linkonce processing. The existing size > 1 check in the import-matching loop is now redundant and has been removed. Add a test for the case where two modules export the same symbol but neither imports it. Fixes the piglit clLinkProgram test where two modules each define get_number() with Export linkage but neither imports it.
1 parent 113784c commit 9cf29ef

2 files changed

Lines changed: 44 additions & 4 deletions

File tree

source/link/linker.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,17 @@ spv_result_t GetImportExportPairs(const MessageConsumer& consumer,
504504
<< possible_export.second.name << "\".";
505505
}
506506

507+
// Detect multiple strong definitions of the same symbol.
508+
// The import-matching loop below only visits symbols that are imported, so it
509+
// would silently accept multiple exports when nothing imports them. Check
510+
// all exports here so that duplicates are always rejected.
511+
for (const auto& exp : exports) {
512+
if (exp.second.size() > 1u)
513+
return DiagnosticStream(position, consumer, "", SPV_ERROR_INVALID_BINARY)
514+
<< "Too many external references, " << exp.second.size()
515+
<< ", were found for \"" << exp.first << "\".";
516+
}
517+
507518
// Find the import/export pairs
508519
for (const auto& import : imports) {
509520
std::vector<LinkageSymbolInfo> possible_exports;
@@ -512,10 +523,6 @@ spv_result_t GetImportExportPairs(const MessageConsumer& consumer,
512523
if (possible_exports.empty() && !allow_partial_linkage)
513524
return DiagnosticStream(position, consumer, "", SPV_ERROR_INVALID_BINARY)
514525
<< "Unresolved external reference to \"" << import.name << "\".";
515-
else if (possible_exports.size() > 1u)
516-
return DiagnosticStream(position, consumer, "", SPV_ERROR_INVALID_BINARY)
517-
<< "Too many external references, " << possible_exports.size()
518-
<< ", were found for \"" << import.name << "\".";
519526

520527
if (!possible_exports.empty())
521528
linkings_to_do->emplace_back(import, possible_exports.front());

test/link/matching_imports_to_exports_test.cpp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,39 @@ OpDecorate %1 LinkageAttributes "foo" Export
234234
}
235235
}
236236

237+
TEST_F(MatchingImportsToExports, MultipleDefinitionsNoImport) {
238+
// Two modules export the same symbol but nothing imports it.
239+
// The linker must still reject this as a duplicate strong definition.
240+
const std::string body1 = R"(
241+
OpCapability Linkage
242+
OpCapability Addresses
243+
OpCapability Kernel
244+
OpMemoryModel Physical64 OpenCL
245+
OpDecorate %1 LinkageAttributes "foo" Export
246+
%2 = OpTypeFloat 32
247+
%3 = OpConstant %2 42
248+
%1 = OpVariable %2 Uniform %3
249+
)";
250+
const std::string body2 = R"(
251+
OpCapability Linkage
252+
OpCapability Addresses
253+
OpCapability Kernel
254+
OpMemoryModel Physical64 OpenCL
255+
OpDecorate %1 LinkageAttributes "foo" Export
256+
%2 = OpTypeFloat 32
257+
%3 = OpConstant %2 -1
258+
%1 = OpVariable %2 Uniform %3
259+
)";
260+
261+
spvtest::Binary linked_binary;
262+
EXPECT_EQ(SPV_ERROR_INVALID_BINARY,
263+
AssembleAndLink({body1, body2}, &linked_binary))
264+
<< GetErrorMessage();
265+
EXPECT_THAT(GetErrorMessage(),
266+
HasSubstr("Too many external references, 2, were found "
267+
"for \"foo\"."));
268+
}
269+
237270
TEST_F(MatchingImportsToExports, SameNameDifferentTypes) {
238271
const std::string body1 = R"(
239272
OpCapability Linkage

0 commit comments

Comments
 (0)