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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Only nits left from me, but someone with more familiarity with this stuff should review it.



================
Comment at: llvm/test/tools/llvm-objdump/COFF/arm64ec.yaml:105
+
+## Check error handlnig of invalid code map RVA.
+# RUN: yaml2obj --docnum=2 %s -o %t
----------------



================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1430
+    } else if (const auto *COFFObj = dyn_cast<COFFObjectFile>(&Obj)) {
+      auto *CHPEMetadata = COFFObj->getCHPEMetadata();
+      if (CHPEMetadata && CHPEMetadata->CodeMapCount) {
----------------
Unlike the previous line, this ISN'T (in my opinion) a case where we should use `auto`. It's not obvious at the callsite that `CHPEMetadata` is of type `chpe_metadata` to me, because `getCHPEMetadata()` could return any type (in particular, it could easily be `Expected<chpe_metadata *>` if there was a chance for errors being reported etc. The reason you get away with it in the previous line is because the `dyn_cast` clearly shows the type being cast to.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:2234
+  } else if (const auto *COFFObj = dyn_cast<COFFObjectFile>(Obj)) {
+    auto *CHPEMetadata = COFFObj->getCHPEMetadata();
+    if (CHPEMetadata && CHPEMetadata->CodeMapCount) {
----------------
No `auto` here too


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:2241
+      std::string Error;
+      const auto *X64Target =
+          TargetRegistry::lookupTarget("", X64Triple, Error);
----------------
Ditto.


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

https://reviews.llvm.org/D149095