[PATCH] D69399: [yaml2obj/obj2yaml] - Add support for SHT_GNU_HASH section.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/ObjectYAML/ELFEmitter.cpp:1087
+  // We've finished writing the header. Here we write the bloom filter.
+  for (llvm::yaml::Hex64 Val : *Section.BloomFilter)
+    support::endian::write<typename ELFT::uint>(OS, Val,
----------------
grimar wrote:
> jhenderson wrote:
> > What if `BloomFilter` is `None`?
> It can't be `None` here.
> See the code in `validate()` method below: `BloomFilter`, `HashBuckets`, `HashValues` and `Header` must be provided when there is no `Content` and must be used together.
Right, sorry, got my timeline wrong on what methods can get called when.


================
Comment at: llvm/test/tools/obj2yaml/elf-gnu-hash-section.yaml:87
+## create at least one entry in Bloom filter if they want to disable it.
+## Also the dynamic symbol table has a null entry and having SymNdx = 0
+## here is at least strange.
----------------
"Also the" -> "Also, the"


================
Comment at: llvm/test/tools/obj2yaml/elf-gnu-hash-section.yaml:124
+    HashValues:  []
+## Case 5: Check that when hash values data size contains a set of 4 byte words,
+## we use the "HashValues" property to dump it, but fallback to dumping the whole
----------------
Okay, I think this comment needs rephrasing. How about:

"Check that we use the various properties to dump the data when it has a size that is a multiple of 4, but fallback to..."


================
Comment at: llvm/test/tools/yaml2obj/elf-gnu-hash-section.yaml:137
+Sections:
+  - Name:  .gnu.hash.only.header
+    Type:  SHT_GNU_HASH
----------------
.gnu.hash.no.header?


================
Comment at: llvm/test/tools/yaml2obj/elf-gnu-hash-section.yaml:150
+Sections:
+  - Name:  .gnu.hash.only.header
+    Type:  SHT_GNU_HASH
----------------
.gnu.hash.no.bloomfilter?


================
Comment at: llvm/test/tools/yaml2obj/elf-gnu-hash-section.yaml:165
+Sections:
+  - Name:  .gnu.hash.only.header
+    Type:  SHT_GNU_HASH
----------------
.gnu.hash.no.nobuckets?


================
Comment at: llvm/test/tools/yaml2obj/elf-gnu-hash-section.yaml:180
+Sections:
+  - Name:  .gnu.hash.only.header
+    Type:  SHT_GNU_HASH
----------------
.gnu.hash.no.novalues?


================
Comment at: llvm/test/tools/yaml2obj/elf-gnu-hash-section.yaml:260
+
+## Test we can override the number of buckets and the number of words in the bloom filter
+## using the "NBuckets" and "Shift2" keys.
----------------
This bloom hasn't been changed to Bloom.


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

https://reviews.llvm.org/D69399