DWO/DWP should not be indexed directly.
Instead, the corresponding base file should be used.
Details
Details
Diff Detail
Diff Detail
- Repository
- rL LLVM
Event Timeline
Comment Actions
Looks fine to me as long as you got a clean test suite run. Be sure to keep an eye on the buildbots after this goes in.
Comment Actions
How are you end up calling SymbolFileDWARFDwo::Index? If I remember correctly you are not supposed to index a dwo file directly because without the main object file you won't have all of the necessary information.
Comment Actions
@tberghammer - for example, any call of SymbolFileDWARF::FindCompleteObjCDefinitionTypeForDIE (with an object of the type SymbolFileDWARFDwo)
will trigger indexing of the dwo file.
@labath - yeah, i will add a test & update this patch
Comment Actions
I never tried debugging Objective-C using dwo but I am pretty sure this won't fix the issue you are seeing for FindCompleteObjCDefinitionTypeForDIE correctly because this way you will index the compile unit twice (once from the main object file and once from the dwo), then create 2 CompilerType for the 2 indexed version and will start hitting random issues in expression evaluation when clang will get confused by 2 declaration for the same type. If I am not mistaken then because of this, calling Index on a Dwo file is a pretty bad idea. Instead of trying to get it work we should change it to be an assert so people don't call it by accident.
I am not sure if it makes sense to call FindCompleteObjCDefinitionTypeForDIE on a SymbolFileDWARFDwo because generally you want to do the type lookup in a full object file and not only in a single dwo. Do you have a full stack trace for a scenario where this happens? I suspect that the problem is at a higher layer then you are trying to fix it.
If the problem is actually with FindCompleteObjCDefinitionTypeForDIE then the correct solution would be to override it in SymbolFileDWARFDwo with "return GetBaseSymbolFile()->FindCompleteObjCDefinitionTypeForDIE(...)" as we are already doing it for a few methods.
Comment Actions
@tberghammer, SymbolFileDWARF (the base class of SymbolFileDWARFDwo) calls Index()
"lazily" in may places, so indexing of dwo happens almost inevitably (at the moment)
(FindCompleteObjCDefinitionTypeForDIE is just one example).
because this way you will index the compile unit twice (once from the main object file and once from the dwo),
then create 2 CompilerType for the 2 indexed version and will start hitting random issues in
expression evaluation when clang will get confused by 2 declaration for the same type.
there are two separate CompileUnits here, not one,
There is a compile unit represented by m_base_dwarf_cu (roughly speaking for .o) and and there is another one
(which we can get, for example, via SymbolFileDWARFDwo::GetCompileUnit()) (roughly speaking for dwo).
(please, correct me if i'm wrong). For the latter GetOffset() would typically return 0,
for the former GetOffset() typically returns the higher 32 bits of Id.
Comment Actions
i'd like to think about this problem a little bit more (maybe i'm missing smth) + as i said above - will add a test
Comment Actions
The main point is that you should almost always interact with the SymbolFileDWARF instance belonging to the main object file and it (actually DWARFCompileUnit) will know when to access data from the Dwo file instead. The original goal was to not leak pointers to SymbolFileDWARFDwo instances out from symbol file dwarf and the dwarf parsing logic.
because this way you will index the compile unit twice (once from the main object file and once from the dwo),
then create 2 CompilerType for the 2 indexed version and will start hitting random issues in
expression evaluation when clang will get confused by 2 declaration for the same type.there are two separate CompileUnits here, not one,
There is a compile unit represented by m_base_dwarf_cu (roughly speaking for .o) and and there is another one
(which we can get, for example, via SymbolFileDWARFDwo::GetCompileUnit()) (roughly speaking for dwo).
(please, correct me if i'm wrong). For the latter GetOffset() would typically return 0,
for the former GetOffset() typically returns the higher 32 bits of Id.
We have 2 DWARFCompileUnit but actually they belong to the same "compile time compile unit" so when you call Index on the one in the main object file (SymbolFileDWARF instance) it will read the data in from the Dwo file as well, add that data into its index and create some stub types in clang (they will be filled in later lazily).
Comment Actions
The main point is that you should almost always interact with the SymbolFileDWARF instance belonging to the main object file and it
(actually DWARFCompileUnit) will know when to access data from the Dwo >file instead.
The original goal was to not leak pointers to SymbolFileDWARFDwo instances out from symbol file dwarf and the dwarf parsing logic.
i see ur point, let me think a bit more about it.
One thing which i have on my mind - with that approach it's not clear (not the right wording, but anyway) how would "recursive" queries work. Whenever the "owner" SymbolFileDWARF interacts with
the SymbolFileDWARFDwo (obtained by cu->GetDwoSymbolFile()) the latter (SymbolFileDWARFDwo) may try to query itself (internally), and we will face some other issues again (if i am not mistaken).
Comment Actions
Change the approach: add assert to ensure we don't double index .dwo files,
rerun the tests.
@tberghammer - many thanks for the suggestions.
p.s. didn't add new tests (in this commit) - standalone repo is not trivial (objc on linux) and requires more work - probably would prefer to do this later as a follow-up (i.e. at the moment dwps are not tested yet either)