[PATCH] D106624: [WIP][DWARF] Don't process .debug_info relocations for DWO Context

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D106624#2901747 <https://reviews.llvm.org/D106624#2901747>, @ayermolo wrote:

> In D106624#2901386 <https://reviews.llvm.org/D106624#2901386>, @dblaikie wrote:
>
>>> If relocations for that exist then sure we should apply them (is that even a thing?)
>>
>> No, relocations don't exist for dwo sections (if there are any relocations for dwo sections, that's a bug in the producer) - so skipping all relocation application for .dwo sections is fine/correct (could error out if there are relocation sections for dwo sections, though, if you like - but probably should do that in a separate patch).
>
> This is related to your question about (IsDWOContext && !Name.contains(".dwo") check. Why I though both of them are needed. Looking at code on line 1776
>
>   else if (RelSecName == "debug_info.dwo")
>             Map = &static_cast<DWARFSectionMap &>(
>                        InfoDWOSections[*RelocatedSection])
>                        .Relocs;
>
> I thought there might be relocations for .debug_info.dwo so I structured a check in two parts flag plus ".dwo" so that if context is DWO and relocations for ".dwo" exist it will still process them.
> Now I am confused. If relocations don't exist for dwo, why is that code there?
> I traced it to https://reviews.llvm.org/D53907. Which seems to be DWARF V5 related.

Yeah, I think that's just added for symmetry - probably copying what was done for .debug_types.dwo too (which I probably wrote) which also doesn't need relocation handling.

> If there are no relocations for .dwo then just checking IsDWOContext is enough, and !Name.contains(".dwo") is not necessary.

Yeah, if you could remove that (& if you're willing, a separate patch to warn about relocations on .dwo sections) check, that'd be great!

Might be worth replacing the bool "IsDWO" parameters with an enum to make the callsites more legible rather than an anonymous "false" value? (might also be an opportunity to/worth creating a 3rd value for the enum - some kind of "indeterminate" state (& that one might be good to skip and warn on relocations in dwo sections) - this would come up when directly dumping a .dwo file, I think?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106624