In many cases it is helpful to know at what address the resolved function starts.
This patch adds a new StartAddress member to the DILineInfo structure.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
This seems like a reasonable idea in principle to me, and the code as-is looks good for JSON output. I feel like it shouldn't be limited to just that output format however. I'm uncertain how to represent this in other output formats, but it may want to be through a new llvm-symbolizer option (e.g. --print-start-address), or maybe even it could just be part of the verbose output (since that is where the StartLine output is printed. What do you think?
llvm/lib/DebugInfo/DWARF/DWARFContext.cpp | ||
---|---|---|
1067–1069 | No need for braces for single-line if statements. |
it could just be part of the verbose output (since that is where the StartLine output is printed.
I like this more. Feels like we already have enough options.
What about compatibility with gnu adr2line? Unless I’m missing something, they do not offer such data. Do we care?
llvm/lib/DebugInfo/DWARF/DWARFContext.cpp | ||
---|---|---|
1067–1069 | Any thoughts on the behavior you want if a function has more than one code range (eg: PROPELLER or other uses of basic block sections - where the function will described with DW_AT_ranges instead of DW_AT_low_pc)? |
We definitely care about GNU compatibility for the GNU output style. There are two options here: 1) discuss on the GNU binutils mailing list whether they'd be interested in implementing the same feature, maybe with a concrete proposal of what the output would look like - if they accept it, model our output on theirs. 2) Add the output to the LLVM-style verbose output only (also to JSON output of course).
Don't forget to update the examples (and potentially text) in the docs to match whatever the new output is.
llvm/lib/DebugInfo/DWARF/DWARFContext.cpp | ||
---|---|---|
1067–1069 | I've got no concrete preference here currently, but I can think of the following options:
|
llvm/lib/DebugInfo/DWARF/DWARFContext.cpp | ||
---|---|---|
1067–1069 | Yeah, I'm OK with any of that - though (3) isn't really possible with LLVM's DWARF output - the list of blocks isn't ordered, so you can't find the first one only from inspecting the address ranges. (yes, I'm pretty sure in LLVM's output the first block in the list of ranges is the start of the function, but nothing in DWARF communicates/guarantees that) - there's a separate attribute that can be emitted on subprograms that's for the entry point address which could be used for this (or you can lookup the symbol table - if the function/executable has a symbol table - and use that address) |
Of course I though about multiple ranges case. And preferred keeping it simple for the beginning. This patch does not print StartAddress if DW_AT_low_pc is missing. That’s the case #1 in the above list of options. As already mentioned, in case of multiple ranges we do not know for sure where a function starts, at least not without an extra analyses of the code. We will need more solid use case and samples to move to that direction. Not a subject of this patch.
To summarize.
- I will add StartAddress to the verbose output for LLVM.
- If/when time permits I’ll follow up with gnu community with the proposal pointing to LLVM output as an example.
The patch is coming shortly.
By the way, llvm-symbolizer always uses symbol table. Opts.UseSymbolTable = true is hard coded and user has no way to control that.
Fair enough.
(re: symbol table - I meant for figuring out the address - I think the "UseSymbolTable" is for the names, not the addresses (see the comments in SymbolizableObjectFile::shouldOverrideWithSymbolTable))
Reminder:
Don't forget to update the examples (and potentially text) in the docs to match whatever the new output is.
llvm/lib/DebugInfo/Symbolize/DIPrinter.cpp | ||
---|---|---|
110 | I feel like it would be a good idea to reduce code duplication of this function. I'd suggest pulling the common bits out into separate static or private functions, that can be called as necessary. Example: void PlainPrinterBase::printVerbose(StringRef Filename, const DILineInfo &Info) { printFileAndFunctionStart(...); printLineColumnDiscriminator(...); } void LLVMPrinter::printVerbose(StringRef Filename, const DILineInfo &Info) { printFileAndFunctionStart(...); if (Info.StartAddress) { OS << " Function start address: 0x"; OS.write_hex(*Info.StartAddress); OS << '\n'; } printLineColumnDiscriminator(...); } Alternatively, put the start address printing in another virtual function, and then just have the PlainPrinterBase::printVerbose function callt that function. |
LGTM, thanks!
llvm/docs/CommandGuide/llvm-symbolizer.rst | ||
---|---|---|
377 ↗ | (On Diff #345980) | I don't think you need the word "start" here (we didn't before for the line part after all). |
No need for braces for single-line if statements.