[PATCH] D85208: [llvm-readobj/elf] - Add a testing for --stackmap and refine the implementation.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/include/llvm/Object/StackMapParser.h:322
 
+  /// Validates the header of the stack map section given.
+  static Error validateHeader(ArrayRef<uint8_t> StackMapSection) {
----------------



================
Comment at: llvm/include/llvm/Object/StackMapParser.h:328
+          "the stack map section size (" + Twine(StackMapSection.size()) +
+          ") is less than the minimal possible size of its header (16)");
+
----------------
`minimal` -> `minimum`


================
Comment at: llvm/test/tools/llvm-readobj/ELF/stackmap.test:30
+    ShSize:       [[SHSIZE=<none>]]
+## An arbirtary second broken .llvm_stackmaps section.
+  - Name:         .llvm_stackmaps (1)
----------------



================
Comment at: llvm/test/tools/llvm-readobj/ELF/stackmap.test:47
+
+## Check we report a warning when the size of the .llvm_stackmaps is less than the minimal possible size of its header.
+
----------------
You probably want a line break in this comment now too.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:3430
 
-  ArrayRef<uint8_t> StackMapContentsArray = unwrapOrError(
-      ObjF->getFileName(), Obj->getSectionContents(StackMapSection));
+  auto Warn = [&](Error&& E) {
+    this->reportUniqueWarning(createError("unable to read the stack map from " +
----------------
Is this clang-formatted correctly? That `&&` looks in the wrong place to me.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:3438-3441
+  if (!ContentOrErr) {
+    Warn(ContentOrErr.takeError());
+    return;
+  }
----------------
Is there a test for this case failing?


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

https://reviews.llvm.org/D85208