[Lldb-commits] [PATCH] D114288: [NFC] Refactor symbol table parsing.

[Greg Clayton via Phabricator via lldb-commits]

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