[PATCH] [yaml2obj] ELF Relocations Support

Simon Atanasyan simon at atanasyan.com
Wed Apr 9 15:30:13 PDT 2014



================
Comment at: lib/Object/ELFYAML.cpp:579-580
@@ +578,4 @@
+    if (auto S = dyn_cast<ELFYAML::ContentSection>(Section.get())) {
+      IO.mapTag("!Section", true);
+      MappingTraits<ELFYAML::ContentSection>::mapping(IO, *S);
+    } else if (auto S = dyn_cast<ELFYAML::RelocationSection>(Section.get())) {
----------------
Simon Atanasyan wrote:
> Sean Silva wrote:
> > Shankar Kalpathi Easwaran wrote:
> > > Sean Silva wrote:
> > > > Simon Atanasyan wrote:
> > > > > Sean Silva wrote:
> > > > > > Now that I think about it, since in the future the ELFYAML::Section hierarchy is going to be growing, I think it would make sense to standardize on a consistent tag name scheme. It just bothers me that the generic "!Section" is being used for ContentSection, while "!Relocations" for RelocationSection. Can we standardize on having the tag be the name of the class?
> > > > > Agreed. But I am not sure that ContentSection is a good name for tag. What do you think?
> > > > Now that I think more about it, I think that the tag system is just overcomplicating things. I think it would make a lot more sense and be simpler to just autodetect which ELFYAML::Section subclass to create based on which keys are present in the section entry.
> > > > 
> > > > i.e., the deserialization deserializes `Content:` *and* `Relocations:` (and any future additions) as optional, then chooses which subclass to instantiate based on what is present. Of course, it issues an error if there is an illegal combination of keys.
> > > > 
> > > > The idea is that `Content:`, `Relocations:`, etc. (any future ones) would be just sugar for filling in the `Content:` in a more convenient/readable way; this approach makes the whole process easier to understand from a user's perspective.
> > > > 
> > > > I think the goal of the tags (making it a bit more "strongly typed" on disk) is admirable, but at the end of the day we are still duck typing (from a tag perspective) between Rel and Rela (and probably a bunch of other things). It's also just plain redundant.
> > > This may be over-engineering yaml2obj IMHO.
> > It's not clear what your comment was aimed at (my last paragraph or the approach I suggested?).
> I see the point but I think now we do not have enough information to make a correct design decision. There are only two different types of section and it is rather easy (I hope) to check what entries are in the Section block and select an appropriate binary representation. But suppose that in the future we will have more mandatory and optional Section entries. I am not sure that it will be always possible to select a binary representation in that case.
> 
> Let's define two types of section:
> ```
> SectionFoo
>   MegaContent (mandatory field)
>   UselessContent (optional field)
> 
> SectionBar
>   MegaContent (mandatory field)
>   NotSoUselessContent (optional field)
> ```
> 
> It's difficult to recognize a section type in the following YAML file:
> ```
> - Name: .section_name
>   MegaContent: "bla-bla-bla"
> ```
> 
> If we define much more section type later or, on the contrary, understand that only `RawContentSection` and `RelocationSection` satisfy our requirements, we can combine two approaches:
> # Tag defines section type unambiguously.
> # If Tag is omitted, we can try to deduce section type from the set of declared fields.
> 
> Now I would like to keep the solution simple. Next we need to implement `obj2yaml` conversion for ELF binaries. This might give us additional info to select persistent design.
>From the other side, I will try to implement the approach suggested by you tomorrow. Let's take a look on the result and compare solutions.


http://reviews.llvm.org/D3302






More information about the llvm-commits mailing list