[Lldb-commits] [PATCH] D42563: [lldb] attempt to fix DIERef::GetUID

[Alexander Shaposhnikov via Phabricator via lldb-commits]

alexshap added a comment.

@tberghammer

> ELF specific as nothing really ties SymbolFileDWARF with the ELF format

the only reason why it's currently guarded to ELF is the fact that 
for Mach-O uuid also encodes something and i didn't want to change that (see the comment in the old version of the code)
(although i don't know if it's up to date).

> Do you know who is using the partially incorrect value returned from here?

yes, i do. I mentioned in the summary one example:

> For example, inside SymbolFileDWARF::ParseVariablesForContext there is a call 
> DWARFDIE function_die = info->GetDIE(DIERef(sc.function->GetID(), this)) - and because of the "broken" cu_offset (inside DIERef) the method GetDIE returns an invalid DWARFDIE.

If necessary, i can provide more details.

> One possible idea what can provide a nice fix is to change SymbolFileDWARF::GetID() to return (DW_INVALID_OFFSET << 32) and then treat that value as a special case indicating that the compile unit offset >is invalid and we should look for the DIE based on the die_offset part of the ID only

yeah, i thought about this as well, one thing that makes me worry - is die_offset sufficient to uniquely identify DIE ? otherwise dropping cu_offset might lead to similar hard-to-debug/hard-to-fix bugs/collisions


Repository:
  rL LLVM

https://reviews.llvm.org/D42563