[PATCH] D71872: [yaml2obj/obj2yaml] - Add support for SHT_RELR sections.

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 27 10:07:26 PST 2019


MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

This is new code.. Perhaps give @jhenderson a chance to review if you are not in a hurry.



================
Comment at: llvm/lib/ObjectYAML/ELFEmitter.cpp:839
+  SHeader.sh_entsize =
+      Section.EntSize ? (uint64_t)(*Section.EntSize) : sizeof(Elf_Relr);
+
----------------
the parentheses around `uint64_t` can be deleted.


================
Comment at: llvm/lib/ObjectYAML/ELFEmitter.cpp:846
+
+  for (const llvm::yaml::Hex64 &Hex : *Section.Entries) {
+    if (!ELFT::Is64Bits && Hex > UINT32_MAX)
----------------
Hex64 is a small, trivially copyable type. Consider dropping `const &`.


================
Comment at: llvm/tools/obj2yaml/elf2yaml.cpp:745
+    for (Elf_Relr Rel : *Relrs)
+      S->Entries->emplace_back(Rel);
+    return S.release();
----------------
Nit: when there is a choice between push_back and emplace_back with the same arguments, push_back may be more readable, because push_back expresses the intent more specifically.

For example, for `std::vector<std::vector<int>> a`, it accepts `a.emplace_back(10)` but not `a.push_back(10)`. push_back makes the reader think less.


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

https://reviews.llvm.org/D71872





More information about the llvm-commits mailing list