[PATCH] D78927: [yaml2obj] - Introduce the "Offset" property for sections.

[George Rimar via Phabricator via llvm-commits]

grimar marked an inline comment as done.
grimar added inline comments.


================
Comment at: llvm/lib/ObjectYAML/ELFEmitter.cpp:1035-1036
+    if ((uint64_t)*Offset < CurrentOffset) {
+      reportError("the 'Offset' value (0x" +
+                  Twine::utohexstr((uint64_t)*Offset) + ") goes backward");
+      return CurrentOffset;
----------------
MaskRay wrote:
> grimar wrote:
> > jhenderson wrote:
> > > I can see why you've done this, but I think there's also a use-case for a section header table where the sections aren't in offset order, which I believe would run into this problem. You could use the SHOffset field instead, but that has its own problems (doesn't place data at the specified location, shouldn't be included in program header calculations etc). 
> > yaml2obj does not support writing section out of order currently. This patch has no intention to change this.
> > We always write sections in the same order they are listed in a YAML.
> > 
> > To implement what you want perhaps we might want just to introduce another field, like "SectionHeaderIndex".
> > Or may be expand YAML format to it allow to describe section header table (and may be it's location) explicitly.
> `alignToOffset(sec)` can be called according to the section offset order in `Doc.Chunks` in `ELFState<ELFT>::initSectionHeaders` may be sufficient, but I haven't tried.
> alignToOffset(sec) can be called according to the section offset order in Doc.Chunks in ELFState<ELFT>::initSectionHeaders may be sufficient.

Not sure I understand what you suggest? Calling of `alignToOffset` in `initSectionHeaders` is what this patch does already.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78927/new/

https://reviews.llvm.org/D78927