[lld] r223341 - [PECOFF] Improve /export compatibility.
Rui Ueyama
ruiu at google.com
Thu Dec 4 15:25:18 PST 2014
MSVC link.exe actually prints out an interesting fatal error message like
this for _foo and _foo at 4.
LINK : warning LNK4022: cannot find unique match for symbol 'foo'
LINK : warning LNK4002: _foo defined in 2.obj
LINK : warning LNK4002: _foo at 4 defined in 1.obj
LINK : error LNK2001: unresolved external symbol foo
On Wed, Dec 3, 2014 at 10:18 PM, David Majnemer <david.majnemer at gmail.com>
wrote:
> What happens if the object file contains 'foo' and '_foo at 8' and you
> /export:foo?
>
> On Wed, Dec 3, 2014 at 10:09 PM, Rui Ueyama <ruiu at google.com> wrote:
>
>> Author: ruiu
>> Date: Thu Dec 4 00:09:39 2014
>> New Revision: 223341
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=223341&view=rev
>> Log:
>> [PECOFF] Improve /export compatibility.
>>
>> Looks like the rule of /export is more complicated than
>> I was thinking. If /export:foo, for example, is given, and
>> if the actual symbol name in an object file is _foo@<number>,
>> we need to export that symbol as foo, not as the mangled name.
>>
>> If only /export:_foo@<number> is given, the symbol is exported
>> as _foo@<number>.
>>
>> If both /export:foo and /export:_foo@<number> are given,
>> they are considered as duplicates, and the linker needs to
>> choose the unmangled name.
>>
>> The basic idea seems that the linker needs to export a symbol
>> with the same name as given as /export.
>>
>> We exported mangled symbols. This patch fixes that issue.
>>
>> Modified:
>> lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h
>> lld/trunk/lib/ReaderWriter/PECOFF/EdataPass.cpp
>> lld/trunk/lib/ReaderWriter/PECOFF/LinkerGeneratedSymbolFile.h
>> lld/trunk/test/pecoff/export.test
>>
>> Modified: lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h?rev=223341&r1=223340&r2=223341&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h (original)
>> +++ lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h Thu Dec 4
>> 00:09:39 2014
>> @@ -65,14 +65,17 @@ public:
>> return getExternalName().compare(other.getExternalName()) < 0;
>> }
>>
>> + StringRef getRealName() const {
>> + return mangledName.empty() ? name : mangledName;
>> + }
>> +
>> StringRef getExternalName() const {
>> - if (!externalName.empty())
>> - return externalName;
>> - return name;
>> + return externalName.empty() ? name : externalName;
>> }
>>
>> std::string name;
>> std::string externalName;
>> + std::string mangledName;
>> int ordinal;
>> bool noname;
>> bool isData;
>>
>> Modified: lld/trunk/lib/ReaderWriter/PECOFF/EdataPass.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/PECOFF/EdataPass.cpp?rev=223341&r1=223340&r2=223341&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/lib/ReaderWriter/PECOFF/EdataPass.cpp (original)
>> +++ lld/trunk/lib/ReaderWriter/PECOFF/EdataPass.cpp Thu Dec 4 00:09:39
>> 2014
>> @@ -26,20 +26,54 @@ using llvm::object::export_directory_tab
>> namespace lld {
>> namespace pecoff {
>>
>> +typedef PECOFFLinkingContext::ExportDesc ExportDesc;
>> +
>> +// dedupExports removes duplicate export entries. If two exports are
>> +// referring the same symbol, they are considered duplicates.
>> +// This could happen if the same symbol name is specified as an argument
>> +// to /export more than once, or an unmangled and mangled name of the
>> +// same symbol are given to /export. In the latter case, we choose
>> +// unmangled (shorter) name.
>> +static void dedupExports(PECOFFLinkingContext &ctx) {
>> + std::vector<ExportDesc> &exports = ctx.getDllExports();
>> + // Pass 1: find duplicate entries
>> + std::set<const ExportDesc *> dup;
>> + std::map<StringRef, ExportDesc *> map;
>> + for (ExportDesc &exp : exports) {
>> + if (!exp.externalName.empty())
>> + continue;;
>>
>
> Extra semi-colon?
>
>
>> + StringRef symbol = exp.getRealName();
>> + auto it = map.find(symbol);
>> + if (it == map.end()) {
>> + map[symbol] = &exp;
>> + } else if (symbol.size() < it->second->getRealName().size()) {
>> + map[symbol] = &exp;
>> + dup.insert(it->second);
>> + } else {
>> + dup.insert(&exp);
>> + }
>> + }
>> + // Pass 2: remove duplicate entries
>> + auto pred = [&](const ExportDesc &exp) {
>> + return dup.count(&exp) == 1;
>> + };
>> + exports.erase(std::remove_if(exports.begin(), exports.end(), pred),
>> + exports.end());
>> +}
>> +
>> static void assignOrdinals(PECOFFLinkingContext &ctx) {
>> - std::vector<PECOFFLinkingContext::ExportDesc> &exports =
>> ctx.getDllExports();
>> + std::vector<ExportDesc> &exports = ctx.getDllExports();
>> int maxOrdinal = -1;
>> - for (PECOFFLinkingContext::ExportDesc &desc : exports)
>> + for (ExportDesc &desc : exports)
>> maxOrdinal = std::max(maxOrdinal, desc.ordinal);
>>
>> std::sort(exports.begin(), exports.end(),
>> - [](const PECOFFLinkingContext::ExportDesc &a,
>> - const PECOFFLinkingContext::ExportDesc &b) {
>> + [](const ExportDesc &a, const ExportDesc &b) {
>> return a.getExternalName().compare(b.getExternalName()) < 0;
>> });
>>
>> int nextOrdinal = (maxOrdinal == -1) ? 1 : (maxOrdinal + 1);
>> - for (PECOFFLinkingContext::ExportDesc &desc : exports)
>> + for (ExportDesc &desc : exports)
>> if (desc.ordinal == -1)
>> desc.ordinal = nextOrdinal++;
>> }
>> @@ -51,7 +85,7 @@ static bool getExportedAtoms(PECOFFLinki
>> definedAtoms[atom->name()] = atom;
>>
>> for (PECOFFLinkingContext::ExportDesc &desc : ctx.getDllExports()) {
>> - auto it = definedAtoms.find(desc.name);
>> + auto it = definedAtoms.find(desc.getRealName());
>> if (it == definedAtoms.end()) {
>> llvm::errs() << "Symbol <" << desc.name
>> << "> is exported but not defined.\n";
>> @@ -142,6 +176,7 @@ EdataPass::createOrdinalTable(const std:
>> }
>>
>> void EdataPass::perform(std::unique_ptr<MutableFile> &file) {
>> + dedupExports(_ctx);
>> assignOrdinals(_ctx);
>>
>> std::vector<TableEntry> entries;
>>
>> Modified: lld/trunk/lib/ReaderWriter/PECOFF/LinkerGeneratedSymbolFile.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/PECOFF/LinkerGeneratedSymbolFile.h?rev=223341&r1=223340&r2=223341&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/lib/ReaderWriter/PECOFF/LinkerGeneratedSymbolFile.h
>> (original)
>> +++ lld/trunk/lib/ReaderWriter/PECOFF/LinkerGeneratedSymbolFile.h Thu
>> Dec 4 00:09:39 2014
>> @@ -261,29 +261,11 @@ public:
>> if (!findDecoratedSymbol(_ctx, _syms.get(), sym.str(), replace))
>> return nullptr;
>>
>> - // We found a decorated symbol. There may be another symbol that
>> - // has the same decorated name. If that's the case, we remove the
>> - // duplicate item.
>> - std::vector<ExportDesc> &exp = _ctx->getDllExports();
>> - auto isFound = std::find_if(
>> - exp.begin(), exp.end(),
>> - [&](ExportDesc &e) { return e.getExternalName().equals(replace);
>> });
>> - if (isFound != exp.end()) {
>> - exp.erase(
>> - std::remove_if(exp.begin(), exp.end(),
>> - [&](ExportDesc &e) { return e.name == sym; }),
>> - exp.end());
>> - } else {
>> - for (ExportDesc &e : exp) {
>> - if (e.name == sym) {
>> - e.name = replace;
>> - break;
>> - }
>> - }
>> - if (_ctx->deadStrip())
>> - _ctx->addDeadStripRoot(_ctx->allocate(replace));
>> - }
>> -
>> + for (ExportDesc &exp : _ctx->getDllExports())
>> + if (exp.name == sym)
>> + exp.mangledName = replace;
>> + if (_ctx->deadStrip())
>> + _ctx->addDeadStripRoot(_ctx->allocate(replace));
>> return new (_alloc) impl::SymbolRenameFile(sym, replace);
>> }
>>
>>
>> Modified: lld/trunk/test/pecoff/export.test
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/test/pecoff/export.test?rev=223341&r1=223340&r2=223341&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/test/pecoff/export.test (original)
>> +++ lld/trunk/test/pecoff/export.test Thu Dec 4 00:09:39 2014
>> @@ -59,30 +59,31 @@ CHECK5-NEXT: 2 0x2010 exportfn
>> CHECK6: Export Table:
>> CHECK6: DLL name: export.test.tmp6.dll
>> CHECK6: Ordinal RVA Name
>> -CHECK6-NEXT: 1 0x2010 ?exportfn8@@YAXXZ
>> +CHECK6-NEXT: 1 0x2010 exportfn3 at 256
>> +CHECK6-NEXT: 2 0x2010 exportfn8
>>
>> -# RUN: lld -flavor link /out:%t6.dll /dll /entry:init \
>> +# RUN: lld -flavor link /out:%t7.dll /dll /entry:init \
>> # RUN: /export:exportfn7 /export:exportfn7 at 8 \
>> # RUN: /export:exportfn8 /export:exportfn8 /export:exportfn3 -- %t.obj
>> -# RUN: llvm-objdump -p %t6.dll | FileCheck -check-prefix=DUP %s
>> +# RUN: llvm-objdump -p %t7.dll | FileCheck -check-prefix=DUP %s
>>
>> DUP: Export Table:
>> -DUP: DLL name: export.test.tmp6.dll
>> +DUP: DLL name: export.test.tmp7.dll
>> DUP: Ordinal RVA Name
>> -DUP-NEXT: 1 0x2010 ?exportfn8@@YAXXZ
>> -DUP-NEXT: 2 0x2010 exportfn3 at 256
>> -DUP-NEXT: 3 0x2010 exportfn7 at 8
>> +DUP-NEXT: 1 0x2010 exportfn3
>> +DUP-NEXT: 2 0x2010 exportfn7
>> +DUP-NEXT: 3 0x2010 exportfn8
>> DUP-NOT: ?exportfn8@@YAXXZ
>> DUP-NOT: exportfn3 at 256
>>
>> # RUN: yaml2obj %p/Inputs/export.obj.yaml > %t.obj
>> #
>> -# RUN: lld -flavor link /out:%t1.dll /dll /entry:init \
>> +# RUN: lld -flavor link /out:%t8.dll /dll /entry:init \
>> # RUN: /export:f1=exportfn1 /export:f2 at 4=exportfn2,private -- %t.obj
>> -# RUN: llvm-objdump -p %t1.dll | FileCheck -check-prefix=EQUAL %s
>> +# RUN: llvm-objdump -p %t8.dll | FileCheck -check-prefix=EQUAL %s
>>
>> EQUAL: Export Table:
>> -EQUAL: DLL name: export.test.tmp1.dll
>> +EQUAL: DLL name: export.test.tmp8.dll
>> EQUAL: Ordinal RVA Name
>> EQUAL-NEXT: 1 0x2010 exportfn3 at 256
>> EQUAL-NEXT: 2 0x2008 f1
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141204/625c646a/attachment.html>
More information about the llvm-commits
mailing list