[lld] r197307 - [PECOFF] Export undecorated symbols from DLL.
Rui Ueyama
ruiu at google.com
Wed Dec 18 23:45:52 PST 2013
I removed the use of undecorateSymbol() from EdataPass.cpp in r197364 and
instead introduced TableEntry to have undecorated and decorated name. It
eliminated the possibility of accidental over-stripping. I will delete
undecorateSymbol() function as it's now dead-code.
On Sun, Dec 15, 2013 at 4:59 PM, Saleem Abdulrasool
<compnerd at compnerd.org>wrote:
> On Sat, Dec 14, 2013 at 11:36 PM, Rui Ueyama <ruiu at google.com> wrote:
>
>> On Sun, Dec 15, 2013 at 3:27 PM, Saleem Abdulrasool <
>> compnerd at compnerd.org> wrote:
>>
>>> On Fri, Dec 13, 2013 at 8:32 PM, Rui Ueyama <ruiu at google.com> wrote:
>>>
>>>> Author: ruiu
>>>> Date: Fri Dec 13 22:32:29 2013
>>>> New Revision: 197307
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=197307&view=rev
>>>> Log:
>>>> [PECOFF] Export undecorated symbols from DLL.
>>>>
>>>> Symbol names exported from a DLL should be undecorated, not prefixed by
>>>> an underscore ones.
>>>>
>>>> Modified:
>>>> lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h
>>>> lld/trunk/lib/ReaderWriter/PECOFF/EdataPass.cpp
>>>> lld/trunk/lib/ReaderWriter/PECOFF/EdataPass.h
>>>> lld/trunk/lib/ReaderWriter/PECOFF/PECOFFLinkingContext.cpp
>>>> 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=197307&r1=197306&r2=197307&view=diff
>>>>
>>>> ==============================================================================
>>>> --- lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h (original)
>>>> +++ lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h Fri Dec
>>>> 13 22:32:29 2013
>>>> @@ -99,16 +99,8 @@ public:
>>>>
>>>> StringRef searchLibraryFile(StringRef path) const;
>>>>
>>>> - /// Returns the decorated name of the given symbol name. On 32-bit
>>>> x86, it
>>>> - /// adds "_" at the beginning of the string. On other architectures,
>>>> the
>>>> - /// return value is the same as the argument.
>>>> - StringRef decorateSymbol(StringRef name) const {
>>>> - if (_machineType != llvm::COFF::IMAGE_FILE_MACHINE_I386)
>>>> - return name;
>>>> - std::string str = "_";
>>>> - str.append(name);
>>>> - return allocate(str);
>>>> - }
>>>> + StringRef decorateSymbol(StringRef name) const;
>>>> + StringRef undecorateSymbol(StringRef name) const;
>>>>
>>>> void setEntrySymbolName(StringRef name) {
>>>> if (!name.empty())
>>>>
>>>> Modified: lld/trunk/lib/ReaderWriter/PECOFF/EdataPass.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/PECOFF/EdataPass.cpp?rev=197307&r1=197306&r2=197307&view=diff
>>>>
>>>> ==============================================================================
>>>> --- lld/trunk/lib/ReaderWriter/PECOFF/EdataPass.cpp (original)
>>>> +++ lld/trunk/lib/ReaderWriter/PECOFF/EdataPass.cpp Fri Dec 13 22:32:29
>>>> 2013
>>>> @@ -62,15 +62,16 @@ EdataPass::createAddressTable(const std:
>>>> }
>>>>
>>>> edata::EdataAtom *
>>>> -EdataPass::createNamePointerTable(const std::vector<const DefinedAtom
>>>> *> &atoms,
>>>> +EdataPass::createNamePointerTable(const PECOFFLinkingContext &ctx,
>>>> + const std::vector<const DefinedAtom
>>>> *> &atoms,
>>>> MutableFile *file) {
>>>> EdataAtom *table =
>>>> new (_alloc) EdataAtom(_file, sizeof(uint32_t) * atoms.size());
>>>>
>>>> size_t offset = 0;
>>>> for (const DefinedAtom *atom : atoms) {
>>>> - COFFStringAtom *stringAtom = new (_alloc)
>>>> - COFFStringAtom(_file, _stringOrdinal++, ".edata",
>>>> atom->name());
>>>> + auto *stringAtom = new (_alloc) COFFStringAtom(
>>>> + _file, _stringOrdinal++, ".edata",
>>>> ctx.undecorateSymbol(atom->name()));
>>>> file->addAtom(*stringAtom);
>>>> addDir32NBReloc(table, stringAtom, offset);
>>>> offset += sizeof(uint32_t);
>>>> @@ -121,7 +122,7 @@ void EdataPass::perform(std::unique_ptr<
>>>> addDir32NBReloc(table, addressTable,
>>>> offsetof(export_directory_table_entry,
>>>>
>>>> ExportAddressTableRVA));
>>>>
>>>> - EdataAtom *namePointerTable = createNamePointerTable(atoms,
>>>> file.get());
>>>> + EdataAtom *namePointerTable = createNamePointerTable(_ctx, atoms,
>>>> file.get());
>>>> file->addAtom(*namePointerTable);
>>>> addDir32NBReloc(table, namePointerTable,
>>>> offsetof(export_directory_table_entry,
>>>> NamePointerRVA));
>>>>
>>>> Modified: lld/trunk/lib/ReaderWriter/PECOFF/EdataPass.h
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/PECOFF/EdataPass.h?rev=197307&r1=197306&r2=197307&view=diff
>>>>
>>>> ==============================================================================
>>>> --- lld/trunk/lib/ReaderWriter/PECOFF/EdataPass.h (original)
>>>> +++ lld/trunk/lib/ReaderWriter/PECOFF/EdataPass.h Fri Dec 13 22:32:29
>>>> 2013
>>>> @@ -66,7 +66,8 @@ private:
>>>> edata::EdataAtom *
>>>> createAddressTable(const std::vector<const DefinedAtom *> &atoms);
>>>> edata::EdataAtom *
>>>> - createNamePointerTable(const std::vector<const DefinedAtom *> &atoms,
>>>> + createNamePointerTable(const PECOFFLinkingContext &ctx,
>>>> + const std::vector<const DefinedAtom *> &atoms,
>>>> MutableFile *file);
>>>> edata::EdataAtom *
>>>> createOrdinalTable(const std::vector<const DefinedAtom *> &atoms);
>>>>
>>>> Modified: lld/trunk/lib/ReaderWriter/PECOFF/PECOFFLinkingContext.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/PECOFF/PECOFFLinkingContext.cpp?rev=197307&r1=197306&r2=197307&view=diff
>>>>
>>>> ==============================================================================
>>>> --- lld/trunk/lib/ReaderWriter/PECOFF/PECOFFLinkingContext.cpp
>>>> (original)
>>>> +++ lld/trunk/lib/ReaderWriter/PECOFF/PECOFFLinkingContext.cpp Fri Dec
>>>> 13 22:32:29 2013
>>>> @@ -183,6 +183,24 @@ StringRef PECOFFLinkingContext::searchLi
>>>> return filename;
>>>> }
>>>>
>>>> +/// Returns the decorated name of the given symbol name. On 32-bit
>>>> x86, it
>>>> +/// adds "_" at the beginning of the string. On other architectures,
>>>> the
>>>> +/// return value is the same as the argument.
>>>> +StringRef PECOFFLinkingContext::decorateSymbol(StringRef name) const {
>>>> + if (_machineType != llvm::COFF::IMAGE_FILE_MACHINE_I386)
>>>> + return name;
>>>> + std::string str = "_";
>>>> + str.append(name);
>>>> + return allocate(str);
>>>> +}
>>>> +
>>>> +StringRef PECOFFLinkingContext::undecorateSymbol(StringRef name) const
>>>> {
>>>> + if (_machineType != llvm::COFF::IMAGE_FILE_MACHINE_I386)
>>>> + return name;
>>>> + assert(name.startswith("_"));
>>>> + return name.substr(1);
>>>> +}
>>>>
>>>>
>>> While this is entirely correct, Im left wondering if this can cause a
>>> subtle bug in the export table. Assume that I have a symbol call _symbol.
>>> When decorated on x86, it would be __symbol. However, if undecorateSymbol
>>> is invoked on this symbol, it would happily mangle _symbol to symbol,
>>> resulting in an invalid export. Do you think a class to wrap a StringRef
>>> and a flag set would be too expensive to avoid a situation like this?
>>>
>>
>> When the control reached that code, it's guaranteed that an underscore
>> has always been added to a symbol, so it wouldn't happen that *_symbol*would accidentally be mangled to
>> *symbol*.
>>
>
> Right. I was thinking more about an accidental incorrect usage (or did I
> miss something and is it not possible to accidentally pass the undecorated
> symbol to it?). I was merely thinking out loud about the idea of enforcing
> that via a wrapper since I couldn't think of a quick way of enforcing that
> via the type system.
>
>
>>
>>>
>>>> Writer &PECOFFLinkingContext::writer() const { return *_writer; }
>>>>
>>>> ErrorOr<Reference::Kind>
>>>>
>>>> Modified: lld/trunk/test/pecoff/export.test
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/lld/trunk/test/pecoff/export.test?rev=197307&r1=197306&r2=197307&view=diff
>>>>
>>>> ==============================================================================
>>>> --- lld/trunk/test/pecoff/export.test (original)
>>>> +++ lld/trunk/test/pecoff/export.test Fri Dec 13 22:32:29 2013
>>>> @@ -8,7 +8,7 @@ CHECK: Contents of section .edata:
>>>> CHECK-NEXT: 1000 00000000 {{........}} 00000000 3c100000
>>>> CHECK-NEXT: 1010 01000000 02000000 02000000 28100000
>>>> CHECK-NEXT: 1020 30100000 38100000 08200000 10200000
>>>> -CHECK-NEXT: 1030 50100000 5b100000 00000100 6578706f
>>>> +CHECK-NEXT: 1030 50100000 5a100000 00000100 6578706f
>>>> CHECK-NEXT: 1040 72742e74 6573742e 746d702e 646c6c00
>>>> -CHECK-NEXT: 1050 5f657870 6f727466 6e31005f 6578706f
>>>> -CHECK-NEXT: 1060 7274666e 3200
>>>> +CHECK-NEXT: 1050 6578706f 7274666e 31006578 706f7274
>>>> +CHECK-NEXT: 1060 666e3200
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>
>>>
>>>
>>>
>>> --
>>> Saleem Abdulrasool
>>> compnerd (at) compnerd (dot) org
>>>
>>
>>
>
>
> --
> Saleem Abdulrasool
> compnerd (at) compnerd (dot) org
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131219/9fbb3771/attachment.html>
More information about the llvm-commits
mailing list