[PATCH] D149095: [llvm-objdump] Support CHPE code ranges in disassembler.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

(I'll go on a trip for a few days and won't look at this change. Land if jhenderson approves:) .)



================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1429
+          PrimaryTarget.SubtargetInfo->checkFeatures("+thumb-mode");
+    } else if (const COFFObjectFile *COFFObj = dyn_cast<COFFObjectFile>(&Obj)) {
+      const chpe_metadata *CHPEMetadata = COFFObj->getCHPEMetadata();
----------------
This is an instance that `auto` is obvious https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1434
+        if (Error E = COFFObj->getRvaPtr(CHPEMetadata->CodeMap, CodeMapInt))
+          reportError(std::move(E), Obj.getFileName());
+        CodeMap = reinterpret_cast<const chpe_range_entry *>(CodeMapInt);
----------------
The error is not tested? For metadata issues, consider whether a warning suffices to continue other dumping functionality.

We also have a `reportUniqueWarning` now, though a `Dumper` object is required.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1628
+        if (Start < End) {
+          MappingSymbols.emplace_back(Start - SectionAddr, 's');
+          MappingSymbols.emplace_back(End - SectionAddr, 'p');
----------------
FWIW, I am going to change MappingSymbols in D156190


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

https://reviews.llvm.org/D149095