[PATCH] D95232: Symbolizer - Teach symbolizer to work directly on object file.

[praveen velliengiri via Phabricator via llvm-commits]

pvellien added a comment.

In D95232#2572950 <https://reviews.llvm.org/D95232#2572950>, @dblaikie wrote:

> In D95232#2566553 <https://reviews.llvm.org/D95232#2566553>, @MaskRay wrote:
>
>> (CC @jhenderson @rnk )
>>
>> The issues with the `const std::string &` parameter `symbolize*` API:
>>
>> - The LLVMSymbolizer instance needs to construct `ObjectFile`, which may be a duplicate if the application has an existing `ObjectFile` instance.
>> - The file reading error is buried in the API. The application cannot easily tell there is an IO error or incorrect address error and may keep symbolizing more addresses after an IO error. In addition, the user may want to handle the IO error themselves.
>> - It does magic things (opening an auxiliary file) which may be of security concern for some usage: it constructs a .dsym path for a Mach-O file; it constructs a path from build ID for an ELF file; it constructs a `.gnu_debuglink` path if the section is present in an ELF file; it constructs a PDB path for a PE file. Personally I'd like the feature to open auxiliary files to be specified explicitly by the user on ELF.
>
> These ^ are the reasons you're suggesting it would be good to refactor the existing code to take an ObjectFile instead of a std::string file name?

Yes

>> The `const ObjectFile &` `symbolize*` does not have the ability to open magic auxiliary files.  This is not clear from the API.
>
> This ^ is a limitation of these newly proposed ObjectFile APIs? So they won't be entirely compatible with the existing string-based APIs? So it would be difficult/not possible to refactor the existing code to use these APIs?

I think we can pass the file with debuginfo as an additional parameter as suggested by @MaskRay regarding refactoring all the use-cases within llvm to new APIs I'm not sure how complicated it would be in tools such as symbolizer, objdump etc

>> If we want to keep just one set of `symbolize*` overloads and go for `const ObjectFile &`, we perhaps need to store `ObjectFile` handles instead of `StringRef` handles for LLVMSymbolizer's internal maps, and make `Expected<SymbolizableModule *> LLVMSymbolizer::getOrCreateModuleInfo(const std::string &ModuleName)` public.
>
> What's the current lifetime management of the StringRefs in these internal maps? If they come from multiple different external calls passing in strings, that seems like it'd have lifetime problems (might not be clear the caller needs to keep the underlying string data alive) & would have similar problems with ObjectFile lifetime semantics. But if that's the existing contract/complication, perhaps it's not causing any particular problems.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95232