[PATCH] D74923: [DebugInfo]: Do not start parsing macinfo/macinfo.dwo if the section's are empty.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D74923#1885255 <https://reviews.llvm.org/D74923#1885255>, @SouraVX wrote:

> In D74923#1885240 <https://reviews.llvm.org/D74923#1885240>, @dblaikie wrote:
>
> > Does this matter? "parsing" an empty section would be successfully parsing zero contributions & dumping nothing. That seems like a fine implementation & avoids needing this special case to check the section size first.
>
>
> Sort of, consider this for a moment -- 
>  First we try to parse macinfo --- which for now is empty. assume
>  then `Macro` is constructed due to this call.
>
>   const DWARFDebugMacro *DWARFContext::getDebugMacinfo() {
>     if (Macro)
>       return Macro.get();
>   DWARFDataExtractor MacinfoData(*DObj, DObj->getMacinfoSection(), isLittleEndian(),
>                                    0);
>     Macro.reset(new DWARFDebugMacro());
>     Macro->parse(*this, MacinfoData, &Offset);
>     return Macro.get();
>
>
> Now after wards we try to parse/dump macro section using mostly similar function then,
>
>   const DWARFDebugMacro *DWARFContext::getDebugMacro() {
>   if (Macro)  -- since this is constructed previously, function call will return from here it self. hence no contents dumped
>       return Macro.get();
>   DWARFDataExtractor MacroData(*DObj, DObj->getMacroSection(), isLittleEndian(),
>                                    0);
>     ...
>
>
> BTW, in current implementation, My checks are heavy or the empty Macro construction/parsing/dumping is heavy ??


These should be stored in two separate variables - if the current "getDebugMacinfo" stores its state in a member called "Macro", then probably worth renaming that member variable to "Macinfo" so that the future "getDebugMacro" can store its state in a member named "Macro" and there's no confusion about which one is which.

By the sounds of it, storing them in the same variable would break dumping of mixed binaries that link together DWARFv4 using debug_macinfo and DWARFv5 using debug_macro, so that's no good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74923