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

[Sean Silva via llvm-commits]

On Thu, Jul 21, 2016 at 5:49 PM, Rui Ueyama <ruiu at google.com> wrote:

> > 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).
>
> I took a quick look at the previous linker script parser to refresh my
> memory and realized that it's written in a totally different way than the
> current linker script processor. It also contains features which we don't
> need. If you import it, you got to ditch the current linker script
> implementation and star over from large refactoring. It doesn't seem an
> option for us at all.
>
Well, the stuff that is already there is <1k LOC and mostly parsing code
that would become redundant. I think it is doable and would be of long-term
benefit, but I'm not the person doing the work of course.

Also, you'll note that your primary comment on George's patch was about how
to define the AST data structures. This is not going to have a shortcut;
somebody has to sit down and grok the language and its semantics and
structure in order to correctly parse the language and model its semantics
in an AST. This work has already been largely done. Except as a learning
experience (which I think is a valid reason), there doesn't seem to be much
of a point in redoing that.

I would at least suggest to George to check out Rafael Auler's linker
script parser from the SVN history and use it like another data point for
understanding / implementing these features.

-- Sean Silva


> > 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/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/0aa36a1d/attachment.html>