[PATCH] D70756: llvm-symbolizer: support DW_FORM_loclistx locations.

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDie.cpp:503
+      if (auto LoclistOffset = U->getLoclistOffset(Offset))
+        Offset = *LoclistOffset + getDwarfUnit()->getLocSectionBase();
+      else
----------------
ostannard wrote:
> It looks like this patch conflicts with D71006, which adds the loc section base inside DWARFUnit::getLoclistOffset, so it doesn't need adding here.
fixed


================
Comment at: llvm/test/tools/llvm-symbolizer/frame-loclistx.s:1
+// Test dwarf-5 DW_AT_location [DW_FORM_loclistx].
+// REQUIRES: aarch64-registered-target
----------------
jhenderson wrote:
> I think this test would make more sense in the DebugInfo test directory, as this isn't really testing llvm-symbolizer's specific features, and is more testing the DebugInfo library code.
> 
> A couple of other points:
> * I've noticed that a number of text editors don't work well with C++ style comments for .s files. I think you should use '#' (and '##' for comments to improve clarity of comments versus lit/FileCheck directives).
> * The assembly is very complex. Can you reduce it down significantly to just what you need for your test?
Moved the test to DebugInfo.

C++ styles comments are what clang generates in the assembly output. Anyway, I've replaced them with "#".

> The assembly is very complex. Can you reduce it down significantly to just what you need for your test?

I'm not really comfortable hacking on the dwarf assembly. I'm worried it's very easy to create a malformed test input this way, and I'm not familiar with the format enough to recognize it.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70756