[PATCH] D63879: [yaml2obj] - Allow overriding sh_offset field from the YAML.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D63879#1561995 <https://reviews.llvm.org/D63879#1561995>, @grimar wrote:

> In D63879#1560868 <https://reviews.llvm.org/D63879#1560868>, @jhenderson wrote:
>
> >
>
>
> Yes, I had the same concerns. But I think it is fine generally: at any time we still can introduce a new field that might have a behavior you describe.
>  Its name can be `SecOffset`. See: currently all YAML fields names we have do not match the ELF fields name directly. E.g. we have `Name` but it doesn't match `sh_name`.
>  In this patch I named the field `ShOffset` to match the `sh_offset`. We can have an agreement that if a field matches the ELF field name then its intention os to
>  override the field's value. It is in line with D63771 <https://reviews.llvm.org/D63771> btw.


Okay, I can buy that, but this needs clear comments in the code, stating that this does NOT place the section data at the specified offset.



================
Comment at: test/tools/yaml2obj/elf-override-shoffset.yaml:1
+## Check we are able to set custom sh_offset field
+## for different sections.
----------------
I wonder if it would be worth somehow showing in a test case that this field doesn't place section data at the specified offset? What do you think? Relatedly, having an offset outside the file and showing that the file size doesn't increase would probably be worthwhile.


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

https://reviews.llvm.org/D63879