[PATCH] D22589: [ELF] - Introduce output section description class for holding section command

[Sean Silva via llvm-commits]

On Thu, Jul 21, 2016 at 3:45 AM, Rui Ueyama <ruiu at google.com> wrote:

> I don't think it would work because parsing linker scripts is not a
> difficult part.
>

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).

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).


> That parser was not plugged into an actual logic to handle complex linker
> script commands.
>

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.

-- Sean Silva


>
> On Thu, Jul 21, 2016 at 2:57 AM, Sean Silva <chisophugis at gmail.com> wrote:
>
>>
>>
>> On Wed, Jul 20, 2016 at 2:06 PM, Rui Ueyama via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> ruiu added inline comments.
>>>
>>> ================
>>> Comment at: ELF/LinkerScript.h:49-59
>>> @@ -48,8 +48,13 @@
>>>
>>> +struct OutputSectionDescription {
>>> +  std::vector<StringRef> Phdrs;
>>> +  std::vector<uint8_t> Filler;
>>> +};
>>> +
>>>  struct SectionsCommand {
>>>    SectionsCommandKind Kind;
>>>    std::vector<StringRef> Expr;
>>>    StringRef Name;
>>> -  std::vector<StringRef> Phdrs;
>>> +  OutputSectionDescription Section;
>>>  };
>>>
>>> ----------------
>>> I think this is towards the right direction, but it doesn't still
>>> reflect the structure of SECTIONS linker script command.
>>>
>>> As per the linker script manual (#1), SECTIONS command contains one of
>>> the following sub-commands:
>>>
>>>  - an ENTRY command
>>>  - an overlay description
>>>  - a symbol assignment
>>>  - an output section description
>>>
>>> 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.
>>>
>>>   enum SectionsCommandKind { AssignmentKind, DescriptionKind  };
>>>
>>>   struct SectionsCommand {
>>>     SectionsCommandKind Kind;
>>>     std::vector<StringRef> Expr;  // For AssignmentKind
>>>     OutputSectionDescription Desc; // For DescriptionKind
>>>   };
>>>
>>> The above struct doesn't have `Name` field because SECTIONS doesn't have
>>> a name.
>>>
>>> Name is instead in output section description as described in #2. To
>>> reflect that grammar, you want to define a class like this.
>>>
>>>   struct OutputSectionDescription {
>>>     StringRef Name;
>>>     std::vector<OutputSectionCommand> Commands;
>>>     std::vector<uint8_t> Filler;
>>>   };
>>>
>>> And then you want to define OutputSectionCommand to reflect
>>> output-section-command as described in #2.
>>>
>>> So the point is to construct an AST whose structure logically matches
>>> with the grammar.
>>>
>>
>> Why don't we just bring back Rafael Auler's parser that was deleted? It
>> was quite complete IIRC.
>>
>> -- Sean Silva
>>
>>
>>>
>>> #1 https://sourceware.org/binutils/docs/ld/SECTIONS.html#SECTIONS
>>> #2 <https://sourceware.org/binutils/docs/ld/SECTIONS.html#SECTIONS%232>
>>> https://sourceware.org/binutils/docs/ld/Output-Section-Description.html#Output-Section-Description
>>>
>>>
>>> https://reviews.llvm.org/D22589
>>>
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160721/4b0d28cf/attachment.html>