[LLVM][DWARF] Create thread safe context for DWP DWARFContext

[ayermolo]

Right now DWARFContext that is created is the same when input is DWO or DWP file.
Changed so that for DWP it creates a thread safe context. This allows for a multi threaded safe access to {cu,tu}-index

[llvmbot]

@llvm/pr-subscribers-debuginfo

Changes

Right now DWARFContext is created is the same when input is DWO or DWP file.
Changed so that for DWP it creates a thread safe context.

---

Full diff: https://github.com/llvm/llvm-project/pull/68262.diff

1 Files Affected:

• (modified) llvm/lib/DebugInfo/DWARF/DWARFContext.cpp (+5-2)

diff --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
index 1e1ab814673f423..6a91be97255b8bb 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
@@ -575,8 +575,11 @@ class ThreadUnsafeDWARFContextState : public DWARFContext::DWARFContextState {
 
     auto S = std::make_shared<DWOFile>();
     S->File = std::move(Obj.get());
-    S->Context = DWARFContext::create(*S->File.getBinary(),
-                                      DWARFContext::ProcessDebugRelocations::Ignore);
+    StringRef FileName = S->File.getBinary()->getFileName();
+    S->Context = DWARFContext::create(
+        *S->File.getBinary(), DWARFContext::ProcessDebugRelocations::Ignore,
+        nullptr, "", nullptr, nullptr,
+        FileName.find(".dwp") != StringRef::npos);
     *Entry = S;
     auto *Ctxt = S->Context.get();
     return std::shared_ptr<DWARFContext>(std::move(S), Ctxt);

[ayermolo]

@dwblaikie @clayborg ping :D

[dwblaikie]

Not sure I follow - wouldn't it be suitable for a ThreadUnsafeDWARFContext to create a ThreadUnsafeDWARFContext for the DWP? And only a ThreadSafeDWARFContext would create a ThreadSafeDWARFContext for the DWP?

[clayborg]

Not sure I follow - wouldn't it be suitable for a ThreadUnsafeDWARFContext to create a ThreadUnsafeDWARFContext for the DWP? And only a ThreadSafeDWARFContext would create a ThreadSafeDWARFContext for the DWP?

No one is creating a thread safe context right now except llvm-gsymutil. So we could enable thread safe everywhere, but people seemed to think there would be some performance hit if that was done.

[clayborg]

Not sure I follow - wouldn't it be suitable for a ThreadUnsafeDWARFContext to create a ThreadUnsafeDWARFContext for the DWP? And only a ThreadSafeDWARFContext would create a ThreadSafeDWARFContext for the DWP?

No one is creating a thread safe context right now except llvm-gsymutil. So we could enable thread safe everywhere, but people seemed to think there would be some performance hit if that was done.

If this issue here was we had a thread safe DWARContext for the main DWARF, and this wasn't being correctly propagated into the .dwp DWARFContext, then this should be fixed by detecting the thread safe setting from the DWARContext and using that setting for any child DWARFContext (.dwp and any .dwo files that are not in a .dwp file) objects.

[dwblaikie]

Not sure I follow - wouldn't it be suitable for a ThreadUnsafeDWARFContext to create a ThreadUnsafeDWARFContext for the DWP? And only a ThreadSafeDWARFContext would create a ThreadSafeDWARFContext for the DWP?

No one is creating a thread safe context right now except llvm-gsymutil. So we could enable thread safe everywhere, but people seemed to think there would be some performance hit if that was done.

If this issue here was we had a thread safe DWARContext for the main DWARF, and this wasn't being correctly propagated into the .dwp DWARFContext, then this should be fixed by detecting the thread safe setting from the DWARContext and using that setting for any child DWARFContext (.dwp and any .dwo files that are not in a .dwp file) objects.

But this code change is being made in the ThreadUnsafeDWARFContextState - the thread unsafe class is making a thread safe DWP context. That doesn't seem right? Shouldn't only a ThreadSafeDWARFContextState create a thread safe context for the DWP?

[clayborg]

So sounds like we need to change this patch to propagate the "ThreadSafe" from one context to any child DWARFContext things (like for owned .dwp and .dwo contexts. I would recommend adding a new virtual function to DWARFContext:

virtual bool IsThreadSafe() = 0;

Then have ThreadUnsafeDWARFContextState override this and return false, and ThreadSafeState override it return true. Then it will be easy to propagate the setting using the current "DWARFContext::State" iivar from a DWARFContext to create another.

[ayermolo]

OK, will make the change so that DWO/DWP DWARFContext is controlled by main binary DWARFContext thread safetiness.

[dwblaikie]

Sounds OK