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

Thu Dec 19 14:33:43 PST 2019

dblaikie added a comment.

In D70756#1791676 <https://reviews.llvm.org/D70756#1791676>, @eugenis wrote:

> In D70756#1790896 <https://reviews.llvm.org/D70756#1790896>, @jhenderson wrote:
>
> > I've got no more comments, but I'm not going to LGTM it because I disagree with the principle of not minimising test cases (I have better things to do with my time than try to understand what is actually important in a test case that has started failing/needs updating etc etc).
>
>
> I understand your concern. But by minimizing the test case, I (not an expert in dwarf by any measure) can make it invalid in a way that is not detected by the current parser code. How do you feel about fixing a bunch of tests, all broken in different ways, when LLVM becomes smarter about it? This test case may be a bit long, but I'm reasonably sure that it is correct.

Yeah, while I wouldn't generally consider not being an expert as sufficient for this - I'd push back a bit if it were only that. But hand writing DWARF assembly isn't exactly trivial (DIE offsets being a simple example of something that would need to be manually adjusted currently - though we've talked about modifying LLVM's output to be more symbolic & thus easier to edit) but also changing abbreviations in lock-step with the DIEs themselves, etc.

At some point we might end up with yaml2obj or something like it growing higher level descriptions to make it easy to write varying degrees of valid DWARF - so it's easy to write to the level of correctness/incorrectness that's desired for a test without a lot of redundancy/verbosity in the written form. (eg: something that autoselects correct forms, requires certain attributes, etc, automatically generates abbreviations - but then lets you drop down below that if you want a quirky attribute added/removed, etc)

(I wouldn't mind if we had some way to turn off the call site entry DIEs, as they do add a fair bit of noise to simple test cases like this - but doesn't look like there's a flag for it & I don't think it's worth hand-editing the DWARF to remove them)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70756