[lld] r193565 - [PECOFF] Fix __ImageBase symbol.

[Rui Ueyama]

On Mon, Oct 28, 2013 at 9:16 PM, Shankar Easwaran
<shankare at codeaurora.org>wrote:

> Hi Rui,
>
> This is an incorrect subclass of SimpleFile. The MemberFile wouldnot
> return an empty vector of absoluteAtoms when absolute() is called.
>

Do you mean that the MemberFile should not return an empty vector of
AbsoluteAtoms, but it will return an empty symbol, and that's the problem?
If so, I'd think the code is not wrong. MemberFile will actually return a
non-empty vector for absolute(), because _imageBase is added by addAtom()
in the MemberFile's initializer.



> I think the right way of doing this is to create a SimpleFile and add an
> AbsoluteAtom to it.

Thanks
>
> Shankar Easwaran
>
>
>
> On 10/28/2013 7:33 PM, Rui Ueyama wrote:
>
>> Author: ruiu
>> Date: Mon Oct 28 19:33:34 2013
>> New Revision: 193565
>>
>> URL: http://llvm.org/viewvc/llvm-**project?rev=193565&view=rev<http://llvm.org/viewvc/llvm-project?rev=193565&view=rev>
>> Log:
>> [PECOFF] Fix __ImageBase symbol.
>>
>> __ImageBase is an absolute symbol whose address is the same as the image
>> base
>> address. What we did before this patch was to create __ImageBase symbol
>> as a
>> symbol whose *contents* (not location) is the image base address, which is
>> clearly wrong.
>>
>> Modified:
>>      lld/trunk/lib/ReaderWriter/**PECOFF/**LinkerGeneratedSymbolFile.h
>>      lld/trunk/test/pecoff/**imagebase.test
>>
>> Modified: lld/trunk/lib/ReaderWriter/**PECOFF/**
>> LinkerGeneratedSymbolFile.h
>> URL: http://llvm.org/viewvc/llvm-**project/lld/trunk/lib/**
>> ReaderWriter/PECOFF/**LinkerGeneratedSymbolFile.h?**
>> rev=193565&r1=193564&r2=**193565&view=diff<http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/PECOFF/LinkerGeneratedSymbolFile.h?rev=193565&r1=193564&r2=193565&view=diff>
>> ==============================**==============================**
>> ==================
>> --- lld/trunk/lib/ReaderWriter/**PECOFF/**LinkerGeneratedSymbolFile.h
>> (original)
>> +++ lld/trunk/lib/ReaderWriter/**PECOFF/**LinkerGeneratedSymbolFile.h
>> Mon Oct 28 19:33:34 2013
>> @@ -27,50 +27,26 @@ namespace coff {
>>     namespace {
>>   -// The symbol __ImageBase is a linker generated symbol. No standard
>> library
>> -// files define it, but the linker is expected to prepare it as if it
>> was read
>> -// from a file. The content of the atom is a 4-byte integer equal to the
>> image
>> -// base address. Note that because the name is prefixed by an underscore
>> on x86
>> -// Win32, the actual symbol name will be ___ImageBase (three
>> underscores).
>> -class ImageBaseAtom : public COFFLinkerInternalAtom {
>> -public:
>> -  ImageBaseAtom(const PECOFFLinkingContext &context, const File &file,
>> -                uint32_t imageBase)
>> -      : COFFLinkerInternalAtom(file, assembleRawContent(imageBase))**,
>> -        _name(context.decorateSymbol("**__ImageBase")) {}
>> -
>> -  virtual StringRef name() const { return _name; }
>> -  virtual uint64_t ordinal() const { return 0; }
>> -  virtual ContentType contentType() const { return typeData; }
>> -  virtual ContentPermissions permissions() const { return permRW_; }
>> -  virtual DeadStripKind deadStrip() const { return deadStripAlways; }
>> -
>> -private:
>> -  std::vector<uint8_t> assembleRawContent(uint32_t imageBase) {
>> -    std::vector<uint8_t> data = std::vector<uint8_t>(4);
>> -    *(reinterpret_cast<uint32_t *>(&data[0])) = imageBase;
>> -    return data;
>> -  }
>> -
>> -  StringRef _name;
>> -};
>> -
>> -// The file to wrap ImageBaseAtom. This is the only member file of
>> -// LinkerGeneratedSymbolFile.
>> +// A virtual file containing absolute symbol __ImageBase. __ImageBase (or
>> +// ___ImageBase on x86) is a linker-generated symbol whose address is
>> the same
>> +// as the image base address.
>> +//
>> +// This is the only member file of LinkerGeneratedSymbolFile.
>>   class MemberFile : public SimpleFile {
>>   public:
>> -  MemberFile(const PECOFFLinkingContext &context)
>> -      : SimpleFile(context, "Member of the Linker Internal File"),
>> -        _atom(context, *this, context.getBaseAddress()) {
>> -    addAtom(_atom);
>> +  MemberFile(const PECOFFLinkingContext &ctx)
>> +      : SimpleFile(ctx, "Member of the Linker Internal File"),
>> +        _imageBaseAtom(*this, ctx.decorateSymbol("__**ImageBase"),
>> +                       Atom::scopeGlobal, ctx.getBaseAddress()) {
>> +    addAtom(_imageBaseAtom);
>>     };
>>       bool contains(StringRef name) const {
>> -    return _atom.name() == name;
>> +    return _imageBaseAtom.name() == name;
>>     }
>>     private:
>> -  ImageBaseAtom _atom;
>> +  COFFAbsoluteAtom _imageBaseAtom;
>>   };
>>     } // anonymous namespace
>>
>> Modified: lld/trunk/test/pecoff/**imagebase.test
>> URL: http://llvm.org/viewvc/llvm-**project/lld/trunk/test/pecoff/**
>> imagebase.test?rev=193565&r1=**193564&r2=193565&view=diff<http://llvm.org/viewvc/llvm-project/lld/trunk/test/pecoff/imagebase.test?rev=193565&r1=193564&r2=193565&view=diff>
>> ==============================**==============================**
>> ==================
>> --- lld/trunk/test/pecoff/**imagebase.test (original)
>> +++ lld/trunk/test/pecoff/**imagebase.test Mon Oct 28 19:33:34 2013
>> @@ -2,11 +2,12 @@
>>   #
>>   # RUN: lld -flavor link /out:%t1 /subsystem:console /entry:_start
>> /opt:noref \
>>   # RUN:   -- %t.obj \
>> -# RUN:   && llvm-objdump -disassemble %t1 | FileCheck
>> -check-prefix=CHECK1 %s
>> +# RUN:   && llvm-objdump -disassemble %t1 | FileCheck
>> -check-prefix=DEFAULT %s
>>   #
>>   # RUN: lld -flavor link /out:%t1 /subsystem:console /entry:_start
>> /base:65536 \
>>   # RUN:   /opt:noref -- %t.obj \
>> -# RUN:   && llvm-objdump -disassemble %t1 | FileCheck
>> -check-prefix=CHECK2 %s
>> +# RUN:   && llvm-objdump -disassemble %t1 | FileCheck -check-prefix=BASE
>> %s
>>   -CHECK1: a1 00 20 40 00    movl    4202496, %eax
>> -CHECK2: a1 00 20 01 00    movl    73728, %eax
>> +DEFAULT: a1 00 00 40 00    movl    4194304, %eax
>> +
>> +BASE:    a1 00 00 01 00    movl    65536, %eax
>>
>>
>> ______________________________**_________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/**mailman/listinfo/llvm-commits<http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>>
>>
>>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
> by the Linux Foundation
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131028/feb5c9a0/attachment.html>