[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #92328)

David Blaikie via lldb-commits lldb-commits at lists.llvm.org
Tue Jun 4 11:34:21 PDT 2024


dwblaikie wrote:

> > > You can repro this by running ./bin/lldb-dotest -p TestConstStaticIntegralMember.py --dwarf-version 5
> > > Could you take a look @ZequanWu ?
> > 
> > 
> > It should be fixed by the following diff. We just need to skip the declaration DIEs that are struct/class/union. This failure you see is caused by skipping a declaration DIE `DW_TAG_variable`.
> > ```
> > diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
> > index 56717bab1ecd..6330470b970e 100644
> > --- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
> > +++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
> > @@ -87,7 +87,8 @@ bool DebugNamesDWARFIndex::ProcessEntry(
> >      return true;
> >    // Clang erroneously emits index entries for declaration DIEs in case when the
> >    // definition is in a type unit (llvm.org/pr77696). Weed those out.
> > -  if (die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0))
> > +  if (die.IsStructUnionOrClass() &&
> > +      die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0))
> >      return true;
> >    return callback(die);
> >  }
> > ```
> > 
> > 
> >     
> >       
> >     
> > 
> >       
> >     
> > 
> >     
> >   
> > Can you (or anyone with commit access) commit above fix for me? I somehow cannot pull/push from github.
> 
> Ah right, I remember encountering these `DW_AT_declaration`s in the index for DWARFv5 static member variables. Technically this isn't standard conforming right? I thought anything `DW_AT_declaration` shouldn't be indexed (though in the case of these static variables it's very useful that they do, because we don't emit definitions for these constants that the debugger could look for instead). @dwblaikie @felipepiovezan Should we allow such entries in the index? And does this warrant rephrasing in the DWARF spec?

At least we aren't producing that on x86 from the compiler: https://godbolt.org/z/ereKsasWf
```
...

0x00000029:     DW_TAG_variable
                  DW_AT_name	("i")
                  DW_AT_type	(0x00000033 "const int")
                  DW_AT_decl_file	("/app/example.cpp")
                  DW_AT_decl_line	(2)
                  DW_AT_external	(true)
                  DW_AT_declaration	(true)
                  DW_AT_const_value	(3)
...
```
and the `.debug_names` only includes `t1`, `main`, and `int`, nothing for `i`.

Ah, right - some of the previous context on this is https://github.com/llvm/llvm-project/pull/70639 (which got reverted in https://github.com/llvm/llvm-project/pull/74580 )- though I guess we probably had some discourse and dwarf workgroup discussions about how to do this that aren't likned from the PR... maybe it was all private chat, not sure.

https://github.com/llvm/llvm-project/pull/92328


More information about the lldb-commits mailing list