[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:
> > 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.
> Sorry for the confusion. I mean (in a future change), we may change the order ELFState<ELFT>::alignToOffset is called on each section. We call alignToOffset following the order of section offsets. This may allow out-of-order sh_offset fields in the section header table.
> 
> https://reviews.llvm.org/D78422#1991831 says
> 
> > FWIW in embedded systems it is quite common to have linker scripts where different OutputSections need to be copied to various disjoint memory locations and these are often not in order of ascending address.
What I do not like in idea of allowing disordered offsets is:
1) It might open a worm can: it will be easy to create overlapping sections or something alike. We then might want the code to diagnose it,
but it sounds a bit too complex and probably not what we want.
2) Out logic of creating program headers assumes that chunks are ordered by offsets. Changing this assumption will introduce an additional complexity there.

I am thinking about the possible extending the YAML instead. E.g. we could have something like:

```
--- !ELF
FileHeader:
  Class:   ELFCLASS64
  Data:    ELFDATA2LSB
  Type:    ET_REL
  Machine: EM_X86_64
  SectionHeader:
    Location: <location> ## before sections/after sections
    Sections:
      - .foo
      - .bar
Sections:
  - Name:  .foo
    Type:  SHT_PROGBITS
    Size:  0x8
...
```

The benefit is that it is more flexible and should not touch the rest of the code.


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

https://reviews.llvm.org/D78927