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

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 5 02:53:43 PDT 2020


grimar added inline comments.


================
Comment at: llvm/tools/llvm-readobj/COFFDumper.cpp:63
 
-static inline Error createError(const Twine &Err) {
-  return make_error<StringError>(Err, object_error::parse_failed);
----------------
MaskRay wrote:
> This seems an unrelated change.
It is related: I had to include the `ELF.h` to `StackMapParser.h` (to use `object::createError`) and the former contains the equal `createError` implementation. So I had to remove this one to fix compilation error.

aside: noticable, that under MSVS it compiles fine without early inclusion of the `ELF.h` to  `StackMapParser.h` . But it is just because MSVS has relaxed rules about the order of static methods I believe. E.g. you can define a static helper at line 100 and use if from the line 90.


================
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 " +
----------------
jhenderson wrote:
> Is this clang-formatted correctly? That `&&` looks in the wrong place to me.
Fixed.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:3438-3441
+  if (!ContentOrErr) {
+    Warn(ContentOrErr.takeError());
+    return;
+  }
----------------
jhenderson wrote:
> Is there a test for this case failing?
Oh, it was not tested. Thanks!


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

https://reviews.llvm.org/D85208



More information about the llvm-commits mailing list