[PATCH] D83482: [yaml2obj] - Add a syntax to override e_phoff, e_phentsize and e_phnum fields.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 13 03:24:46 PDT 2020


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


================
Comment at: llvm/lib/ObjectYAML/ELFYAML.cpp:873-875
+  IO.mapOptional("PhOff", FileHdr.PhOff);
+  IO.mapOptional("PhEntSize", FileHdr.PhEntSize);
+  IO.mapOptional("PhNum", FileHdr.PhNum);
----------------
jhenderson wrote:
> The section header table related fields use "SH" not "Sh". It probably makes sense for us to standardise on one form or the other. In other words, I think we should probably use "PH*" here.
Fields in `Sections` class also use `Sh*`:

```
  // This can be used to override the offset stored in the sh_name field.
  // It does not affect the name stored in the string table.
  Optional<llvm::yaml::Hex64> ShName;

  // This can be used to override the sh_offset field. It does not place the
  // section data at the offset specified.
  Optional<llvm::yaml::Hex64> ShOffset;

  // This can be used to override the sh_size field. It does not affect the
  // content written.
  Optional<llvm::yaml::Hex64> ShSize;

  // This can be used to override the sh_flags field.
  Optional<llvm::yaml::Hex64> ShFlags;
```

So we have 2 options it seems:
1) Use "SH*" + "PH*" everywhere (i.e make the renaming you suggest + rename fields in `Sections`).
2) Rename "PH*" -> "Ph*" in a follow-up here (I assumed this action when wrote this patch).

What should we do?


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

https://reviews.llvm.org/D83482





More information about the llvm-commits mailing list