[PATCH] D105985: Support GSYM in llvm-symbolizer.

Mon Jul 19 00:11:03 PDT 2021

jhenderson added a comment.

In D105985#2882575 <https://reviews.llvm.org/D105985#2882575>, @simon.giesecke wrote:

> Hm, I think it makes sense to have a common ground for testing both the DWARF and the GSYM path, to ensure these are (mostly) consistent. If I rewrite this to a different approach, then sym.test should be similarly changed. OTOH, this might mean that rewriting both tests might be done at a later point?

I don't disagree that in principle any debug information implementation should be able to represent the same things, and therefore the testing here should be (in an ideal world) identical. However, there are a couple of points worth raising: the existing sym.test is testing features that are not specific to the debug format. Ideally, they should be pulled into their own test file (see above my comments about rewriting that test). Also, copying this test as-is would increase our maintenance burden over the longer term. A better approach might be to write the GSYM test from scratch, as if the DWARF test didn't exist, and then write a DWARF equivalent one (alternatively the other way around would also work). The important parts of sym.test could then be split off.

> I can remove the second canned binary (actually, it might not be required at all) and the canned GSYM file, and create both at runtime from addr.exe. However, the point is that this uses the same underlying binary as sym.test.

As noted above, I don't think it's good for sym.test to be using a canned binary either. The reason it does is purely because at the time of writing it wasn't possible to avoid the canned binary. We have ways to do that now however.

> Generally, I think option 1 would be too fragile, and I guess that's the reason why sym.test uses the canned binary?

cross-project-tests is new. sym.test is very old (see above). It's hard to judge whether 1 would be fragile, but the general opinion of the LLD developers is that the output is likely to be fairly stable for simple things going forward, so I don't think it would be too fragile (see the initial discussion on the mailing list about bringing up that test-suite).

> Option 2... might be feasible, but I don't readily know how to do that.

I'm afraid I don't know anything about gsym and therefore how to do this either. It should be possible though somehow.

> Option 3... hm I am not sure what you imagine there? Generate a GSYM file without a corresponding binary? That might make sense for additional tests, but again, the test I added was purposefully using the same binary as sym.test.

Some of this point may not make sense at all, given my lack of GSYM knowledge, but see also my above comments.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105985