clayborg added a comment.

In D114288#3149188 <https://reviews.llvm.org/D114288#3149188>, @labath wrote:

> I don't believe this solution is correct.
>
> How did this work before? Is it because `ObjectFileELF::GetSymtab` contained 
> the same (incorrect) unique_ptr pattern?

It did, but because each ObjectFile subclass did things there own way, it 
worked only because ObjectFileELF was never taking the module lock to begin 
with. All others object files did.

> How about we just prime the symtab (or whatever is needed) before we start 
> the thread pool indexing?

Doesn't work, I tried that. The problem is the way the .dwo file stuff 
currently works. With some of the tests, we are loading a .dwo file as the main 
module. In this cause we end up with a target module, then ends up opening up a 
".dwo" module as a _separate_ object file in the DWO support in the DWARF 
classes (not the same ObjectFile that the target Module has, but a different 
one), but this extra object file is saying its module is the main module. I 
didn't make any of the DWO changes to our DWARF classes, so I am still coming 
up to speed with why this weird case where two different object files claim the 
same module is their owning module. It is probably  because of the way the test 
was written. But that being said, we should be able to load the .dwo files as a 
main module to inspect what we can out of them. Do all .dwo files claim the 
main executable that references them is their owning module? If so, many other 
deadlocks can happen during indexing.

Regardless of this problem, the DWARF indexing has caused deadlocks before due 
to the module lock. Like the the Module::GetDescription(...) when we enabled 
DWARF logging. So even if we solve this one problem, we can run into it again 
the next time DWARF indexing causes some file to load/fetch something. For many 
of the things inside of the module that are populated once (section list, 
symbol table, etc), we will run into issues with any threading code do the 
module lock.

One way to possibly fix this is to stop the Module class from using the mutex 
when populating these "compute once and then hand out an object" and have them 
use a llvm::once_flag as a member variable to control access. We might need to 
pull the same trick I do with the ObjectFile where the llvm::once_flag is put 
into a std::unique_ptr<llvm::once_flag> so it can be cleared if the this object 
that has handed out can be cleared or changed (like the trick I used in 
ObjectFile::ClearSymtab()).

So the fix is not straight forward if we aren't comfortable with relying on the 
Symtab mutex to protect the symbol table that I proposed in this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114288

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to