<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jul 21, 2016 at 3:45 AM, Rui Ueyama <span dir="ltr"><<a href="mailto:ruiu@google.com" target="_blank">ruiu@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I don't think it would work because parsing linker scripts is not a difficult part.</div></blockquote><div><br></div><div>Maybe it isn't, but why rewrite tested, documented code? (I don't know if you remember, but the documentation on the linker script parser that we had was really, really good).</div><div><br></div><div>Also, there are some some nasty parts like context-dependent lexing that you probably don't want to waste time rewriting (IIRC the existing code handled them correctly).</div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"> That parser was not plugged into an actual logic to handle complex linker script commands.</div></blockquote><div><br></div><div>Exactly. It seems more effective to focus on that instead of frivolously rewriting the AST generation part which we already have a good implementation of.</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jul 21, 2016 at 2:57 AM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div>On Wed, Jul 20, 2016 at 2:06 PM, Rui Ueyama via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">ruiu added inline comments.<br>
<br>
================<br>
Comment at: ELF/LinkerScript.h:49-59<br>
@@ -48,8 +48,13 @@<br>
<span><br>
+struct OutputSectionDescription {<br>
+  std::vector<StringRef> Phdrs;<br>
+  std::vector<uint8_t> Filler;<br>
+};<br>
+<br>
 struct SectionsCommand {<br>
   SectionsCommandKind Kind;<br>
   std::vector<StringRef> Expr;<br>
   StringRef Name;<br>
-  std::vector<StringRef> Phdrs;<br>
+  OutputSectionDescription Section;<br>
 };<br>
<br>
</span>----------------<br>
I think this is towards the right direction, but it doesn't still reflect the structure of SECTIONS linker script command.<br>
<br>
As per the linker script manual (#1), SECTIONS command contains one of the following sub-commands:<br>
<br>
 - an ENTRY command<br>
 - an overlay description<br>
 - a symbol assignment<br>
 - an output section description<br>
<br>
Since we do not support the first two, I'll ignore them in this comment. So focus on the last two. We can represent the two like this.<br>
<br>
  enum SectionsCommandKind { AssignmentKind, DescriptionKind  };<br>
<br>
  struct SectionsCommand {<br>
    SectionsCommandKind Kind;<br>
    std::vector<StringRef> Expr;  // For AssignmentKind<br>
    OutputSectionDescription Desc; // For DescriptionKind<br>
  };<br>
<br>
The above struct doesn't have `Name` field because SECTIONS doesn't have a name.<br>
<br>
Name is instead in output section description as described in #2. To reflect that grammar, you want to define a class like this.<br>
<br>
  struct OutputSectionDescription {<br>
    StringRef Name;<br>
    std::vector<OutputSectionCommand> Commands;<br>
    std::vector<uint8_t> Filler;<br>
  };<br>
<br>
And then you want to define OutputSectionCommand to reflect output-section-command as described in #2.<br>
<br>
So the point is to construct an AST whose structure logically matches with the grammar.<br></blockquote><div><br></div></div></div><div>Why don't we just bring back Rafael Auler's parser that was deleted? It was quite complete IIRC.</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>
<br>
#1 <a href="https://sourceware.org/binutils/docs/ld/SECTIONS.html#SECTIONS%232" rel="noreferrer" target="_blank">https://sourceware.org/binutils/docs/ld/SECTIONS.html#SECTIONS<br>
#2</a> <a href="https://sourceware.org/binutils/docs/ld/Output-Section-Description.html#Output-Section-Description" rel="noreferrer" target="_blank">https://sourceware.org/binutils/docs/ld/Output-Section-Description.html#Output-Section-Description</a><br>
<br>
<br>
<a href="https://reviews.llvm.org/D22589" rel="noreferrer" target="_blank">https://reviews.llvm.org/D22589</a><br>
<br>
<br>
<br></span>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>