<div dir="ltr">On Mon, Oct 28, 2013 at 9:16 PM, Shankar Easwaran <span dir="ltr"><<a href="mailto:shankare@codeaurora.org" target="_blank">shankare@codeaurora.org</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">

<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hi Rui,<br>
<br>
This is an incorrect subclass of SimpleFile. The MemberFile wouldnot return an empty vector of absoluteAtoms when absolute() is called.<br></blockquote><div><br></div><div>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.</div>

<div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
I think the right way of doing this is to create a SimpleFile and add an AbsoluteAtom to it. </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">


Thanks<br>
<br>
Shankar Easwaran<div class=""><div class="h5"><br>
<br>
<br>
On 10/28/2013 7:33 PM, Rui Ueyama wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Author: ruiu<br>
Date: Mon Oct 28 19:33:34 2013<br>
New Revision: 193565<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=193565&view=rev" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project?rev=193565&view=rev</a><br>
Log:<br>
[PECOFF] Fix __ImageBase symbol.<br>
<br>
__ImageBase is an absolute symbol whose address is the same as the image base<br>
address. What we did before this patch was to create __ImageBase symbol as a<br>
symbol whose *contents* (not location) is the image base address, which is<br>
clearly wrong.<br>
<br>
Modified:<br>
     lld/trunk/lib/ReaderWriter/<u></u>PECOFF/<u></u>LinkerGeneratedSymbolFile.h<br>
     lld/trunk/test/pecoff/<u></u>imagebase.test<br>
<br>
Modified: lld/trunk/lib/ReaderWriter/<u></u>PECOFF/<u></u>LinkerGeneratedSymbolFile.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/PECOFF/LinkerGeneratedSymbolFile.h?rev=193565&r1=193564&r2=193565&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/lld/trunk/lib/<u></u>ReaderWriter/PECOFF/<u></u>LinkerGeneratedSymbolFile.h?<u></u>rev=193565&r1=193564&r2=<u></u>193565&view=diff</a><br>


==============================<u></u>==============================<u></u>==================<br>
--- lld/trunk/lib/ReaderWriter/<u></u>PECOFF/<u></u>LinkerGeneratedSymbolFile.h (original)<br>
+++ lld/trunk/lib/ReaderWriter/<u></u>PECOFF/<u></u>LinkerGeneratedSymbolFile.h Mon Oct 28 19:33:34 2013<br>
@@ -27,50 +27,26 @@ namespace coff {<br>
    namespace {<br>
  -// The symbol __ImageBase is a linker generated symbol. No standard library<br>
-// files define it, but the linker is expected to prepare it as if it was read<br>
-// from a file. The content of the atom is a 4-byte integer equal to the image<br>
-// base address. Note that because the name is prefixed by an underscore on x86<br>
-// Win32, the actual symbol name will be ___ImageBase (three underscores).<br>
-class ImageBaseAtom : public COFFLinkerInternalAtom {<br>
-public:<br>
-  ImageBaseAtom(const PECOFFLinkingContext &context, const File &file,<br>
-                uint32_t imageBase)<br>
-      : COFFLinkerInternalAtom(file, assembleRawContent(imageBase))<u></u>,<br>
-        _name(context.decorateSymbol("<u></u>__ImageBase")) {}<br>
-<br>
-  virtual StringRef name() const { return _name; }<br>
-  virtual uint64_t ordinal() const { return 0; }<br>
-  virtual ContentType contentType() const { return typeData; }<br>
-  virtual ContentPermissions permissions() const { return permRW_; }<br>
-  virtual DeadStripKind deadStrip() const { return deadStripAlways; }<br>
-<br>
-private:<br>
-  std::vector<uint8_t> assembleRawContent(uint32_t imageBase) {<br>
-    std::vector<uint8_t> data = std::vector<uint8_t>(4);<br>
-    *(reinterpret_cast<uint32_t *>(&data[0])) = imageBase;<br>
-    return data;<br>
-  }<br>
-<br>
-  StringRef _name;<br>
-};<br>
-<br>
-// The file to wrap ImageBaseAtom. This is the only member file of<br>
-// LinkerGeneratedSymbolFile.<br>
+// A virtual file containing absolute symbol __ImageBase. __ImageBase (or<br>
+// ___ImageBase on x86) is a linker-generated symbol whose address is the same<br>
+// as the image base address.<br>
+//<br>
+// This is the only member file of LinkerGeneratedSymbolFile.<br>
  class MemberFile : public SimpleFile {<br>
  public:<br>
-  MemberFile(const PECOFFLinkingContext &context)<br>
-      : SimpleFile(context, "Member of the Linker Internal File"),<br>
-        _atom(context, *this, context.getBaseAddress()) {<br>
-    addAtom(_atom);<br>
+  MemberFile(const PECOFFLinkingContext &ctx)<br>
+      : SimpleFile(ctx, "Member of the Linker Internal File"),<br>
+        _imageBaseAtom(*this, ctx.decorateSymbol("__<u></u>ImageBase"),<br>
+                       Atom::scopeGlobal, ctx.getBaseAddress()) {<br>
+    addAtom(_imageBaseAtom);<br>
    };<br>
      bool contains(StringRef name) const {<br>
-    return _<a href="http://atom.name" target="_blank">atom.name</a>() == name;<br>
+    return _imageBaseAtom.name() == name;<br>
    }<br>
    private:<br>
-  ImageBaseAtom _atom;<br>
+  COFFAbsoluteAtom _imageBaseAtom;<br>
  };<br>
    } // anonymous namespace<br>
<br>
Modified: lld/trunk/test/pecoff/<u></u>imagebase.test<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/test/pecoff/imagebase.test?rev=193565&r1=193564&r2=193565&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/lld/trunk/test/pecoff/<u></u>imagebase.test?rev=193565&r1=<u></u>193564&r2=193565&view=diff</a><br>


==============================<u></u>==============================<u></u>==================<br>
--- lld/trunk/test/pecoff/<u></u>imagebase.test (original)<br>
+++ lld/trunk/test/pecoff/<u></u>imagebase.test Mon Oct 28 19:33:34 2013<br>
@@ -2,11 +2,12 @@<br>
  #<br>
  # RUN: lld -flavor link /out:%t1 /subsystem:console /entry:_start /opt:noref \<br>
  # RUN:   -- %t.obj \<br>
-# RUN:   && llvm-objdump -disassemble %t1 | FileCheck -check-prefix=CHECK1 %s<br>
+# RUN:   && llvm-objdump -disassemble %t1 | FileCheck -check-prefix=DEFAULT %s<br>
  #<br>
  # RUN: lld -flavor link /out:%t1 /subsystem:console /entry:_start /base:65536 \<br>
  # RUN:   /opt:noref -- %t.obj \<br>
-# RUN:   && llvm-objdump -disassemble %t1 | FileCheck -check-prefix=CHECK2 %s<br>
+# RUN:   && llvm-objdump -disassemble %t1 | FileCheck -check-prefix=BASE %s<br>
  -CHECK1: a1 00 20 40 00    movl    4202496, %eax<br>
-CHECK2: a1 00 20 01 00    movl    73728, %eax<br>
+DEFAULT: a1 00 00 40 00    movl    4194304, %eax<br>
+<br>
+BASE:    a1 00 00 01 00    movl    65536, %eax<br>
<br>
<br>
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
<br>
<br>
</blockquote>
<br>
<br></div></div><span class=""><font color="#888888">
-- <br>
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation<br>
<br>
</font></span></blockquote></div><br></div></div>