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

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.

LGTM, with one remaining suggestion.



================
Comment at: llvm/test/tools/yaml2obj/ELF/section-offset.yaml:4
+## Show how the 'Offset' field key can be used.
+## Show that it might affect the file size.
+
----------------
grimar wrote:
> jhenderson wrote:
> > This comment needs updating, since you're checking the section header table offset now.
> Actually what I think we want to show is that the `Offset` key might affect the file size.
> It is the main difference with the `ShOffset` property which never can affect it.
> Currently (since you asked to remove the file size test previously) it is tested implicitly:
> the `Offset` changes physical offsets of the following sections, but still worth to mention I think.
> 
> Changing of the section header table offset is a size effect. If it was placed before sections and not after,
> it would not change.
I guess more generally, changing the Offset field changes the layout of the file. Consequently, anything appearing after the section will have a higher offset than it might otherwise do (including sections, the section header table, and the end of the file). Perhaps we should update the comment to say that:

"Show that it can affect the layout of the rest of the file."


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

https://reviews.llvm.org/D78927