[PATCH] D136039: [DebugInfo] Fix potential CU mismatch for attachRangesOrLowHighPC
DianQK via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sat Oct 22 06:20:35 PDT 2022
DianQK marked 4 inline comments as done.
DianQK added a comment.
Thank you very much for your help!
================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:1016
LexicalScope *Scope) {
- DIE &ScopeDIE = updateSubprogramScopeDIE(Sub);
+ DIE *SPDie = getOrCreateSubprogramDIE(Sub, includeMinimalInlineScopes());
+ DwarfCompileUnit *ContextCU =
----------------
dblaikie wrote:
> DianQK wrote:
> > dblaikie wrote:
> > > Hmm, I'm not following - can you help me understand why this got pulled out of `updateSubprogramScopeDIE`?
> > I need get `SPDie` to get `ContextCU` before `attachRangesOrLowHighPC`.
> > The `attachRangesOrLowHighPC` call in `updateSubprogramScopeDIE` and later `createAndAddScopeChildren`.
> >
> > Maybe I could use the following code:
> > ```
> > DIE *SPDie = getOrCreateSubprogramDIE(Sub, includeMinimalInlineScopes());
> > auto *ContextCU = static_cast<DwarfCompileUnit *>(SPDie->getUnit());
> > DIE &ScopeDIE = ContextCU->updateSubprogramScopeDIE(Sub);
> > ```
> >
> > I tried. It fails at `cross-cu-inlining-2.ll` (maybe because call `getOrCreateSubprogramDIE` in different CU?).
> >
> > Dwarf before the change is:
> > ```
> > 0x00000038: DW_TAG_subprogram [5] * (0x0000002b)
> > DW_AT_low_pc [DW_FORM_addr] (0x0000000000000010 ".text")
> > DW_AT_high_pc [DW_FORM_data4] (0x00000001)
> > DW_AT_frame_base [DW_FORM_exprloc] (DW_OP_reg7 RSP)
> > DW_AT_name [DW_FORM_strp] ( .debug_str[0x00000036] = "bar")
> > DW_AT_decl_file [DW_FORM_data1] ("A.swift")
> > DW_AT_decl_line [DW_FORM_data1] (21)
> > DW_AT_external [DW_FORM_flag_present] (true)
> > ```
> > After:
> > ```
> > 0x00000038: DW_TAG_subprogram [5] (0x0000002b)
> > DW_AT_name [DW_FORM_strp] ( .debug_str[0x00000036] = "bar")
> > DW_AT_decl_file [DW_FORM_data1] ("A.swift")
> > DW_AT_decl_line [DW_FORM_data1] (21)
> > DW_AT_external [DW_FORM_flag_present] (true)
> >
> > 0x0000003f: DW_TAG_subprogram [6] * (0x0000002b)
> > DW_AT_low_pc [DW_FORM_addr] (0x0000000000000010 ".text")
> > DW_AT_high_pc [DW_FORM_data4] (0x00000001)
> > DW_AT_frame_base [DW_FORM_exprloc] (DW_OP_reg7 RSP)
> > ```
> >
> > I think it's ok. But the old version is better?
> >
> Right, the old version is better. It could be made to work. Something like this:
> ```
> diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
> index f2551e1f3419..b2fe2cf396e1 100644
> --- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
> +++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
> @@ -443,8 +443,14 @@ void DwarfCompileUnit::attachLowHighPC(DIE &D, const MCSymbol *Begin,
> // Find DIE for the given subprogram and attach appropriate DW_AT_low_pc
> // and DW_AT_high_pc attributes. If there are global variables in this
> // scope then create and insert DIEs for these variables.
> -DIE &DwarfCompileUnit::updateSubprogramScopeDIE(const DISubprogram *SP,
> - DIE *SPDie) {
> +DIE &DwarfCompileUnit::updateSubprogramScopeDIE(const DISubprogram *SP) {
> + DIE *SPDie = getOrCreateSubprogramDIE(SP, includeMinimalInlineScopes());
> + auto *ContextCU = static_cast<DwarfCompileUnit *>(SPDie->getUnit());
> + return ContextCU->updateSubprogramScopeDIEImpl(SP, SPDie);
> +}
> +
> +DIE &DwarfCompileUnit::updateSubprogramScopeDIEImpl(const DISubprogram *SP, DIE *SPDie) {
> +
> SmallVector<RangeSpan, 2> BB_List;
> // If basic block sections are on, ranges for each basic block section has
> // to be emitted separately.
> @@ -1013,9 +1019,8 @@ sortLocalVars(SmallVectorImpl<DbgVariable *> &Input) {
>
> DIE &DwarfCompileUnit::constructSubprogramScopeDIE(const DISubprogram *Sub,
> LexicalScope *Scope) {
> - DIE *SPDie = getOrCreateSubprogramDIE(Sub, includeMinimalInlineScopes());
> - auto *ContextCU = static_cast<DwarfCompileUnit *>(SPDie->getUnit());
> - DIE &ScopeDIE = ContextCU->updateSubprogramScopeDIE(Sub, SPDie);
> + DIE &ScopeDIE = updateSubprogramScopeDIE(Sub);
> + auto *ContextCU = static_cast<DwarfCompileUnit *>(ScopeDIE.getUnit());
>
> if (Scope) {
> assert(!Scope->getInlinedAt());
> diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
> index 0b0d97e74bc4..d346c79b5f7b 100644
> --- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
> +++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
> @@ -191,7 +191,8 @@ public:
> /// DW_AT_low_pc and DW_AT_high_pc attributes. If there are global
> /// variables in this scope then create and insert DIEs for these
> /// variables.
> - DIE &updateSubprogramScopeDIE(const DISubprogram *SP, DIE *SPDie);
> + DIE &updateSubprogramScopeDIE(const DISubprogram *SP);
> + DIE &updateSubprogramScopeDIEImpl(const DISubprogram *SP, DIE *D);
>
> void constructScopeDIE(LexicalScope *Scope, DIE &ParentScopeDIE);
>
> ```
>
> But yeah, since this is the only caller, seems simpler to do it here as you've done. Thanks for walking me through it.
Thanks, I have a patch added a `constructSubprogramScopeDIEImplused` to reduce `ContextCU` using that was not published, Because I found some code add `*Impl` doing similar things.
But only two locations need `ContextCU`, so I removed this.
> But why did I forget to use this on `updateSubprogramScopeDIE`? I'm being foolish.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136039/new/
https://reviews.llvm.org/D136039
More information about the llvm-commits
mailing list