[PATCH] D157459: Make DWARFContext more thread safe.

[Greg Clayton via Phabricator via llvm-commits]

clayborg added a comment.

In D157459#4589175 <https://reviews.llvm.org/D157459#4589175>, @bulbazord wrote:

> I think I'm somewhat on the same page as David here. The big concern I have is that although this probably works today, it's not very resilient or future-proof. It's difficult to guarantee future contributors or future reviewers will know or remember to guard member variable mutations with the mutex. I won't block this patch because I know the work to refactor the abstraction to be concurrency-safe is non-trivial, but I am concerned about the long term implications.

IMHO this is just creating thread safe code and is no different that any other patch. Unless there is a mandate in LLVM code to not create thread safe code for performance reasons? The implications would be that we, as reviewers, would be aware of this and any modifications to this file we would just tell people to use a LockGuard.



================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:1070-1094
 const DWARFDebugNames &DWARFContext::getDebugNames() {
   return getAccelTable(Names, *DObj, DObj->getNamesSection(),
                        DObj->getStrSection(), isLittleEndian());
 }
 
 const AppleAcceleratorTable &DWARFContext::getAppleNames() {
   return getAccelTable(AppleNames, *DObj, DObj->getAppleNamesSection(),
----------------
bulbazord wrote:
> I think either all of these getters either need to be protected or `getAccelTable` needs to be protected. They can all mutate these member variables.
Yes, these should be protected as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157459