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

[Simon Giesecke via Phabricator via llvm-commits]

simon.giesecke added a comment.

In D105985#2882537 <https://reviews.llvm.org/D105985#2882537>, @jhenderson wrote:

> In D105985#2882467 <https://reviews.llvm.org/D105985#2882467>, @simon.giesecke wrote:
>
>> In D105985#2879253 <https://reviews.llvm.org/D105985#2879253>, @jhenderson wrote:
>>
>>> 1. Make sure to review the llvm-symbolizer documentation to see if there's anything there that needs adding/updating given this change (there may not be).
>>
>> I checked llvm/docs/CommandGuide/llvm-symbolizer.rst, and it doesn't say anything right now about how the data source is selected. Maybe it should, and I can add something, but then this should probably be more than just saying that GSYM is preferred when it's present. I think there's similar logic for .dwp files?
>
> I don't think we need to go into that in this change. I guess the point of the llvm-symbolizer documentation is more about how to use the tool, not what does the tool do under-the-hood, but I thought it was worth checking.

Ok.

> In D105985#2879285 <https://reviews.llvm.org/D105985#2879285>, @simon.giesecke wrote:
>
>>> 2. Can you avoid using pre-canned binaries for the test input, by somehow generating the inputs on the fly? The existing pre-canned binaries in the llvm-symbolizer tests are already sub-optimal, in my opinion, as they are opaque and make testing less flexible.
>>
>> The sym-gsymonly test is mostly a copy of the sym test, and the binary is just the stripped binary. These additional files don't need to be part of the repository, we could run `llvm-gsymutil --convert` and strip DWARF from it as part of the test.
>>
>> If this should be migrated to remove the use of the canned binary entirely, I fear I am not knowledgable enough on the test infrastructure to do that.
>>
>> This should probably have been stated in the test file.
>
> I don't know how gsym data is actually generated, so some of these suggestions may not make much sense.
> Option 1) Add the test to the new cross-project-tests test directory. This was created for tests that rely on clang/lld or whatever to generate valid test inputs from source, without the need for a canned binary. You'd include the source and build it directly at run-time. This would only work though if your test addresses are unlikely to change as changes occur in the compiler/linker. llvm-symbolizer was one of the primary motivations for this actually.
> Option 2) Check in the assembly required to build the input, and use llvm-mc and other tools (llvm-strip/llvm-gsymutil etc) to turn it into the desired output at runtime.
> Option 3) Generate the gsym data directly at run-time somehow.

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 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.

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

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

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105985