This patch adds support for DWARFv5 new forms: DW_MACRO_define_strx, DW_MACRO_undef_strx.
We could also remove "offset_is_64_bit" if we fix DataExtractor to handle it as suggested below where "offset_is_64_bit" is used.
I have though about adding support to DataExtractor for extracting offsets. Anyone that initializes a DataExtractor would need to set the offset size once, and then everyone else could just call:
new_offset = debug_macro_data.GetOffset(offset);
We would need to add a field to DataExtractor:
Optional<uint32_t> m_offset_size; ///< The address size to use when extracting offsets. Defaults to m_addr_size if this has no value
It would default to the m_addr_size if m_offset_size isn't set. The "m_context.getOrLoadXXXX()" calls would then set the offset size correctly for the current DWARF when it creates the DWARFDataExtractor.
To future proof this a bit should we pass in the "lldb_private::DWARFContext" instead of the first three arguments?
Move this into DWARFDebugMacroEntry::ReadMacroEntries() so that we don't have to pass "header" as an argument?
Presumably because the input is X86 assembly. That said, would it be possible to test the feature without constructing a process, so we *can* test it everywhere?
I have a high-level question/comment. Are you planning to implement debug_macro reading in llvm-dwarfdump? Even if you aren't, I am expecting that you will be asked to do that as a part of testing for your debug_macro generation patch... And in that case, it would be much better to write a debug_macro parser in first, and reuse that in lldb, similar to how its done for rnglists, loclists, and other "minor" sections. The slightly tricky part there is that you need to make sure to not intertwine it too much with other llvm dwarf classes, as lldb does not use all of llvm dwarf (yet). However, given that the debug_macro section is pretty independent, that shouldn't be too much of a problem?
Surely, this should be set according to the actual str_offsets_base value, no?
Agreed, however I think this enhancement has to be done separately.
I tried, other ways also. Seems like this is bare minimum, We need to run the program to be able to expand macros. analogous behavior with GDB also.
Hi Pavel, I've mostly completed debug_macro generation[clang/llvm] and dumping it using llvm-dwarfdump. I'll soon file a review for that also. Maybe we can address this concern while reviewing that. Till that, I think it would be nice to have minimum support for macros.
And in that case, it would be much better to write a debug_macro parser in first, and reuse that in lldb, similar to how its done for rnglists, loclists, and other "minor" sections. The slightly tricky part there is that you need to make sure to not intertwine it too much with other llvm dwarf classes, as lldb does not use all of llvm dwarf (yet). However, given that the debug_macro section is pretty independent, that shouldn't be too much of a problem?
I don't think this issue is particularly urgent -- a consumer is not very useful if you don't have a producer which can produce that data. And reusing the parser in llvm would allow us to largely drop the testing part of this discussion -- if the nitty-gritty encoding details are tested in llvm, then here we just need to make sure the integration works, and we already mostly have a test for that (TestMacros.py).
It wouldn't be unreasonable to add some command to query active macro definitions at a given point in the program (similar to gdb's info macros command). In lldb, the best place to plug this into might be the "image lookup" command -- right now "image lookup --verbose --file a.cpp --line 10" will give you the function containing that line as well as all variables which are active at that point. It seems natural to also list the active macros too.
However, that would most likely require adding a new "macros" field to the SymbolContext class, which is a concept used throughout lldb, and so doing it may be fairly tricky and it may be better to create a separate command for this at first ("image dump macros"?)...
(also note that in the current form, this test will not only require x86, but also a linux (or at least elf) system)