[PATCH] D73086: [DWARF5] Added support for debug_macro section parsing and dumping in llvm-dwarfdump.
David Blaikie via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 2 19:03:21 PST 2020
dblaikie added a comment.
In D73086#1901390 <https://reviews.llvm.org/D73086#1901390>, @SouraVX wrote:
> Hi @dblaikie, It seems to that existing implementation also contains a bug. As the Declaration implies here/Please Correct me if I've mistaken/
>
> using MacroList = SmallVector<Entry, 4>;
>
> /// A list of all the macro entries in the debug_macinfo section. --- slightly ambigous
> std::vector<MacroList> MacroLists; -- collection of macrolist per CU's ? correct?.
>
>
> hence the dumping code --
>
> for (const auto &Macros : MacroLists) { -- iterate through all CU's MacroList ?
> for (const Entry &E : Macros) {
>
>
> But if you closely notice, all contributions from different CU's are ending up in `MacroList` as opposed to intended `MacroLists`. Hence, even `MacroLists` is vector, still contains only one `MacroList` regardless of the no. of CU's present in macinfo section. Is this functionality intended ? Then why a vector to be used here.
>
> This bug originates while parsing the content:
>
> MacroList *M = nullptr;
> while (data.isValidOffset(Offset)) {
> if (!M) {
> MacroLists.emplace_back();
> M = &MacroLists.back();
> }
>
>
> This minor patch removes the vector and still correctly dumps single/multiple CU's from macinfo section.
>
> diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugMacro.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugMacro.h
> index 7880bcdf688..4fe3bf43e79 100644
> --- a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugMacro.h
> +++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugMacro.h
> @@ -42,7 +42,7 @@ class DWARFDebugMacro {
> using MacroList = SmallVector<Entry, 4>;
>
> /// A list of all the macro entries in the debug_macinfo section.
> - std::vector<MacroList> MacroLists;
> + MacroList MacroLists;
>
> public:
> DWARFDebugMacro() = default;
> diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugMacro.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugMacro.cpp
> index 8cb259ebc62..fa7f69f16d8 100644
> --- a/llvm/lib/DebugInfo/DWARF/DWARFDebugMacro.cpp
> +++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugMacro.cpp
> @@ -17,8 +17,7 @@ using namespace dwarf;
>
> void DWARFDebugMacro::dump(raw_ostream &OS) const {
> unsigned IndLevel = 0;
> - for (const auto &Macros : MacroLists) {
> - for (const Entry &E : Macros) {
> + for (const Entry &E : MacroLists) {
> // There should not be DW_MACINFO_end_file when IndLevel is Zero. However,
> // this check handles the case of corrupted ".debug_macinfo" section.
> if (IndLevel > 0)
> @@ -51,21 +50,14 @@ void DWARFDebugMacro::dump(raw_ostream &OS) const {
> }
> OS << "\n";
> }
> - OS << "\n";
> - }
> }
>
> void DWARFDebugMacro::parse(DataExtractor data) {
> uint64_t Offset = 0;
> - MacroList *M = nullptr;
> while (data.isValidOffset(Offset)) {
> - if (!M) {
> - MacroLists.emplace_back();
> - M = &MacroLists.back();
> - }
> // A macro list entry consists of:
> - M->emplace_back();
> - Entry &E = M->back();
> + MacroLists.emplace_back();
> + Entry &E = MacroLists.back();
> // 1. Macinfo type
> E.Type = data.getULEB128(&Offset);
>
>
> Should we correct/fix this or is it intended part of functionality ?
Yeah, it looks like maybe the missing code that was probably intended (by the existence of MacroLists, etc), was this an assignment of null to M, here:
if (E.Type == 0) {
// Reached end of a ".debug_macinfo" section contribution.
M = nullptr;
continue;
}
& some test that dumps more than one contribution & shows they're dumped as separate results - though it seems the dumping code doesn't print anything at the start of a MacroList (between the "for MacroLists" and "for Macros" loop headers) - so it'd be impossible to test this change. Also, such a header would be useful/necessary for reading the DWARF in a meaningful way - currently there's no way to know which debug_macro contribution is associated with which CU - the DW_AT_macro_info attribute prints the offset, but without those offsets printed in the debug_macinfo section, there's no way to know where the macros for a given CU are. See the simple "0x00000000:" header printed at the start of debug_loc and debug_ranges contributions - something like that would be good.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73086/new/
https://reviews.llvm.org/D73086
More information about the llvm-commits
mailing list