[PATCH] D75342: [obj2yaml] - Teach tool to dump program headers.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 24 03:44:09 PDT 2020


grimar marked 3 inline comments as done.
grimar added inline comments.


================
Comment at: llvm/include/llvm/ObjectYAML/ELFYAML.h:180
 
+  // Hold the original section index. Used for dumping program headers.
+  unsigned OriginalSecNdx;
----------------
jhenderson wrote:
> Hold -> Holds (!)
> 
> (For reference, third-person singular verbs usually have an 's' on the end, but not third-person plural - previously multiple variables were being referenced, so plural, but now there is only one)
Thanks.. :]


================
Comment at: llvm/tools/obj2yaml/elf2yaml.cpp:226-231
+  // The contents of these sections are described by other parts of the YAML
+  // file. We only keep allocatable section because their positions and
+  // addresses are important, e.g. for creating program headers. Some sections,
+  // like .symtab or .strtab normally are not allocatable and do not have
+  // virtual addresses. We want to avoid noise in the YAML output and assume
+  // that they are placed at the end.
----------------
jhenderson wrote:
> This comment refers to allocatable sections, which I don't think is quite right?
We still only leave allocatable versions in the output, though I think this comment can be better now.
I'll split the mentioned NFC patch soon. It will include this piece of code and I'll rewrite this comment.


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

https://reviews.llvm.org/D75342





More information about the llvm-commits mailing list