[PATCH] D79165: [DebugInfo] - DWARFDebugFrame: do not call abort() on errors.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp:532
+            Entries.back()->cfis().parse(Data, &Offset, EndStructureOffset))
+      return std::move(E);
 
----------------
MaskRay wrote:
> The return type has been changed to `Error`. `return std::move(E)` should be written as `return E` to avoid clang `warning: moving a local object in a return statement prevents
>  copy elision [-Wpessimizing-move]`
Ah. I was under impression that `return E` instead of `std::move(E)` won't compile, because it usually doesn't (when the function returns `Expected<>`, what is very common), but I've missed we return an `Error` here. Thanks!


================
Comment at: llvm/test/tools/llvm-readobj/ELF/unwind.test:237
+    Type: SHT_X86_64_UNWIND
+## The content is generated from the following code. It has no CIE.
+## See the DebugInfoX86/eh-frame-cie-id.s test case for more history.
----------------
MaskRay wrote:
> Perhaps an assembly test is better here? It will require `REQUIRES: x86-registered-target`.
> 
> (You may be interested in this thread: 
> http://lists.llvm.org/pipermail/llvm-dev/2020-April/141203.html "[yaml2obj] GSoC-20: Add DWARF support to yaml2obj" also see the same thread on 2020-March
I expect one day we will be able to rewrite it to yaml. At least adding a support for `.eh_frame` does not feel too hard to me, though I've not tried and may miss something. And this sample is little, so I'd leave it as YAML.


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

https://reviews.llvm.org/D79165