Make DWARFContext more thread safe.

[clayborg]

Summary

llvm-gsymutil uses a DWARFContext from multiple threads as it parses each compile unit. As it finds issues it might end up dumping a DIE to an output stream which can cause accesses to the DWARFContext from multiple threads. In llvm-gsymutil it can end up dumping a DIE from multiple threads into thread specific stream which was causing DWARFContext::getTUIndex() to be called and can crash the process.

This fix puts a recursive mutex into the DWARFContext class and makes most APIs threadsafe for access. Many of the methods in DWARFContext will check if a member variable has been filled in yet, and parse what is needed and populate a member variagle with the results. Now a mutex protects these functions.

[dblaikie]

Not sure how I feel about adding multithreading at this level, versus in a wrapper API for those that need it? (insert an interface (DWARFContext, then rename the current concrete implementation as DWARFContextImpl) then have a ThreadSafeDWARFContext that wraps (either as a template, or with a unique_ptr<DWARFContext> member) another DWARFContext and adds the locking) Though it's probably not the end of the world to add it here directly.

Can probably use Class Template Argument Deduction when locking, like this:

std::unique_lock LockGuard(Mutex);

[clayborg]

Switch to using "std::unique_lock LockGuard(Mutex);" and let template argument deduction happen.

[clayborg]

In D157459#4573865, @dblaikie wrote:

Not sure how I feel about adding multithreading at this level, versus in a wrapper API for those that need it? (insert an interface (DWARFContext, then rename the current concrete implementation as DWARFContextImpl) then have a ThreadSafeDWARFContext that wraps (either as a template, or with a unique_ptr<DWARFContext> member) another DWARFContext and adds the locking) Though it's probably not the end of the world to add it here directly.

I am fine with either way. The main issue about having a class that wraps a DWARFContext is internally the DWARFDie and DWARFUnit and possibly others, have access to their DWARFContext via the DWARFUnit and they can call some of these APIs. So wrapping won't really help for these important cases. Seems cleaner to me to have the mutex live in this class, but I am open to everyone's comments to figure out what is best for the codebase.

Can probably use Class Template Argument Deduction when locking, like this:

std::unique_lock LockGuard(Mutex);

Good idea, I fixed all locations as suggested.

[clayborg]

I ran into this issue when I had multiple threads trying to call DWARFDie::dump(...). This internally got ahold of the DWARFContext and calls DWARFContext::getTUIndex() when trying to dump a DW_AT_type attribute since it prints the DIE offset and then finds the referenced DIE to get the name and print the typename.

[ayermolo]

In D157459#4573865, @dblaikie wrote:

Not sure how I feel about adding multithreading at this level, versus in a wrapper API for those that need it? (insert an interface (DWARFContext, then rename the current concrete implementation as DWARFContextImpl) then have a ThreadSafeDWARFContext that wraps (either as a template, or with a unique_ptr<DWARFContext> member) another DWARFContext and adds the locking) Though it's probably not the end of the world to add it here directly.

Can probably use Class Template Argument Deduction when locking, like this:

std::unique_lock LockGuard(Mutex);

Is there a downside just adding it here, instead of adding complexity of another class and wrapper APIs?

[dblaikie]

In D157459#4578926, @ayermolo wrote:

In D157459#4573865, @dblaikie wrote:

Not sure how I feel about adding multithreading at this level, versus in a wrapper API for those that need it? (insert an interface (DWARFContext, then rename the current concrete implementation as DWARFContextImpl) then have a ThreadSafeDWARFContext that wraps (either as a template, or with a unique_ptr<DWARFContext> member) another DWARFContext and adds the locking) Though it's probably not the end of the world to add it here directly.

Can probably use Class Template Argument Deduction when locking, like this:

std::unique_lock LockGuard(Mutex);

Is there a downside just adding it here, instead of adding complexity of another class and wrapper APIs?

Nothing drastic that I can think of - though perhaps a more vague "DWARFContext is already... not great (exactly which entry points you should use when, what guarantees they provide, when you need to parse things up-front versus lazily, how the memory is managed (mostly it just grows unbounded for instance/never gets cleaned up short of throwing the whole context away) and I worry that trying to figure out when/where multithreaded use is valid or not will add another dimension on an already uncertain and complex interface". But it's probably not the straw that breaks the camel's back, as it were - and adding a wrapper isn't really going to fix the underlying problems/make them especially easier to fix (it's probably a fairly broad rewrite at some point).

[ayermolo]

In D157459#4580929, @dblaikie wrote:

In D157459#4578926, @ayermolo wrote:

In D157459#4573865, @dblaikie wrote:

Not sure how I feel about adding multithreading at this level, versus in a wrapper API for those that need it? (insert an interface (DWARFContext, then rename the current concrete implementation as DWARFContextImpl) then have a ThreadSafeDWARFContext that wraps (either as a template, or with a unique_ptr<DWARFContext> member) another DWARFContext and adds the locking) Though it's probably not the end of the world to add it here directly.

Can probably use Class Template Argument Deduction when locking, like this:

std::unique_lock LockGuard(Mutex);

Is there a downside just adding it here, instead of adding complexity of another class and wrapper APIs?

Nothing drastic that I can think of - though perhaps a more vague "DWARFContext is already... not great (exactly which entry points you should use when, what guarantees they provide, when you need to parse things up-front versus lazily, how the memory is managed (mostly it just grows unbounded for instance/never gets cleaned up short of throwing the whole context away) and I worry that trying to figure out when/where multithreaded use is valid or not will add another dimension on an already uncertain and complex interface". But it's probably not the straw that breaks the camel's back, as it were - and adding a wrapper isn't really going to fix the underlying problems/make them especially easier to fix (it's probably a fairly broad rewrite at some point).

I see. Thank you for explanation.
Sounds like it might be nice to have, but not a blocker? As you said wrapper is not going to fix an underlying problem with this class.

[aprantl]

This patch is protecting access to the fields of DWARFContext. For all the methods returning section pointers, such as const DWARFDebugLoc *DWARFContext::getDebugLoc() what are concurrency requirements/guarantees? Do these objects all carry their own mutexes?

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

49

Nit: ///

[clayborg]

In D157459#4585664, @aprantl wrote:

This patch is protecting access to the fields of DWARFContext.

Yes, this is designed to only protect members of DWARFContext.

For all the methods returning section pointers, such as const DWARFDebugLoc *DWARFContext::getDebugLoc() what are concurrency requirements/guarantees? Do these objects all carry their own mutexes?

If you add more than one mutex then you can deadlock your program due to A/B locking issues. The current fix in DWARFContext::getDebugLoc() ensures that a complete "Loc" member variable is populated in the current DWARFContext object and ensure any other threads wait until the first thread completes the "Loc" contents.

[bulbazord]

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.

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

1441–1459

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.

[clayborg]

Make all accelerator table get accessors thread safe as they cache results in member variables.

[clayborg]

In 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.

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

1441–1459

Yes, these should be protected as well.

[JDevlieghere]

To Adrian's point: what are the guarantees provided by the DWARFContext? I'm slightly worried about overhead but care more about providing clear expectations. For example, the LLVMContext explicitly provides no locking guarantees [1] and leaves it up to the consumer.

[1] https://llvm.org/doxygen/classllvm_1_1LLVMContext.html#details

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

50

Does this have to be a recursive mutex? I was under the impression that they're somewhat frowned upon. I think the CppCoreGuidelines called it something along the lines of working around bad reasoning and adding unnecessary overhead.

[dblaikie]

In D157459#4590028, @clayborg wrote:

IMHO this is just creating thread safe code and is no different that any other patch.

Ish. It's adding a fairly subtle & incomplete invariant to a fairly complex API (with a bunch of fairly subtle invariants already). Even if this weren't about multithreading, but some other similarly subtle invariant, I think there'd still be similar reasonable concerns.

(DWARFContext's API is wide, this locking only protects it from certain multithreading, and not others - that's what I mean by subtle and incomplete - and so it's going to be difficult to keep track of/maintain this invariant going forward - as @JDevlieghere points out, I think, it'd be hard to document what DWARFContext's guarantees are after/with this patch, because I think it's still a fairly arbitrary selection of functions that can be used in a multithreaded environment)

[bulbazord]

In D157459#4590028, @clayborg wrote:

In 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.

Apologies if I came across as pointed. I appreciate that you are working to make LLVM more thread-safe and I don't think any mandate that would prioritize performance over safety would stand up to scrutiny. I agree that the implication would be that reviewers should and hopefully would understand that access to member variables of DWARFContext need to be synchronized and guarded with a mutex. What I wanted to convey was what @JDevlieghere and @dblaikie are saying.

I also think that in the case of DWARFDebugLine::LineTable a mutex is not enough because you can pass out a const DWARFDebugLine::LineTable * to somebody on one thread and on another thread right afterwards you can call DWARFDebugLine::clearLineTableForUnit, meaning the first thread is left holding a dangling pointer. Future changes to DWARFContext and introduce similar issues with other member variables and it may be difficult to notice.

[clayborg]

In D157459#4590168, @JDevlieghere wrote:

To Adrian's point: what are the guarantees provided by the DWARFContext? I'm slightly worried about overhead but care more about providing clear expectations. For example, the LLVMContext explicitly provides no locking guarantees [1] and leaves it up to the consumer.

I hear you on the complexity and by no means is this a patch that adds complete thread safety to the full DWARF parser.

If an API is simple for someone to wrap up in a thread safe class that contains this object, not sure if LLVMContext is or not, then it is ok to say "I will leave this up to the user". But right now any objects owned by DWARFContext, like DWARFUnit and DWARFDie, can make requests from the DWARFContext at any time that cause crashes as I found in llvm-gsymutil when it parses DWARFDie from each DWARFUnit on separate threads. So it would be hard to wrap up any of the DWARF classes effectively. The crash I ran into was because two different threads tried to dump a DWARFDie and this causes a DWARFContext to mess itself up when calling one of the functions that are now protected by the mutex.

In D157459#4590223, @dblaikie wrote:

In D157459#4590028, @clayborg wrote:

IMHO this is just creating thread safe code and is no different that any other patch.

Ish. It's adding a fairly subtle & incomplete invariant to a fairly complex API (with a bunch of fairly subtle invariants already). Even if this weren't about multithreading, but some other similarly subtle invariant, I think there'd still be similar reasonable concerns.

(DWARFContext's API is wide, this locking only protects it from certain multithreading, and not others - that's what I mean by subtle and incomplete - and so it's going to be difficult to keep track of/maintain this invariant going forward - as @JDevlieghere points out, I think, it'd be hard to document what DWARFContext's guarantees are after/with this patch, because I think it's still a fairly arbitrary selection of functions that can be used in a multithreaded environment).

Indeed. I have a specific crash I am trying to fix here for llvm-gsymutil so it can dump DWARFDie objects into error messages. Right now, if you look at the DWARFTransformer, the current code completely just tries to avoid doing anything that might cause threading issues by doing things serially like:

• parsing all CU DIE trees up front on the main thread to avoid crashing
• parse all line tables for each CU on the main thread to avoid crashing

I was able to make this work, but it is far from efficient, but I worked around it using the "it is up to the client to handle threading issues" mantra. But when I ran into problems being able to dump DIEs, that is something I can't do up front. The issue I ran into was the DWARFDie class was asking for the TUIndex from the DWARFContext, so I tried to fix this by parsing all of the TUIndexes on the main thread, but it still crashed due to split DWARF as each .dwo file had its own DWARFContext. So the only solution was to then parse all compile units up front, then grab the DWARFContext from each .dwo file and call all of the needed functions on it to avoid crashes (like calling the getTUIndex() method) and hope this is enough to avoid crashing. So it got quite messy and I ended up doing a ton of work on the main thread. This fix works and removes all the need for any pre-parsing, so it was much cleaner.

In D157459#4590470, @bulbazord wrote:

In D157459#4590028, @clayborg wrote:

In 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.

Apologies if I came across as pointed. I appreciate that you are working to make LLVM more thread-safe and I don't think any mandate that would prioritize performance over safety would stand up to scrutiny. I agree that the implication would be that reviewers should and hopefully would understand that access to member variables of DWARFContext need to be synchronized and guarded with a mutex. What I wanted to convey was what @JDevlieghere and @dblaikie are saying.

I also think that in the case of DWARFDebugLine::LineTable a mutex is not enough because you can pass out a const DWARFDebugLine::LineTable * to somebody on one thread and on another thread right afterwards you can call DWARFDebugLine::clearLineTableForUnit, meaning the first thread is left holding a dangling pointer. Future changes to DWARFContext and introduce similar issues with other member variables and it may be difficult to notice.

Agreed. It isn't perfect, but it does allow llvm-gsymutil to parse DWARF on multiple threads much more efficiently without having to do the "I must do X, Y and Z on the main thread before allowing all threads to parse the DWARF".

If we don't want to tackle thread safety with this patch I can modify llvm-gsymutil to not print out the DWARFDie information in llvm-gsymutil on the threads that parse the DWARF to avoid the crash, but I would really like to be able to if possible.

[clayborg]

I know this patch is far from adding complete thread safety to this class, but it does address some fairly simple issues that can cause things to crash. This patch protects any function that has the format of:

if (MemberVar)
  return MemberVar;
// Initialize MemberVar and then do expensive work to populate MemberVar 
return MemberVar;

The expensive work to populate can take some time and protecting this with a mutex seems like the right thing to do. Any other threads can easily use the member variable that gets returned in an incomplete state.

Are there any concrete steps I can take to help make this patch work for everyone? I have heard concerns but no real steps or tips on what it would take for people to move this patch along.

[bulbazord]

In D157459#4596356, @clayborg wrote:

I know this patch is far from adding complete thread safety to this class, but it does address some fairly simple issues that can cause things to crash. This patch protects any function that has the format of:

if (MemberVar)
  return MemberVar;
// Initialize MemberVar and then do expensive work to populate MemberVar 
return MemberVar;

The expensive work to populate can take some time and protecting this with a mutex seems like the right thing to do. Any other threads can easily use the member variable that gets returned in an incomplete state.

Are there any concrete steps I can take to help make this patch work for everyone? I have heard concerns but no real steps or tips on what it would take for people to move this patch along.

I can't speak for everyone, but here are a few concrete ways I think this change can be improved:

• This change is probably a regression performance-wise. You have a recursive mutex locking access to all member variables of a given DWARFContext on every access. One thread may try to access the already-calculated TUIndex while another is trying to do expensive work to calculate Abbrev. While this technically will give you correct results, I think we can do better in terms of performance. I'm not 100% sure which members depend on which, but it would be excellent if we could figure that out and guard every member behind its own mutex so we're not stalling on unrelated requests. The only downside I can think of is if we have mutually-dependent requests where thread A locks mutex 1 and then 2 while thread B locks mutex 2 and then 1 (deadlock). Is this possible to do?
• It looks like the "blessed" way of accessing these members is through getter methods. Future contributors may not be aware of this restriction (partially because it's not written down anywhere, partially because it's not obvious by reading the code). Even if you do document it, it's unenforceable except through code review. To address this, I think we can add an abstraction where all the member variables are owned by some other class/struct (e.g. DWARFContextImpl) where all the members are owned by that Impl object and are marked private. Access to those variables would be strictly through getter methods. DWARFContext would then contain an instance of this DWARFContextImpl class so nobody can use the member variables in an unsynchronized fashion. To be more explicit, anytime a method in DWARFContext wants to get the TUIndex, it would have to call something like Impl.getTUIndex(). It's possible people can work around this and still do bad things, but they would have to consciously think about breaking the abstraction to do that.
• Some kind of documentation related to the expectations of DWARFContext in a multithreaded environment go a long way. Even something simple as "This class is considered thread-safe" so that folks know that they can call DWARFContext::getTUIndex without worrying about this exact scenario.

What do y'all think @JDevlieghere @dblaikie @aprantl?

[clayborg]

In D157459#4596842, @bulbazord wrote:

In D157459#4596356, @clayborg wrote:

Are there any concrete steps I can take to help make this patch work for everyone? I have heard concerns but no real steps or tips on what it would take for people to move this patch along.

I can't speak for everyone, but here are a few concrete ways I think this change can be improved:

• This change is probably a regression performance-wise. You have a recursive mutex locking access to all member variables of a given DWARFContext on every access. One thread may try to access the already-calculated TUIndex while another is trying to do expensive work to calculate Abbrev. While this technically will give you correct results, I think we can do better in terms of performance. I'm not 100% sure which members depend on which, but it would be excellent if we could figure that out and guard every member behind its own mutex so we're not stalling on unrelated requests. The only downside I can think of is if we have mutually-dependent requests where thread A locks mutex 1 and then 2 while thread B locks mutex 2 and then 1 (deadlock). Is this possible to do?

I worry about thread deadlocking due to the A/B locking as you stated in these kinds of requests. We went this road early in LLDB's development but as the code evolved and people added more locks we ended up deadlocking. We used to have a lock on the lldb_private::Module and one on lldb_private::ObjectFile and one in lldb_private::SymbolFile and ran into deadlocks as people added new code. We eventually made all of these classes, that are related, use a single mutex and things were much better.

The main reason for the recursive nature is if any of the currently protected methods call another you can and will deadlock your program and I think some of the current calls do call others IIRC.

I

• It looks like the "blessed" way of accessing these members is through getter methods. Future contributors may not be aware of this restriction (partially because it's not written down anywhere, partially because it's not obvious by reading the code). Even if you do document it, it's unenforceable except through code review. To address this, I think we can add an abstraction where all the member variables are owned by some other class/struct (e.g. DWARFContextImpl) where all the members are owned by that Impl object and are marked private. Access to those variables would be strictly through getter methods. DWARFContext would then contain an instance of this DWARFContextImpl class so nobody can use the member variables in an unsynchronized fashion. To be more explicit, anytime a method in DWARFContext wants to get the TUIndex, it would have to call something like Impl.getTUIndex(). It's possible people can work around this and still do bad things, but they would have to consciously think about breaking the abstraction to do that.

That sounds very complex and would make the class even harder to work on and maintain?

• Some kind of documentation related to the expectations of DWARFContext in a multithreaded environment go a long way. Even something simple as "This class is considered thread-safe" so that folks know that they can call DWARFContext::getTUIndex without worrying about this exact scenario.

Seems like this is pretty clear from the places the mutex is used, but I could add documentation if needed.

[ayermolo]

I am somewhat confused about how this is a performance regression. Doesn't sound like it currently working in multi threaded environment anyway. If it is implemented as is, I don't believe most tools that process debug information are multi threaded, and even if they are majority of time is spent actually parsing debug information.

[bulbazord]

In D157459#4596891, @clayborg wrote:

In D157459#4596842, @bulbazord wrote:

• It looks like the "blessed" way of accessing these members is through getter methods. Future contributors may not be aware of this restriction (partially because it's not written down anywhere, partially because it's not obvious by reading the code). Even if you do document it, it's unenforceable except through code review. To address this, I think we can add an abstraction where all the member variables are owned by some other class/struct (e.g. DWARFContextImpl) where all the members are owned by that Impl object and are marked private. Access to those variables would be strictly through getter methods. DWARFContext would then contain an instance of this DWARFContextImpl class so nobody can use the member variables in an unsynchronized fashion. To be more explicit, anytime a method in DWARFContext wants to get the TUIndex, it would have to call something like Impl.getTUIndex(). It's possible people can work around this and still do bad things, but they would have to consciously think about breaking the abstraction to do that.

That sounds very complex and would make the class even harder to work on and maintain?

Here would be a very simplified version to illustrate my idea:

class ThreadSafeDWARFContextImpl {
private:
MutexType Mutex;
std::unique_ptr<DWARFUnitIndex> TUIndex;
public:
const DWARFUnitIndex &getTUIndex();
};

class DWARFContext {
private:
std::unique_ptr<ThreadSafeDWARFContextImpl> Impl;
public:
const DWARFUnitIndex &getTUIndex();
};

const DWARFUnitIndex &ThreadSafeDWARFContextImpl::getTUIndex() {
  // Perform lock of Mutex
  if (TUIndex)
    return TUIndex.get();
  // Else, do the work to compute it
  return *TUIndex;
}

const DWARFUnitIndex &DWARFContext::getTUIndex() {
  return Impl->getTUIndex();
}

It *is* more complex, I agree. The trade-off is that nobody can take touch TUIndex directly from within DWARFContext. They are required to go through Impl to actually get the TUIndex. The point is to have a safeguard other than code review, something that is difficult to write in an unsafe way. C++ doesn't give us a ton of guard rails so I'm suggesting we write our own.

• Some kind of documentation related to the expectations of DWARFContext in a multithreaded environment go a long way. Even something simple as "This class is considered thread-safe" so that folks know that they can call DWARFContext::getTUIndex without worrying about this exact scenario.

Seems like this is pretty clear from the places the mutex is used, but I could add documentation if needed.

I would argue that it's possible to figure out if you read the implementation. Reading the documentation or the header doesn't tell me what the guarantees and expected behaviors are. LLVMContext is a good example -- The documentation states that it's not safe to touch in a concurrent fashion and users must guard access to it. DWARFContext currently has nothing written down in this regard. This patch is making a decision about the use of DWARFContext in a concurrent fashion, we should write down that decision so there are clear expectations. It would be the difference between a bug report ("My code crashed when using DWARFContext in a multithreaded program. It's supposed to be thread-safe so this is a bug.") versus a user misusing an API ("My code crashed when using LLVMContext in a multithreaded program. The documentation says it's not safe to do this, I need to add locks myself").

[bulbazord]

In D157459#4596989, @ayermolo wrote:

I am somewhat confused about how this is a performance regression. Doesn't sound like it currently working in multi threaded environment anyway. If it is implemented as is, I don't believe most tools that process debug information are multi threaded, and even if they are majority of time is spent actually parsing debug information.

I have not measured this change myself, so I can't definitely claim it is 100% a performance regression. However, there are 2 reasons I suspect this:

1. recursive_mutex is usually less performant because every time you try to acquire a lock with it, you need to do extra work to determine if the mutex is already acquired on the calling thread. How much slower, I cannot say as this is probably dependent on many factors (hardware, os, stdlib implementation, etc). I won't say not to use recursive_mutex because sometimes the use is completely justified and unavoidable. We should be aware of the penalty though.
2. One mutex locking the access to every member variable means that only one member can be parsed at a time. If one thread calls DWARFContext::getDebugAbbrev while another tries to parse an unrelated section, you're leaving performance on the table. Maybe that's not a priority or not the point, but if we're going to go this route we should be aware of the tradeoffs.

[bulbazord]

I understand my suggestions may not be trivial to implement, perhaps we could benefit from starting a discussion or RFC on the LLVM discourse? Perhaps others have ideas about how to approach this while addressing our concerns.

[clayborg]

It *is* more complex, I agree. The trade-off is that nobody can take touch TUIndex directly from within DWARFContext. They are required to go through Impl to actually get the TUIndex. The point is to have a safeguard other than code review, something that is difficult to write in an unsafe way. C++ doesn't give us a ton of guard rails so I'm suggesting we write our own.

It is one possible solution yes.

• Some kind of documentation related to the expectations of DWARFContext in a multithreaded environment go a long way. Even something simple as "This class is considered thread-safe" so that folks know that they can call DWARFContext::getTUIndex without worrying about this exact scenario.

Seems like this is pretty clear from the places the mutex is used, but I could add documentation if needed.

I would argue that it's possible to figure out if you read the implementation. Reading the documentation or the header doesn't tell me what the guarantees and expected behaviors are. LLVMContext is a good example -- The documentation states that it's not safe to touch in a concurrent fashion and users must guard access to it. DWARFContext currently has nothing written down in this regard. This patch is making a decision about the use of DWARFContext in a concurrent fashion, we should write down that decision so there are clear expectations. It would be the difference between a bug report ("My code crashed when using DWARFContext in a multithreaded program. It's supposed to be thread-safe so this is a bug.") versus a user misusing an API ("My code crashed when using LLVMContext in a multithreaded program. The documentation says it's not safe to do this, I need to add locks myself").

if DWARFContext was a class that was used on its own this would make more sense to have us allow it to claim to be not usable in threaded code. The problem is its use is intertwined in many of the DWARF classes. The DWARFUnit and the DWARFDie and other classes access this from within their own code so stating that DWARFContext is not threadsafe and your need to guard access to it on your own can't and doesn't work, this is what we have now. When we have a split DWARF binary we have 1 executable and N number of .dwo files which each have their own DWARFContext, which can and will request things from the main DWARFContext if and when needed (like access to the .debug_addr section, TUIndex, CUIndex, etc) and can cause crashes.

[JDevlieghere]

Can we do something like this?

class DWARFContext {
  public:
    void foo();
    void bar();
};

template <typename T>
class ThreadSafeDWARFContext : public T {
  public:
    void foo() {
      std::lock_guard<std::mutex> L(M);
      return T::foo();
    }

  private:
    std::mutex M;
};

I think this would address everyone's concern:

• You don't pay anything when you use the non-thread safe variant. (Dave & my concern)
• Thread safety is localized in the wrapper. (Dave & Alex's concern)
• This works everywhere where a regular DWARFContext works. (Greg's concern)

It still doesn't solve the problem that you might be handing out other non thread safe abstractions (like the line table) but that's a bigger problem.

[clayborg]

In D157459#4597411, @JDevlieghere wrote:

Can we do something like this?

class DWARFContext {
  public:
    void foo();
    void bar();
};

template <typename T>
class ThreadSafeDWARFContext : public T {
  public:
    void foo() {
      std::lock_guard<std::mutex> L(M);
      return T::foo();
    }

  private:
    std::mutex M;
};

Does this mean there will be one mutex for all member variables, or one mutex per member variable? If it is one mutex per variable, then we will deadlock with A/B locking. If it is one mutex for all member variables, then this can work. I am assuming since this is templatized that it means there is one mutex per variable.

The other idea is to enable threading support by using a thread safe context but having the mutex be optional and controlled by how we initialize DWARFContextState with a boolean "EnableThreading":

class OptionalLockGuard {
  std::unique_lock<std::mutex> LockGuard;
public:
  OptionalLockGuard(std::optional<std::mutex> &OptMutex) {
    if (OptMutex) 
      LockGuard.swap(std::unique_lock<std::mutex>(*OptMutex);
  }
};

class DWARFContextState {
  std::optional<std::mutex> OptMutex; // If this has a value, then thread safety is enabled, else it isn't
  TUIndex TU;
  CUIndex CU;
  ... (add all other protected member variables here)
  public:
    DWARFContextState(DWARFContext &D, bool EnableThreading) DC(D) {
      if (EnableThreading)
        OptMutex = std::mutex(); // Enable thread safety conditionally
    }

    void getTUIndex() {
      OptionalLockGuard LockGuard(OptMutex);
      if (TU)
         return TU;
      // Parse and populate TU
      return TU;
    }

  private:
    DWARFContext &DC;
    std::mutex M;
};

This would allow us to enable and disable threading support as needed. We would need to pass in a "bool EnableThreading" to the DWARFContext on creation so it can be passed down into DWARFContextState.

I think this would address everyone's concern:

• You don't pay anything when you use the non-thread safe variant. (Dave & my concern)

Fixed by above solution with no templates needed

• Thread safety is localized in the wrapper. (Dave & Alex's concern)

Should be easy to do, but I still think we might need a recursive mutex since we can't have multiple locks unless the data that gets constructed is guaranteed to not access any other DWARFContext variables. But I can look at the API. If we can get away with multiple locks for things that don't access other DWARFContext calls, then we can use unique locks per item, if some have interdependencies, then we need a common recursive lock for those ivars.

• This works everywhere where a regular DWARFContext works. (Greg's concern)

yes

It still doesn't solve the problem that you might be handing out other non thread safe abstractions (like the line table) but that's a bigger problem.

That can be solved by storing line tables in shared pointers and handing out a shared pointer from this API so it is actually safe.

[clayborg]

Any interest in this happening, or should I abandon and change llvm-gsymutil to just avoid threading?

[JDevlieghere]

In D157459#4599803, @clayborg wrote:

Does this mean there will be one mutex for all member variables, or one mutex per member variable? If it is one mutex per variable, then we will deadlock with A/B locking. If it is one mutex for all member variables, then this can work. I am assuming since this is templatized that it means there is one mutex per variable.

The current implementation uses one mutex for all of them so I'd expect this to do the same.

This would allow us to enable and disable threading support as needed. We would need to pass in a "bool EnableThreading" to the DWARFContext on creation so it can be passed down into DWARFContextState.

• You don't pay anything when you use the non-thread safe variant. (Dave & my concern)

Fixed by above solution with no templates needed

Almost, but you'd still need to check if you need to lock the Mutex on every call, right? Definitely cheaper than locking the real mutex but not entirely free.

• Thread safety is localized in the wrapper. (Dave & Alex's concern)

Should be easy to do, but I still think we might need a recursive mutex since we can't have multiple locks unless the data that gets constructed is guaranteed to not access any other DWARFContext variables. But I can look at the API. If we can get away with multiple locks for things that don't access other DWARFContext calls, then we can use unique locks per item, if some have interdependencies, then we need a common recursive lock for those ivars.

• This works everywhere where a regular DWARFContext works. (Greg's concern)

yes

It still doesn't solve the problem that you might be handing out other non thread safe abstractions (like the line table) but that's a bigger problem.

That can be solved by storing line tables in shared pointers and handing out a shared pointer from this API so it is actually safe.

Yes, that protects the pointer, but not the abstraction it points to. My original comment was a reference to my big picture question of what threading guarantees this class provides, to which "it protects the pointers" is a totally fair answer.

In D157459#4604989, @clayborg wrote:

Any interest in this happening, or should I abandon and change llvm-gsymutil to just avoid threading?

I've been known to like templates so it won't be a surprise that I prefer that solution, so I'd like others to chime in and express their opinions.

[bulbazord]

In D157459#4604989, @clayborg wrote:

Any interest in this happening, or should I abandon and change llvm-gsymutil to just avoid threading?

I'm still interested in seeing this happen, parsing DWARF in concurrent contexts is important. I don't think the current proposed solution addresses my concerns, but I don't particularly care if you go with my suggestion or Jonas's. I'd be interested in hearing what @dblaikie has to say.

[clayborg]

Ok, I wanted to try and protect just the call to "const DWARFUnitIndex &DWARFContext::getCUIndex()" using examples people have mentioned and I came up with this solution to see if anyone likes it:

In DWARFContext.h:

class DWARFContextStateImpl {
public:
  virtual ~DWARFContextStateImpl() = default;
  virtual const DWARFUnitIndex &getCUIndex(DWARFContext &D) = 0;
};

class DWARFContextStateSingleThreaded : public DWARFContextStateImpl {
  std::unique_ptr<DWARFUnitIndex> CUIndex;
public:
  const DWARFUnitIndex &getCUIndex(DWARFContext &D) override;
};

class DWARFContextStateMultiThreaded : public DWARFContextStateSingleThreaded {
  std::recursive_mutex Mutex;
public:
  const DWARFUnitIndex &getCUIndex(DWARFContext &D) override;
};

// DWARFContext member variables
...
// std::unique_ptr<DWARFUnitIndex> CUIndex; // This was the old ivar which is now contained in a subclass of DWARFContextStateImpl
std::unique_ptr<DWARFContextStateImpl> State;
...

Then to implement this we can do the following in the DWARContext.cpp file:

const DWARFUnitIndex &DWARFContext::getCUIndex() {
  return State->getCUIndex(*this);
}

const DWARFUnitIndex &
DWARFContext::DWARFContextStateSingleThreaded::getCUIndex(DWARFContext &D) {
  if (CUIndex)
    return *CUIndex;

  DataExtractor CUIndexData(D.DObj->getCUIndexSection(), D.isLittleEndian(), 0);
  CUIndex = std::make_unique<DWARFUnitIndex>(DW_SECT_INFO);
  bool IsParseSuccessful = CUIndex->parse(CUIndexData);
  if (IsParseSuccessful)
    fixupIndex(D, *CUIndex);
  return *CUIndex;
}

const DWARFUnitIndex &
DWARFContext::DWARFContextStateMultiThreaded::getCUIndex(DWARFContext &D) {
  std::unique_lock LockGuard(Mutex);
  return DWARFContextStateSingleThreaded::getCUIndex(D);
}

Then if we enable threading with a bool in the constructor:

DWARFContext::DWARFContext(..., bool EnableMultiThreading) {
  if (EnableMultiThreading)
    State.reset(new DWARFContextStateMultiThreaded());
  else
    State.reset(new DWARFContextStateSingleThreaded());
}

If we like this idea, I can add all member variables that we want the thread protect into DWARFContextStateSingleThreaded and implement the wrappers for each accessor. Does this kind of solution work for anyone?

[JDevlieghere]

In D157459#4612553, @clayborg wrote:

Ok, I wanted to try and protect just the call to "const DWARFUnitIndex &DWARFContext::getCUIndex()" using examples people have mentioned and I came up with this solution to see if anyone likes it:

In DWARFContext.h:

class DWARFContextStateImpl {
public:
  virtual ~DWARFContextStateImpl() = default;
  virtual const DWARFUnitIndex &getCUIndex(DWARFContext &D) = 0;
};

class DWARFContextStateSingleThreaded : public DWARFContextStateImpl {
  std::unique_ptr<DWARFUnitIndex> CUIndex;
public:
  const DWARFUnitIndex &getCUIndex(DWARFContext &D) override;
};

class DWARFContextStateMultiThreaded : public DWARFContextStateSingleThreaded {
  std::recursive_mutex Mutex;
public:
  const DWARFUnitIndex &getCUIndex(DWARFContext &D) override;
};

// DWARFContext member variables
...
// std::unique_ptr<DWARFUnitIndex> CUIndex; // This was the old ivar which is now contained in a subclass of DWARFContextStateImpl
std::unique_ptr<DWARFContextStateImpl> State;
...

Then to implement this we can do the following in the DWARContext.cpp file:

const DWARFUnitIndex &DWARFContext::getCUIndex() {
  return State->getCUIndex(*this);
}

const DWARFUnitIndex &
DWARFContext::DWARFContextStateSingleThreaded::getCUIndex(DWARFContext &D) {
  if (CUIndex)
    return *CUIndex;

  DataExtractor CUIndexData(D.DObj->getCUIndexSection(), D.isLittleEndian(), 0);
  CUIndex = std::make_unique<DWARFUnitIndex>(DW_SECT_INFO);
  bool IsParseSuccessful = CUIndex->parse(CUIndexData);
  if (IsParseSuccessful)
    fixupIndex(D, *CUIndex);
  return *CUIndex;
}

const DWARFUnitIndex &
DWARFContext::DWARFContextStateMultiThreaded::getCUIndex(DWARFContext &D) {
  std::unique_lock LockGuard(Mutex);
  return DWARFContextStateSingleThreaded::getCUIndex(D);
}

Then if we enable threading with a bool in the constructor:

DWARFContext::DWARFContext(..., bool EnableMultiThreading) {
  if (EnableMultiThreading)
    State.reset(new DWARFContextStateMultiThreaded());
  else
    State.reset(new DWARFContextStateSingleThreaded());
}

If we like this idea, I can add all member variables that we want the thread protect into DWARFContextStateSingleThreaded and implement the wrappers for each accessor. Does this kind of solution work for anyone?

I like this approach 👍

[bulbazord]

In D157459#4612553, @clayborg wrote:

Ok, I wanted to try and protect just the call to "const DWARFUnitIndex &DWARFContext::getCUIndex()" using examples people have mentioned and I came up with this solution to see if anyone likes it:

In DWARFContext.h:

class DWARFContextStateImpl {
public:
  virtual ~DWARFContextStateImpl() = default;
  virtual const DWARFUnitIndex &getCUIndex(DWARFContext &D) = 0;
};

class DWARFContextStateSingleThreaded : public DWARFContextStateImpl {
  std::unique_ptr<DWARFUnitIndex> CUIndex;
public:
  const DWARFUnitIndex &getCUIndex(DWARFContext &D) override;
};

class DWARFContextStateMultiThreaded : public DWARFContextStateSingleThreaded {
  std::recursive_mutex Mutex;
public:
  const DWARFUnitIndex &getCUIndex(DWARFContext &D) override;
};

// DWARFContext member variables
...
// std::unique_ptr<DWARFUnitIndex> CUIndex; // This was the old ivar which is now contained in a subclass of DWARFContextStateImpl
std::unique_ptr<DWARFContextStateImpl> State;
...

Then to implement this we can do the following in the DWARContext.cpp file:

const DWARFUnitIndex &DWARFContext::getCUIndex() {
  return State->getCUIndex(*this);
}

const DWARFUnitIndex &
DWARFContext::DWARFContextStateSingleThreaded::getCUIndex(DWARFContext &D) {
  if (CUIndex)
    return *CUIndex;

  DataExtractor CUIndexData(D.DObj->getCUIndexSection(), D.isLittleEndian(), 0);
  CUIndex = std::make_unique<DWARFUnitIndex>(DW_SECT_INFO);
  bool IsParseSuccessful = CUIndex->parse(CUIndexData);
  if (IsParseSuccessful)
    fixupIndex(D, *CUIndex);
  return *CUIndex;
}

const DWARFUnitIndex &
DWARFContext::DWARFContextStateMultiThreaded::getCUIndex(DWARFContext &D) {
  std::unique_lock LockGuard(Mutex);
  return DWARFContextStateSingleThreaded::getCUIndex(D);
}

Then if we enable threading with a bool in the constructor:

DWARFContext::DWARFContext(..., bool EnableMultiThreading) {
  if (EnableMultiThreading)
    State.reset(new DWARFContextStateMultiThreaded());
  else
    State.reset(new DWARFContextStateSingleThreaded());
}

If we like this idea, I can add all member variables that we want the thread protect into DWARFContextStateSingleThreaded and implement the wrappers for each accessor. Does this kind of solution work for anyone?

This approach works for me. I know it is more complex than what exists today so I appreciate you taking the time to address our concerns.

[clayborg]

Thanks for the feedback, I will update this patch with the updated changes soon!

[clayborg]

Created a DWARFContextState pure virtual class that contains all of the thread sensitive member variables for the DWARFContextClass.

Multi threaded access can be enabled by passing boolean to enable it in the DWARFContext constructor and create static functions. If threading is disabled we instantiate a SingleThreadedState class that contains the ivars and no mutex for thread safety. If we enable threading, a ThreadSafeState is created that uses a mutex to protect access to ivars and calls the work functions in the SingleThreadedState.

[ayermolo]

@dblaikie @bulbazord @JDevlieghere WDYT of this approach?
@clayborg One suggestion, maybe move EnableMultiThreading optional paramenter to the end of argument list? That way constructors in other projects don't need to be modified? Right now this will break LLD and BOLT builds.

[JDevlieghere]

This looks fine to me. My only qualm is with the use of Single/MultiThreading. This could leave someone with the impression that we'll do work in parallel in the DWARFContext. Th is patch is all about thread safety. We have some prior art in llvm that uses the ThreadSafe and you used that for the thread safe context as well. I left some concrete suggestions inline.

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

135

How about bool ThreadSafe?

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

246

How about UnsafeDWARFContextState?

[clayborg]

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

135

works for me

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

246

Following your previous suggestion maybe "ThreadUnsafeDWARFContextState"?

[JDevlieghere]

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

246

Sounds good!

[clayborg]

• Rename single threaded DWARFContextState to ThreadUnsafeState
• Move default paramter for DWARFContext and DWARFContext::create functions to the end
• Rename EnableMultiThreading to ThreadSafe

[clayborg]

Add getTypeUnitMap to the ThreadSafeContext, it was missing.

[JDevlieghere]

LGTM!

[bulbazord]

My concerns were addressed, thanks for working on this!

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

 #include "llvm/DebugInfo/DWARF/DWARFUnit.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/DataExtractor.h"
 #include "llvm/Support/Error.h"
 #include "llvm/TargetParser/Host.h"
 #include <cstdint>
 #include <memory>
+#include <mutex>
 
 namespace llvm {
 
 class MemoryBuffer;
 class AppleAcceleratorTable;
 class DWARFCompileUnit;
 class DWARFDebugAbbrev;
 class DWARFDebugAranges;
 class DWARFDebugFrame;
 class DWARFDebugLoc;
 class DWARFDebugMacro;
 class DWARFDebugNames;
 class DWARFGdbIndex;
 class DWARFTypeUnit;
 class DWARFUnitIndex;
 
 /// DWARFContext
 /// This data structure is the top level entity that deals with dwarf debug
 /// information parsing. The actual data is supplied through DWARFObj.
 class DWARFContext : public DIContext {
-  DWARFUnitVector NormalUnits;
-  std::optional<DenseMap<uint64_t, DWARFTypeUnit *>> NormalTypeUnits;
-  std::unique_ptr<DWARFUnitIndex> CUIndex;
-  std::unique_ptr<DWARFGdbIndex> GdbIndex;
-  std::unique_ptr<DWARFUnitIndex> TUIndex;
-  std::unique_ptr<DWARFDebugAbbrev> Abbrev;
-  std::unique_ptr<DWARFDebugLoc> Loc;
-  std::unique_ptr<DWARFDebugAranges> Aranges;
-  std::unique_ptr<DWARFDebugLine> Line;
-  std::unique_ptr<DWARFDebugFrame> DebugFrame;
-  std::unique_ptr<DWARFDebugFrame> EHFrame;
-  std::unique_ptr<DWARFDebugMacro> Macro;
-  std::unique_ptr<DWARFDebugMacro> Macinfo;
-  std::unique_ptr<DWARFDebugNames> Names;
-  std::unique_ptr<AppleAcceleratorTable> AppleNames;
-  std::unique_ptr<AppleAcceleratorTable> AppleTypes;
-  std::unique_ptr<AppleAcceleratorTable> AppleNamespaces;
-  std::unique_ptr<AppleAcceleratorTable> AppleObjC;
-
-  DWARFUnitVector DWOUnits;
-  std::optional<DenseMap<uint64_t, DWARFTypeUnit *>> DWOTypeUnits;
-  std::unique_ptr<DWARFDebugAbbrev> AbbrevDWO;
-  std::unique_ptr<DWARFDebugMacro> MacinfoDWO;
-  std::unique_ptr<DWARFDebugMacro> MacroDWO;
+public:
+  /// DWARFContextState
+  /// This structure contains all member variables for DWARFContext that need
+  /// to be protected in multi-threaded environments. Threading support can be
+  /// enabled by setting the ThreadSafe to true when constructing a
+  /// DWARFContext to allow DWARRContext to be able to be used in a
+  /// multi-threaded environment, or not enabled to allow for maximum
+  /// performance in single threaded environments.
+  class DWARFContextState {
+  protected:
+    /// Helper enum to distinguish between macro[.dwo] and macinfo[.dwo]
+    /// section.
+    enum MacroSecType {
+      MacinfoSection,
+      MacinfoDwoSection,
+      MacroSection,
+      MacroDwoSection
+    };
+
+    DWARFContext &D;
+  public:
+    DWARFContextState(DWARFContext &DC) : D(DC) {}
+    virtual ~DWARFContextState() = default;
+    virtual DWARFUnitVector &getNormalUnits() = 0;
+    virtual DWARFUnitVector &getDWOUnits(bool Lazy = false) = 0;
+    virtual const DWARFDebugAbbrev *getDebugAbbrevDWO() = 0;
+    virtual const DWARFUnitIndex &getCUIndex() = 0;
+    virtual const DWARFUnitIndex &getTUIndex() = 0;
+    virtual DWARFGdbIndex &getGdbIndex() = 0;
+    virtual const DWARFDebugAbbrev *getDebugAbbrev() = 0;
+    virtual const DWARFDebugLoc *getDebugLoc() = 0;
+    virtual const DWARFDebugAranges *getDebugAranges() = 0;
+    virtual Expected<const DWARFDebugLine::LineTable *>
+        getLineTableForUnit(DWARFUnit *U,
+                            function_ref<void(Error)> RecoverableErrHandler) = 0;
+    virtual void clearLineTableForUnit(DWARFUnit *U) = 0;
+    virtual Expected<const DWARFDebugFrame *> getDebugFrame() = 0;
+    virtual Expected<const DWARFDebugFrame *> getEHFrame() = 0;
+    virtual const DWARFDebugMacro *getDebugMacinfo() = 0;
+    virtual const DWARFDebugMacro *getDebugMacinfoDWO() = 0;
+    virtual const DWARFDebugMacro *getDebugMacro() = 0;
+    virtual const DWARFDebugMacro *getDebugMacroDWO() = 0;
+    virtual const DWARFDebugNames &getDebugNames() = 0;
+    virtual const AppleAcceleratorTable &getAppleNames() = 0;
+    virtual const AppleAcceleratorTable &getAppleTypes() = 0;
+    virtual const AppleAcceleratorTable &getAppleNamespaces() = 0;
+    virtual const AppleAcceleratorTable &getAppleObjC() = 0;
+    virtual std::shared_ptr<DWARFContext>
+        getDWOContext(StringRef AbsolutePath) = 0;
+    virtual const DenseMap<uint64_t, DWARFTypeUnit *> &
+    getTypeUnitMap(bool IsDWO) = 0;
+
+    /// Parse a macro[.dwo] or macinfo[.dwo] section.
+    std::unique_ptr<DWARFDebugMacro>
+    parseMacroOrMacinfo(MacroSecType SectionType);
+
+  };
+  friend class DWARFContextState;
+
+private:
+  /// All important state for a DWARFContext that needs to be threadsafe needs
+  /// to go into DWARFContextState.
+  std::unique_ptr<DWARFContextState> State;
 
   /// The maximum DWARF version of all units.
   unsigned MaxVersion = 0;
 
-  struct DWOFile {
-    object::OwningBinary<object::ObjectFile> File;
-    std::unique_ptr<DWARFContext> Context;
-  };
-  StringMap<std::weak_ptr<DWOFile>> DWOFiles;
-  std::weak_ptr<DWOFile> DWP;
-  bool CheckedForDWP = false;
-  std::string DWPName;
   std::function<void(Error)> RecoverableErrorHandler =
       WithColor::defaultErrorHandler;
   std::function<void(Error)> WarningHandler = WithColor::defaultWarningHandler;
 
-  /// Read compile units from the debug_info section (if necessary)
-  /// and type units from the debug_types sections (if necessary)
-  /// and store them in NormalUnits.
-  void parseNormalUnits();
-
   /// Read compile units from the debug_info.dwo section (if necessary)
   /// and type units from the debug_types.dwo section (if necessary)
   /// and store them in DWOUnits.
   /// If \p Lazy is true, set up to parse but don't actually parse them.
   enum { EagerParse = false, LazyParse = true };
-  void parseDWOUnits(bool Lazy = false);
+  DWARFUnitVector &getDWOUnits(bool Lazy = false);
 
   std::unique_ptr<const DWARFObject> DObj;
 
-  /// Helper enum to distinguish between macro[.dwo] and macinfo[.dwo]
-  /// section.
-  enum MacroSecType {
-    MacinfoSection,
-    MacinfoDwoSection,
-    MacroSection,
-    MacroDwoSection
-  };
-
   // When set parses debug_info.dwo/debug_abbrev.dwo manually and populates CU
   // Index, and TU Index for DWARF5.
   bool ParseCUTUIndexManually = false;
 
 public:
   DWARFContext(std::unique_ptr<const DWARFObject> DObj,
                std::string DWPName = "",
                std::function<void(Error)> RecoverableErrorHandler =
                    WithColor::defaultErrorHandler,
                std::function<void(Error)> WarningHandler =
-                   WithColor::defaultWarningHandler);
+                   WithColor::defaultWarningHandler,
+               bool ThreadSafe = false);
   ~DWARFContext() override;
 
   DWARFContext(DWARFContext &) = delete;
   DWARFContext &operator=(DWARFContext &) = delete;
 
   const DWARFObject &getDWARFObj() const { return *DObj; }
 
   static bool classof(const DIContext *DICtx) {
public:
 
   bool verify(raw_ostream &OS, DIDumpOptions DumpOpts = {}) override;
 
   using unit_iterator_range = DWARFUnitVector::iterator_range;
   using compile_unit_range = DWARFUnitVector::compile_unit_range;
 
   /// Get units from .debug_info in this context.
   unit_iterator_range info_section_units() {
-    parseNormalUnits();
+    DWARFUnitVector &NormalUnits = State->getNormalUnits();
     return unit_iterator_range(NormalUnits.begin(),
                                NormalUnits.begin() +
                                    NormalUnits.getNumInfoUnits());
   }
 
   const DWARFUnitVector &getNormalUnitsVector() {
-    parseNormalUnits();
-    return NormalUnits;
+    return State->getNormalUnits();
   }
 
   /// Get units from .debug_types in this context.
   unit_iterator_range types_section_units() {
-    parseNormalUnits();
+    DWARFUnitVector &NormalUnits = State->getNormalUnits();
     return unit_iterator_range(
         NormalUnits.begin() + NormalUnits.getNumInfoUnits(), NormalUnits.end());
   }
 
   /// Get compile units in this context.
   compile_unit_range compile_units() {
     return make_filter_range(info_section_units(), isCompileUnit);
   }
 
   // If you want type_units(), it'll need to be a concat iterator of a filter of
   // TUs in info_section + all the (all type) units in types_section
 
   /// Get all normal compile/type units in this context.
   unit_iterator_range normal_units() {
-    parseNormalUnits();
+    DWARFUnitVector &NormalUnits = State->getNormalUnits();
     return unit_iterator_range(NormalUnits.begin(), NormalUnits.end());
   }
 
   /// Get units from .debug_info..dwo in the DWO context.
   unit_iterator_range dwo_info_section_units() {
-    parseDWOUnits();
+    DWARFUnitVector &DWOUnits = State->getDWOUnits();
     return unit_iterator_range(DWOUnits.begin(),
                                DWOUnits.begin() + DWOUnits.getNumInfoUnits());
   }
 
   const DWARFUnitVector &getDWOUnitsVector() {
-    parseDWOUnits();
-    return DWOUnits;
+    return State->getDWOUnits();
   }
 
   /// Get units from .debug_types.dwo in the DWO context.
   unit_iterator_range dwo_types_section_units() {
-    parseDWOUnits();
+    DWARFUnitVector &DWOUnits = State->getDWOUnits();
     return unit_iterator_range(DWOUnits.begin() + DWOUnits.getNumInfoUnits(),
                                DWOUnits.end());
   }
 
   /// Get compile units in the DWO context.
   compile_unit_range dwo_compile_units() {
     return make_filter_range(dwo_info_section_units(), isCompileUnit);
   }
 
   // If you want dwo_type_units(), it'll need to be a concat iterator of a
   // filter of TUs in dwo_info_section + all the (all type) units in
   // dwo_types_section.
 
   /// Get all units in the DWO context.
   unit_iterator_range dwo_units() {
-    parseDWOUnits();
+    DWARFUnitVector &DWOUnits = State->getDWOUnits();
     return unit_iterator_range(DWOUnits.begin(), DWOUnits.end());
   }
 
   /// Get the number of compile units in this context.
   unsigned getNumCompileUnits() {
-    parseNormalUnits();
-    return NormalUnits.getNumInfoUnits();
+    return State->getNormalUnits().getNumInfoUnits();
   }
 
   /// Get the number of type units in this context.
   unsigned getNumTypeUnits() {
-    parseNormalUnits();
-    return NormalUnits.getNumTypesUnits();
+    return State->getNormalUnits().getNumTypesUnits();
   }
 
   /// Get the number of compile units in the DWO context.
   unsigned getNumDWOCompileUnits() {
-    parseDWOUnits();
-    return DWOUnits.getNumInfoUnits();
+    return State->getDWOUnits().getNumInfoUnits();
   }
 
   /// Get the number of type units in the DWO context.
   unsigned getNumDWOTypeUnits() {
-    parseDWOUnits();
-    return DWOUnits.getNumTypesUnits();
+    return State->getDWOUnits().getNumTypesUnits();
   }
 
   /// Get the unit at the specified index.
   DWARFUnit *getUnitAtIndex(unsigned index) {
-    parseNormalUnits();
-    return NormalUnits[index].get();
+    return State->getNormalUnits()[index].get();
   }
 
   /// Get the unit at the specified index for the DWO units.
   DWARFUnit *getDWOUnitAtIndex(unsigned index) {
-    parseDWOUnits();
-    return DWOUnits[index].get();
+    return State->getDWOUnits()[index].get();
   }
 
   DWARFCompileUnit *getDWOCompileUnitForHash(uint64_t Hash);
   DWARFTypeUnit *getTypeUnitForHash(uint16_t Version, uint64_t Hash, bool IsDWO);
 
   /// Return the compile unit that includes an offset (relative to .debug_info).
   DWARFCompileUnit *getCompileUnitForOffset(uint64_t Offset);
 
public:
 
   static std::unique_ptr<DWARFContext>
   create(const object::ObjectFile &Obj,
          ProcessDebugRelocations RelocAction = ProcessDebugRelocations::Process,
          const LoadedObjectInfo *L = nullptr, std::string DWPName = "",
          std::function<void(Error)> RecoverableErrorHandler =
              WithColor::defaultErrorHandler,
          std::function<void(Error)> WarningHandler =
-             WithColor::defaultWarningHandler);
+             WithColor::defaultWarningHandler,
+         bool ThreadSafe = false);
 
   static std::unique_ptr<DWARFContext>
   create(const StringMap<std::unique_ptr<MemoryBuffer>> &Sections,
          uint8_t AddrSize, bool isLittleEndian = sys::IsLittleEndianHost,
          std::function<void(Error)> RecoverableErrorHandler =
              WithColor::defaultErrorHandler,
          std::function<void(Error)> WarningHandler =
-             WithColor::defaultWarningHandler);
+             WithColor::defaultWarningHandler,
+         bool ThreadSafe = false);
 
   /// Get address size from CUs.
   /// TODO: refactor compile_units() to make this const.
   uint8_t getCUAddrSize();
 
   Triple::ArchType getArch() const {
     return getDWARFObj().getFile()->getArch();
   }
public:
   /// manually only for DWARF5.
   bool getParseCUTUIndexManually() const { return ParseCUTUIndexManually; }
 
   /// Sets whether CU/TU should be populated manually. TU Index populated
   /// manually only for DWARF5.
   void setParseCUTUIndexManually(bool PCUTU) { ParseCUTUIndexManually = PCUTU; }
 
 private:
-  /// Parse a macro[.dwo] or macinfo[.dwo] section.
-  std::unique_ptr<DWARFDebugMacro>
-  parseMacroOrMacinfo(MacroSecType SectionType);
-
   void addLocalsForDie(DWARFCompileUnit *CU, DWARFDie Subprogram, DWARFDie Die,
                        std::vector<DILocal> &Result);
 };
 
 } // end namespace llvm
 
 #endif // LLVM_DEBUGINFO_DWARF_DWARFCONTEXT_H

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

 using namespace object;
 
 #define DEBUG_TYPE "dwarf"
 
 using DWARFLineTable = DWARFDebugLine::LineTable;
 using FileLineInfoKind = DILineInfoSpecifier::FileLineInfoKind;
 using FunctionNameKind = DILineInfoSpecifier::FunctionNameKind;
 
+
+void fixupIndexV4(DWARFContext &C, DWARFUnitIndex &Index) {
+  using EntryType = DWARFUnitIndex::Entry::SectionContribution;
+  using EntryMap = DenseMap<uint32_t, EntryType>;
+  EntryMap Map;
+  const auto &DObj = C.getDWARFObj();
+  if (DObj.getCUIndexSection().empty())
+    return;
+
+  uint64_t Offset = 0;
+  uint32_t TruncOffset = 0;
+  DObj.forEachInfoDWOSections([&](const DWARFSection &S) {
+    if (!(C.getParseCUTUIndexManually() ||
+          S.Data.size() >= std::numeric_limits<uint32_t>::max()))
+      return;
+
+    DWARFDataExtractor Data(DObj, S, C.isLittleEndian(), 0);
+    while (Data.isValidOffset(Offset)) {
+      DWARFUnitHeader Header;
+      if (!Header.extract(C, Data, &Offset, DWARFSectionKind::DW_SECT_INFO)) {
+        logAllUnhandledErrors(
+            createError("Failed to parse CU header in DWP file"), errs());
+        Map.clear();
+        break;
+      }
+
+      auto Iter = Map.insert({TruncOffset,
+                              {Header.getOffset(), Header.getNextUnitOffset() -
+                                                       Header.getOffset()}});
+      if (!Iter.second) {
+        logAllUnhandledErrors(
+            createError("Collision occured between for truncated offset 0x" +
+                        Twine::utohexstr(TruncOffset)),
+            errs());
+        Map.clear();
+        return;
+      }
+
+      Offset = Header.getNextUnitOffset();
+      TruncOffset = Offset;
+    }
+  });
+
+  if (Map.empty())
+    return;
+
+  for (DWARFUnitIndex::Entry &E : Index.getMutableRows()) {
+    if (!E.isValid())
+      continue;
+    DWARFUnitIndex::Entry::SectionContribution &CUOff = E.getContribution();
+    auto Iter = Map.find(CUOff.getOffset());
+    if (Iter == Map.end()) {
+      logAllUnhandledErrors(createError("Could not find CU offset 0x" +
+                                        Twine::utohexstr(CUOff.getOffset()) +
+                                        " in the Map"),
+                            errs());
+      break;
+    }
+    CUOff.setOffset(Iter->second.getOffset());
+    if (CUOff.getOffset() != Iter->second.getOffset())
+      logAllUnhandledErrors(createError("Length of CU in CU index doesn't "
+                                        "match calculated length at offset 0x" +
+                                        Twine::utohexstr(CUOff.getOffset())),
+                            errs());
+  }
+}
+
+void fixupIndexV5(DWARFContext &C, DWARFUnitIndex &Index) {
+  DenseMap<uint64_t, uint64_t> Map;
+
+  const auto &DObj = C.getDWARFObj();
+  DObj.forEachInfoDWOSections([&](const DWARFSection &S) {
+    if (!(C.getParseCUTUIndexManually() ||
+          S.Data.size() >= std::numeric_limits<uint32_t>::max()))
+      return;
+    DWARFDataExtractor Data(DObj, S, C.isLittleEndian(), 0);
+    uint64_t Offset = 0;
+    while (Data.isValidOffset(Offset)) {
+      DWARFUnitHeader Header;
+      if (!Header.extract(C, Data, &Offset, DWARFSectionKind::DW_SECT_INFO)) {
+        logAllUnhandledErrors(
+            createError("Failed to parse unit header in DWP file"), errs());
+        break;
+      }
+      bool CU = Header.getUnitType() == DW_UT_split_compile;
+      uint64_t Sig = CU ? *Header.getDWOId() : Header.getTypeHash();
+      Map[Sig] = Header.getOffset();
+      Offset = Header.getNextUnitOffset();
+    }
+  });
+  if (Map.empty())
+    return;
+  for (DWARFUnitIndex::Entry &E : Index.getMutableRows()) {
+    if (!E.isValid())
+      continue;
+    DWARFUnitIndex::Entry::SectionContribution &CUOff = E.getContribution();
+    auto Iter = Map.find(E.getSignature());
+    if (Iter == Map.end()) {
+      logAllUnhandledErrors(
+          createError("Could not find unit with signature 0x" +
+                      Twine::utohexstr(E.getSignature()) + " in the Map"),
+          errs());
+      break;
+    }
+    CUOff.setOffset(Iter->second);
+  }
+}
+
+void fixupIndex(DWARFContext &C, DWARFUnitIndex &Index) {
+  if (Index.getVersion() < 5)
+    fixupIndexV4(C, Index);
+  else
+    fixupIndexV5(C, Index);
+}
+
+template <typename T>
+static T &getAccelTable(std::unique_ptr<T> &Cache, const DWARFObject &Obj,
+                        const DWARFSection &Section, StringRef StringSection,
+                        bool IsLittleEndian) {
+  if (Cache)
+    return *Cache;
+  DWARFDataExtractor AccelSection(Obj, Section, IsLittleEndian, 0);
+  DataExtractor StrData(StringSection, IsLittleEndian, 0);
+  Cache.reset(new T(AccelSection, StrData));
+  if (Error E = Cache->extract())
+    llvm::consumeError(std::move(E));
+  return *Cache;
+}
+
+
+std::unique_ptr<DWARFDebugMacro>
+DWARFContext::DWARFContextState::parseMacroOrMacinfo(MacroSecType SectionType) {
+  auto Macro = std::make_unique<DWARFDebugMacro>();
+  auto ParseAndDump = [&](DWARFDataExtractor &Data, bool IsMacro) {
+    if (Error Err = IsMacro ? Macro->parseMacro(SectionType == MacroSection
+                                                    ? D.compile_units()
+                                                    : D.dwo_compile_units(),
+                                                SectionType == MacroSection
+                                                    ? D.getStringExtractor()
+                                                    : D.getStringDWOExtractor(),
+                                                Data)
+                            : Macro->parseMacinfo(Data)) {
+      D.getRecoverableErrorHandler()(std::move(Err));
+      Macro = nullptr;
+    }
+  };
+  const DWARFObject &DObj = D.getDWARFObj();
+  switch (SectionType) {
+  case MacinfoSection: {
+    DWARFDataExtractor Data(DObj.getMacinfoSection(), D.isLittleEndian(), 0);
+    ParseAndDump(Data, /*IsMacro=*/false);
+    break;
+  }
+  case MacinfoDwoSection: {
+    DWARFDataExtractor Data(DObj.getMacinfoDWOSection(), D.isLittleEndian(), 0);
+    ParseAndDump(Data, /*IsMacro=*/false);
+    break;
+  }
+  case MacroSection: {
+    DWARFDataExtractor Data(DObj, DObj.getMacroSection(), D.isLittleEndian(),
+                            0);
+    ParseAndDump(Data, /*IsMacro=*/true);
+    break;
+  }
+  case MacroDwoSection: {
+    DWARFDataExtractor Data(DObj.getMacroDWOSection(), D.isLittleEndian(), 0);
+    ParseAndDump(Data, /*IsMacro=*/true);
+    break;
+  }
+  }
+  return Macro;
+}
+
+class ThreadUnsafeDWARFContextState : public DWARFContext::DWARFContextState {
+
+  DWARFUnitVector NormalUnits;
+  std::optional<DenseMap<uint64_t, DWARFTypeUnit *>> NormalTypeUnits;
+  std::unique_ptr<DWARFUnitIndex> CUIndex;
+  std::unique_ptr<DWARFGdbIndex> GdbIndex;
+  std::unique_ptr<DWARFUnitIndex> TUIndex;
+  std::unique_ptr<DWARFDebugAbbrev> Abbrev;
+  std::unique_ptr<DWARFDebugLoc> Loc;
+  std::unique_ptr<DWARFDebugAranges> Aranges;
+  std::unique_ptr<DWARFDebugLine> Line;
+  std::unique_ptr<DWARFDebugFrame> DebugFrame;
+  std::unique_ptr<DWARFDebugFrame> EHFrame;
+  std::unique_ptr<DWARFDebugMacro> Macro;
+  std::unique_ptr<DWARFDebugMacro> Macinfo;
+  std::unique_ptr<DWARFDebugNames> Names;
+  std::unique_ptr<AppleAcceleratorTable> AppleNames;
+  std::unique_ptr<AppleAcceleratorTable> AppleTypes;
+  std::unique_ptr<AppleAcceleratorTable> AppleNamespaces;
+  std::unique_ptr<AppleAcceleratorTable> AppleObjC;
+  DWARFUnitVector DWOUnits;
+  std::optional<DenseMap<uint64_t, DWARFTypeUnit *>> DWOTypeUnits;
+  std::unique_ptr<DWARFDebugAbbrev> AbbrevDWO;
+  std::unique_ptr<DWARFDebugMacro> MacinfoDWO;
+  std::unique_ptr<DWARFDebugMacro> MacroDWO;
+  struct DWOFile {
+    object::OwningBinary<object::ObjectFile> File;
+    std::unique_ptr<DWARFContext> Context;
+  };
+  StringMap<std::weak_ptr<DWOFile>> DWOFiles;
+  std::weak_ptr<DWOFile> DWP;
+  bool CheckedForDWP = false;
+  std::string DWPName;
+
+public:
+  ThreadUnsafeDWARFContextState(DWARFContext &DC, std::string &DWP) :
+      DWARFContext::DWARFContextState(DC),
+      DWPName(std::move(DWP)) {}
+
+  DWARFUnitVector &getNormalUnits() override {
+    if (NormalUnits.empty()) {
+      const DWARFObject &DObj = D.getDWARFObj();
+      DObj.forEachInfoSections([&](const DWARFSection &S) {
+        NormalUnits.addUnitsForSection(D, S, DW_SECT_INFO);
+      });
+      NormalUnits.finishedInfoUnits();
+      DObj.forEachTypesSections([&](const DWARFSection &S) {
+        NormalUnits.addUnitsForSection(D, S, DW_SECT_EXT_TYPES);
+      });
+    }
+    return NormalUnits;
+  }
+
+  DWARFUnitVector &getDWOUnits(bool Lazy) override {
+    if (DWOUnits.empty()) {
+      const DWARFObject &DObj = D.getDWARFObj();
+
+      DObj.forEachInfoDWOSections([&](const DWARFSection &S) {
+        DWOUnits.addUnitsForDWOSection(D, S, DW_SECT_INFO, Lazy);
+      });
+      DWOUnits.finishedInfoUnits();
+      DObj.forEachTypesDWOSections([&](const DWARFSection &S) {
+        DWOUnits.addUnitsForDWOSection(D, S, DW_SECT_EXT_TYPES, Lazy);
+      });
+    }
+    return DWOUnits;
+  }
+
+  const DWARFDebugAbbrev *getDebugAbbrevDWO() override {
+    if (AbbrevDWO)
+      return AbbrevDWO.get();
+    const DWARFObject &DObj = D.getDWARFObj();
+    DataExtractor abbrData(DObj.getAbbrevDWOSection(), D.isLittleEndian(), 0);
+    AbbrevDWO = std::make_unique<DWARFDebugAbbrev>(abbrData);
+    return AbbrevDWO.get();
+  }
+
+  const DWARFUnitIndex &getCUIndex() override {
+    if (CUIndex)
+      return *CUIndex;
+
+    DataExtractor Data(D.getDWARFObj().getCUIndexSection(),
+                       D.isLittleEndian(), 0);
+    CUIndex = std::make_unique<DWARFUnitIndex>(DW_SECT_INFO);
+    if (CUIndex->parse(Data))
+      fixupIndex(D, *CUIndex);
+    return *CUIndex;
+  }
+  const DWARFUnitIndex &getTUIndex() override {
+    if (TUIndex)
+      return *TUIndex;
+
+    DataExtractor Data(D.getDWARFObj().getTUIndexSection(),
+                       D.isLittleEndian(), 0);
+    TUIndex = std::make_unique<DWARFUnitIndex>(DW_SECT_EXT_TYPES);
+    bool isParseSuccessful = TUIndex->parse(Data);
+    // If we are parsing TU-index and for .debug_types section we don't need
+    // to do anything.
+    if (isParseSuccessful && TUIndex->getVersion() != 2)
+      fixupIndex(D, *TUIndex);
+    return *TUIndex;
+  }
+
+  DWARFGdbIndex &getGdbIndex() override {
+    if (GdbIndex)
+      return *GdbIndex;
+
+    DataExtractor Data(D.getDWARFObj().getGdbIndexSection(), true /*LE*/, 0);
+    GdbIndex = std::make_unique<DWARFGdbIndex>();
+    GdbIndex->parse(Data);
+    return *GdbIndex;
+  }
+
+  const DWARFDebugAbbrev *getDebugAbbrev() override {
+    if (Abbrev)
+      return Abbrev.get();
+
+    DataExtractor Data(D.getDWARFObj().getAbbrevSection(),
+                       D.isLittleEndian(), 0);
+    Abbrev = std::make_unique<DWARFDebugAbbrev>(Data);
+    return Abbrev.get();
+  }
+
+  const DWARFDebugLoc *getDebugLoc() override {
+    if (Loc)
+      return Loc.get();
+
+    const DWARFObject &DObj = D.getDWARFObj();
+    // Assume all units have the same address byte size.
+    auto Data =
+        D.getNumCompileUnits()
+            ? DWARFDataExtractor(DObj, DObj.getLocSection(), D.isLittleEndian(),
+                                 D.getUnitAtIndex(0)->getAddressByteSize())
+            : DWARFDataExtractor("", D.isLittleEndian(), 0);
+    Loc.reset(new DWARFDebugLoc(std::move(Data)));
+    return Loc.get();
+  }
+
+  const DWARFDebugAranges *getDebugAranges() override {
+    if (Aranges)
+      return Aranges.get();
+
+    Aranges.reset(new DWARFDebugAranges());
+    Aranges->generate(&D);
+    return Aranges.get();
+  }
+
+  Expected<const DWARFDebugLine::LineTable *>
+  getLineTableForUnit(DWARFUnit *U, function_ref<void(Error)> RecoverableErrorHandler) override {
+    if (!Line)
+      Line.reset(new DWARFDebugLine);
+
+    auto UnitDIE = U->getUnitDIE();
+    if (!UnitDIE)
+      return nullptr;
+
+    auto Offset = toSectionOffset(UnitDIE.find(DW_AT_stmt_list));
+    if (!Offset)
+      return nullptr; // No line table for this compile unit.
+
+    uint64_t stmtOffset = *Offset + U->getLineTableOffset();
+    // See if the line table is cached.
+    if (const DWARFLineTable *lt = Line->getLineTable(stmtOffset))
+      return lt;
+
+    // Make sure the offset is good before we try to parse.
+    if (stmtOffset >= U->getLineSection().Data.size())
+      return nullptr;
+
+    // We have to parse it first.
+    DWARFDataExtractor Data(U->getContext().getDWARFObj(), U->getLineSection(),
+                            U->isLittleEndian(), U->getAddressByteSize());
+    return Line->getOrParseLineTable(Data, stmtOffset, U->getContext(), U,
+                                     RecoverableErrorHandler);
+
+  }
+
+  void clearLineTableForUnit(DWARFUnit *U) override {
+    if (!Line)
+      return;
+
+    auto UnitDIE = U->getUnitDIE();
+    if (!UnitDIE)
+      return;
+
+    auto Offset = toSectionOffset(UnitDIE.find(DW_AT_stmt_list));
+    if (!Offset)
+      return;
+
+    uint64_t stmtOffset = *Offset + U->getLineTableOffset();
+    Line->clearLineTable(stmtOffset);
+  }
+
+  Expected<const DWARFDebugFrame *> getDebugFrame() override {
+    if (DebugFrame)
+      return DebugFrame.get();
+    const DWARFObject &DObj = D.getDWARFObj();
+    const DWARFSection &DS = DObj.getFrameSection();
+
+    // There's a "bug" in the DWARFv3 standard with respect to the target address
+    // size within debug frame sections. While DWARF is supposed to be independent
+    // of its container, FDEs have fields with size being "target address size",
+    // which isn't specified in DWARF in general. It's only specified for CUs, but
+    // .eh_frame can appear without a .debug_info section. Follow the example of
+    // other tools (libdwarf) and extract this from the container (ObjectFile
+    // provides this information). This problem is fixed in DWARFv4
+    // See this dwarf-discuss discussion for more details:
+    // http://lists.dwarfstd.org/htdig.cgi/dwarf-discuss-dwarfstd.org/2011-December/001173.html
+    DWARFDataExtractor Data(DObj, DS, D.isLittleEndian(),
+                            DObj.getAddressSize());
+    auto DF =
+        std::make_unique<DWARFDebugFrame>(D.getArch(), /*IsEH=*/false,
+                                          DS.Address);
+    if (Error E = DF->parse(Data))
+      return std::move(E);
+
+    DebugFrame.swap(DF);
+    return DebugFrame.get();
+  }
+
+  Expected<const DWARFDebugFrame *> getEHFrame() override {
+    if (EHFrame)
+      return EHFrame.get();
+    const DWARFObject &DObj = D.getDWARFObj();
+
+    const DWARFSection &DS = DObj.getEHFrameSection();
+    DWARFDataExtractor Data(DObj, DS, D.isLittleEndian(),
+                            DObj.getAddressSize());
+    auto DF =
+        std::make_unique<DWARFDebugFrame>(D.getArch(), /*IsEH=*/true,
+                                          DS.Address);
+    if (Error E = DF->parse(Data))
+      return std::move(E);
+    EHFrame.swap(DF);
+    return EHFrame.get();
+  }
+
+  const DWARFDebugMacro *getDebugMacinfo() override {
+    if (!Macinfo)
+      Macinfo = parseMacroOrMacinfo(MacinfoSection);
+    return Macinfo.get();
+  }
+  const DWARFDebugMacro *getDebugMacinfoDWO() override {
+    if (!MacinfoDWO)
+      MacinfoDWO = parseMacroOrMacinfo(MacinfoDwoSection);
+    return MacinfoDWO.get();
+  }
+  const DWARFDebugMacro *getDebugMacro() override {
+    if (!Macro)
+      Macro = parseMacroOrMacinfo(MacroSection);
+    return Macro.get();
+  }
+  const DWARFDebugMacro *getDebugMacroDWO() override {
+    if (!MacroDWO)
+      MacroDWO = parseMacroOrMacinfo(MacroDwoSection);
+    return MacroDWO.get();
+  }
+  const DWARFDebugNames &getDebugNames() override {
+    const DWARFObject &DObj = D.getDWARFObj();
+    return getAccelTable(Names, DObj, DObj.getNamesSection(),
+                         DObj.getStrSection(), D.isLittleEndian());
+  }
+  const AppleAcceleratorTable &getAppleNames() override {
+    const DWARFObject &DObj = D.getDWARFObj();
+    return getAccelTable(AppleNames, DObj, DObj.getAppleNamesSection(),
+                         DObj.getStrSection(), D.isLittleEndian());
+
+  }
+  const AppleAcceleratorTable &getAppleTypes() override {
+    const DWARFObject &DObj = D.getDWARFObj();
+    return getAccelTable(AppleTypes, DObj, DObj.getAppleTypesSection(),
+                         DObj.getStrSection(), D.isLittleEndian());
+
+  }
+  const AppleAcceleratorTable &getAppleNamespaces() override {
+    const DWARFObject &DObj = D.getDWARFObj();
+    return getAccelTable(AppleNamespaces, DObj,
+                         DObj.getAppleNamespacesSection(),
+                         DObj.getStrSection(), D.isLittleEndian());
+
+  }
+  const AppleAcceleratorTable &getAppleObjC() override {
+    const DWARFObject &DObj = D.getDWARFObj();
+    return getAccelTable(AppleObjC, DObj, DObj.getAppleObjCSection(),
+                         DObj.getStrSection(), D.isLittleEndian());
+  }
+
+  std::shared_ptr<DWARFContext>
+  getDWOContext(StringRef AbsolutePath) override {
+    if (auto S = DWP.lock()) {
+      DWARFContext *Ctxt = S->Context.get();
+      return std::shared_ptr<DWARFContext>(std::move(S), Ctxt);
+    }
+
+    std::weak_ptr<DWOFile> *Entry = &DWOFiles[AbsolutePath];
+
+    if (auto S = Entry->lock()) {
+      DWARFContext *Ctxt = S->Context.get();
+      return std::shared_ptr<DWARFContext>(std::move(S), Ctxt);
+    }
+
+    const DWARFObject &DObj = D.getDWARFObj();
+
+    Expected<OwningBinary<ObjectFile>> Obj = [&] {
+      if (!CheckedForDWP) {
+        SmallString<128> DWPName;
+        auto Obj = object::ObjectFile::createObjectFile(
+            this->DWPName.empty()
+                ? (DObj.getFileName() + ".dwp").toStringRef(DWPName)
+                : StringRef(this->DWPName));
+        if (Obj) {
+          Entry = &DWP;
+          return Obj;
+        } else {
+          CheckedForDWP = true;
+          // TODO: Should this error be handled (maybe in a high verbosity mode)
+          // before falling back to .dwo files?
+          consumeError(Obj.takeError());
+        }
+      }
+
+      return object::ObjectFile::createObjectFile(AbsolutePath);
+    }();
+
+    if (!Obj) {
+      // TODO: Actually report errors helpfully.
+      consumeError(Obj.takeError());
+      return nullptr;
+    }
+
+    auto S = std::make_shared<DWOFile>();
+    S->File = std::move(Obj.get());
+    S->Context = DWARFContext::create(*S->File.getBinary(),
+                                      DWARFContext::ProcessDebugRelocations::Ignore);
+    *Entry = S;
+    auto *Ctxt = S->Context.get();
+    return std::shared_ptr<DWARFContext>(std::move(S), Ctxt);
+  }
+
+  const DenseMap<uint64_t, DWARFTypeUnit *> &getNormalTypeUnitMap() {
+    if (!NormalTypeUnits) {
+      NormalTypeUnits.emplace();
+      for (const auto &U :D.normal_units()) {
+        if (DWARFTypeUnit *TU = dyn_cast<DWARFTypeUnit>(U.get()))
+          (*NormalTypeUnits)[TU->getTypeHash()] = TU;
+      }
+    }
+    return *NormalTypeUnits;
+  }
+
+  const DenseMap<uint64_t, DWARFTypeUnit *> &getDWOTypeUnitMap() {
+    if (!DWOTypeUnits) {
+      DWOTypeUnits.emplace();
+      for (const auto &U :D.dwo_units()) {
+        if (DWARFTypeUnit *TU = dyn_cast<DWARFTypeUnit>(U.get()))
+          (*DWOTypeUnits)[TU->getTypeHash()] = TU;
+      }
+    }
+    return *DWOTypeUnits;
+  }
+
+  const DenseMap<uint64_t, DWARFTypeUnit *> &
+  getTypeUnitMap(bool IsDWO) override {
+    if (IsDWO)
+      return getDWOTypeUnitMap();
+    else
+      return getNormalTypeUnitMap();
+  }
+
+
+};
+
+class ThreadSafeState : public ThreadUnsafeDWARFContextState {
+  std::recursive_mutex Mutex;
+
+public:
+  ThreadSafeState(DWARFContext &DC, std::string &DWP) :
+      ThreadUnsafeDWARFContextState(DC, DWP) {}
+
+  DWARFUnitVector &getNormalUnits() override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getNormalUnits();
+  }
+  DWARFUnitVector &getDWOUnits(bool Lazy) override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getDWOUnits(Lazy);
+  }
+  const DWARFUnitIndex &getCUIndex() override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getCUIndex();
+  }
+  const DWARFDebugAbbrev *getDebugAbbrevDWO() override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getDebugAbbrevDWO();
+  }
+
+  const DWARFUnitIndex &getTUIndex() override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getTUIndex();
+  }
+  DWARFGdbIndex &getGdbIndex() override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getGdbIndex();
+  }
+  const DWARFDebugAbbrev *getDebugAbbrev() override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getDebugAbbrev();
+  }
+  const DWARFDebugLoc *getDebugLoc() override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getDebugLoc();
+  }
+  const DWARFDebugAranges *getDebugAranges() override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getDebugAranges();
+  }
+  Expected<const DWARFDebugLine::LineTable *>
+  getLineTableForUnit(DWARFUnit *U, function_ref<void(Error)> RecoverableErrorHandler) override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getLineTableForUnit(U, RecoverableErrorHandler);
+  }
+  void clearLineTableForUnit(DWARFUnit *U) override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::clearLineTableForUnit(U);
+  }
+  Expected<const DWARFDebugFrame *> getDebugFrame() override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getDebugFrame();
+  }
+  Expected<const DWARFDebugFrame *> getEHFrame() override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getEHFrame();
+  }
+  const DWARFDebugMacro *getDebugMacinfo() override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getDebugMacinfo();
+  }
+  const DWARFDebugMacro *getDebugMacinfoDWO() override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getDebugMacinfoDWO();
+  }
+  const DWARFDebugMacro *getDebugMacro() override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getDebugMacro();
+  }
+  const DWARFDebugMacro *getDebugMacroDWO() override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getDebugMacroDWO();
+  }
+  const DWARFDebugNames &getDebugNames() override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getDebugNames();
+  }
+  const AppleAcceleratorTable &getAppleNames() override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getAppleNames();
+  }
+  const AppleAcceleratorTable &getAppleTypes() override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getAppleTypes();
+  }
+  const AppleAcceleratorTable &getAppleNamespaces() override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getAppleNamespaces();
+  }
+  const AppleAcceleratorTable &getAppleObjC() override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getAppleObjC();
+  }
+  std::shared_ptr<DWARFContext>
+  getDWOContext(StringRef AbsolutePath) override {
+    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
+    return ThreadUnsafeDWARFContextState::getDWOContext(AbsolutePath);
+  }
+};
+
+
+
 DWARFContext::DWARFContext(std::unique_ptr<const DWARFObject> DObj,
                            std::string DWPName,
                            std::function<void(Error)> RecoverableErrorHandler,
-                           std::function<void(Error)> WarningHandler)
-    : DIContext(CK_DWARF), DWPName(std::move(DWPName)),
+                           std::function<void(Error)> WarningHandler,
+                           bool ThreadSafe)
+    : DIContext(CK_DWARF),
       RecoverableErrorHandler(RecoverableErrorHandler),
-      WarningHandler(WarningHandler), DObj(std::move(DObj)) {}
+      WarningHandler(WarningHandler), DObj(std::move(DObj)) {
+        if (ThreadSafe)
+          State.reset(new ThreadUnsafeDWARFContextState(*this, DWPName));
+        else
+          State.reset(new ThreadSafeState(*this, DWPName));
+      }
 
 DWARFContext::~DWARFContext() = default;
 
 /// Dump the UUID load command.
 static void dumpUUID(raw_ostream &OS, const ObjectFile &Obj) {
   auto *MachO = dyn_cast<MachOObjectFile>(&Obj);
   if (!MachO)
     return;
if (Error Err = Rnglists.extract(rnglistData, &Offset)) {
         break;
       Offset = TableOffset + Length;
     } else {
       Rnglists.dump(rnglistData, OS, LookupPooledAddress, DumpOpts);
     }
   }
 }
 
-std::unique_ptr<DWARFDebugMacro>
-DWARFContext::parseMacroOrMacinfo(MacroSecType SectionType) {
-  auto Macro = std::make_unique<DWARFDebugMacro>();
-  auto ParseAndDump = [&](DWARFDataExtractor &Data, bool IsMacro) {
-    if (Error Err = IsMacro ? Macro->parseMacro(SectionType == MacroSection
-                                                    ? compile_units()
-                                                    : dwo_compile_units(),
-                                                SectionType == MacroSection
-                                                    ? getStringExtractor()
-                                                    : getStringDWOExtractor(),
-                                                Data)
-                            : Macro->parseMacinfo(Data)) {
-      RecoverableErrorHandler(std::move(Err));
-      Macro = nullptr;
-    }
-  };
-  switch (SectionType) {
-  case MacinfoSection: {
-    DWARFDataExtractor Data(DObj->getMacinfoSection(), isLittleEndian(), 0);
-    ParseAndDump(Data, /*IsMacro=*/false);
-    break;
-  }
-  case MacinfoDwoSection: {
-    DWARFDataExtractor Data(DObj->getMacinfoDWOSection(), isLittleEndian(), 0);
-    ParseAndDump(Data, /*IsMacro=*/false);
-    break;
-  }
-  case MacroSection: {
-    DWARFDataExtractor Data(*DObj, DObj->getMacroSection(), isLittleEndian(),
-                            0);
-    ParseAndDump(Data, /*IsMacro=*/true);
-    break;
-  }
-  case MacroDwoSection: {
-    DWARFDataExtractor Data(DObj->getMacroDWOSection(), isLittleEndian(), 0);
-    ParseAndDump(Data, /*IsMacro=*/true);
-    break;
-  }
-  }
-  return Macro;
-}
 
 static void dumpLoclistsSection(raw_ostream &OS, DIDumpOptions DumpOpts,
                                 DWARFDataExtractor Data, const DWARFObject &Obj,
                                 std::optional<uint64_t> DumpOffset) {
   uint64_t Offset = 0;
 
   while (Data.isValidOffset(Offset)) {
     DWARFListTableHeader Header(".debug_loclists", "locations");
if (shouldDump(Explicit, ".apple_objc", DIDT_ID_AppleObjC,
     getAppleObjC().dump(OS);
   if (shouldDump(Explicit, ".debug_names", DIDT_ID_DebugNames,
                  DObj->getNamesSection().Data))
     getDebugNames().dump(OS);
 }
 
 DWARFTypeUnit *DWARFContext::getTypeUnitForHash(uint16_t Version, uint64_t Hash,
                                                 bool IsDWO) {
-  parseDWOUnits(LazyParse);
-
+  DWARFUnitVector &DWOUnits = State->getDWOUnits();
   if (const auto &TUI = getTUIndex()) {
     if (const auto *R = TUI.getFromHash(Hash))
       return dyn_cast_or_null<DWARFTypeUnit>(
           DWOUnits.getUnitForIndexEntry(*R));
     return nullptr;
   }
-
-  struct UnitContainers {
-    const DWARFUnitVector &Units;
-    std::optional<DenseMap<uint64_t, DWARFTypeUnit *>> &Map;
-  };
-  UnitContainers Units = IsDWO ? UnitContainers{DWOUnits, DWOTypeUnits}
-                               : UnitContainers{NormalUnits, NormalTypeUnits};
-  if (!Units.Map) {
-    Units.Map.emplace();
-    for (const auto &U : IsDWO ? dwo_units() : normal_units()) {
-      if (DWARFTypeUnit *TU = dyn_cast<DWARFTypeUnit>(U.get()))
-        (*Units.Map)[TU->getTypeHash()] = TU;
-    }
-  }
-
-  return (*Units.Map)[Hash];
+  const DenseMap<uint64_t, DWARFTypeUnit *> &Map = State->getTypeUnitMap(IsDWO);
+  auto Iter = Map.find(Hash);
+  if (Iter != Map.end())
+    return Iter->second;
+  return nullptr;
 }
 
 DWARFCompileUnit *DWARFContext::getDWOCompileUnitForHash(uint64_t Hash) {
-  parseDWOUnits(LazyParse);
+  DWARFUnitVector &DWOUnits = State->getDWOUnits(LazyParse);
 
   if (const auto &CUI = getCUIndex()) {
     if (const auto *R = CUI.getFromHash(Hash))
       return dyn_cast_or_null<DWARFCompileUnit>(
           DWOUnits.getUnitForIndexEntry(*R));
     return nullptr;
   }
 
for (const auto &DWOCU : dwo_compile_units()) {
     }
     if (DWOCU->getDWOId() == Hash)
       return dyn_cast<DWARFCompileUnit>(DWOCU.get());
   }
   return nullptr;
 }
 
 DWARFDie DWARFContext::getDIEForOffset(uint64_t Offset) {
-  parseNormalUnits();
-  if (auto *CU = NormalUnits.getUnitForOffset(Offset))
+  if (auto *CU = State->getNormalUnits().getUnitForOffset(Offset))
     return CU->getDIEForOffset(Offset);
   return DWARFDie();
 }
 
 bool DWARFContext::verify(raw_ostream &OS, DIDumpOptions DumpOpts) {
   bool Success = true;
   DWARFVerifier verifier(OS, *this, DumpOpts);
 
   Success &= verifier.handleDebugAbbrev();
   if (DumpOpts.DumpType & DIDT_DebugCUIndex)
     Success &= verifier.handleDebugCUIndex();
   if (DumpOpts.DumpType & DIDT_DebugTUIndex)
     Success &= verifier.handleDebugTUIndex();
   if (DumpOpts.DumpType & DIDT_DebugInfo)
     Success &= verifier.handleDebugInfo();
   if (DumpOpts.DumpType & DIDT_DebugLine)
     Success &= verifier.handleDebugLine();
   if (DumpOpts.DumpType & DIDT_DebugStrOffsets)
     Success &= verifier.handleDebugStrOffsets();
   Success &= verifier.handleAccelTables();
   return Success;
 }
 
-void fixupIndexV4(const DWARFObject &DObj, DWARFContext &C,
-                DWARFUnitIndex &Index) {
-  using EntryType = DWARFUnitIndex::Entry::SectionContribution;
-  using EntryMap = DenseMap<uint32_t, EntryType>;
-  EntryMap Map;
-  if (DObj.getCUIndexSection().empty())
-    return;
-
-  uint64_t Offset = 0;
-  uint32_t TruncOffset = 0;
-  DObj.forEachInfoDWOSections([&](const DWARFSection &S) {
-    if (!(C.getParseCUTUIndexManually() ||
-          S.Data.size() >= std::numeric_limits<uint32_t>::max()))
-      return;
-
-    DWARFDataExtractor Data(DObj, S, C.isLittleEndian(), 0);
-    while (Data.isValidOffset(Offset)) {
-      DWARFUnitHeader Header;
-      if (!Header.extract(C, Data, &Offset, DWARFSectionKind::DW_SECT_INFO)) {
-        logAllUnhandledErrors(
-            createError("Failed to parse CU header in DWP file"), errs());
-        Map.clear();
-        break;
-      }
-
-      auto Iter = Map.insert({TruncOffset,
-                              {Header.getOffset(), Header.getNextUnitOffset() -
-                                                       Header.getOffset()}});
-      if (!Iter.second) {
-        logAllUnhandledErrors(
-            createError("Collision occured between for truncated offset 0x" +
-                        Twine::utohexstr(TruncOffset)),
-            errs());
-        Map.clear();
-        return;
-      }
-
-      Offset = Header.getNextUnitOffset();
-      TruncOffset = Offset;
-    }
-  });
-
-  if (Map.empty())
-    return;
-
-  for (DWARFUnitIndex::Entry &E : Index.getMutableRows()) {
-    if (!E.isValid())
-      continue;
-    DWARFUnitIndex::Entry::SectionContribution &CUOff = E.getContribution();
-    auto Iter = Map.find(CUOff.getOffset());
-    if (Iter == Map.end()) {
-      logAllUnhandledErrors(createError("Could not find CU offset 0x" +
-                                        Twine::utohexstr(CUOff.getOffset()) +
-                                        " in the Map"),
-                            errs());
-      break;
-    }
-    CUOff.setOffset(Iter->second.getOffset());
-    if (CUOff.getOffset() != Iter->second.getOffset())
-      logAllUnhandledErrors(createError("Length of CU in CU index doesn't "
-                                        "match calculated length at offset 0x" +
-                                        Twine::utohexstr(CUOff.getOffset())),
-                            errs());
-  }
-}
-
-void fixupIndexV5(const DWARFObject &DObj, DWARFContext &C,
-                  DWARFUnitIndex &Index) {
-  DenseMap<uint64_t, uint64_t> Map;
-
-  DObj.forEachInfoDWOSections([&](const DWARFSection &S) {
-    if (!(C.getParseCUTUIndexManually() ||
-          S.Data.size() >= std::numeric_limits<uint32_t>::max()))
-      return;
-    DWARFDataExtractor Data(DObj, S, C.isLittleEndian(), 0);
-    uint64_t Offset = 0;
-    while (Data.isValidOffset(Offset)) {
-      DWARFUnitHeader Header;
-      if (!Header.extract(C, Data, &Offset, DWARFSectionKind::DW_SECT_INFO)) {
-        logAllUnhandledErrors(
-            createError("Failed to parse unit header in DWP file"), errs());
-        break;
-      }
-      bool CU = Header.getUnitType() == DW_UT_split_compile;
-      uint64_t Sig = CU ? *Header.getDWOId() : Header.getTypeHash();
-      Map[Sig] = Header.getOffset();
-      Offset = Header.getNextUnitOffset();
-    }
-  });
-  if (Map.empty())
-    return;
-  for (DWARFUnitIndex::Entry &E : Index.getMutableRows()) {
-    if (!E.isValid())
-      continue;
-    DWARFUnitIndex::Entry::SectionContribution &CUOff = E.getContribution();
-    auto Iter = Map.find(E.getSignature());
-    if (Iter == Map.end()) {
-      logAllUnhandledErrors(
-          createError("Could not find unit with signature 0x" +
-                      Twine::utohexstr(E.getSignature()) + " in the Map"),
-          errs());
-      break;
-    }
-    CUOff.setOffset(Iter->second);
-  }
-}
-
-void fixupIndex(const DWARFObject &DObj, DWARFContext &C,
-                DWARFUnitIndex &Index) {
-  if (Index.getVersion() < 5)
-    fixupIndexV4(DObj, C, Index);
-  else
-    fixupIndexV5(DObj, C, Index);
-}
-
 const DWARFUnitIndex &DWARFContext::getCUIndex() {
-  if (CUIndex)
-    return *CUIndex;
-
-  DataExtractor CUIndexData(DObj->getCUIndexSection(), isLittleEndian(), 0);
-  CUIndex = std::make_unique<DWARFUnitIndex>(DW_SECT_INFO);
-  bool IsParseSuccessful = CUIndex->parse(CUIndexData);
-  if (IsParseSuccessful)
-    fixupIndex(*DObj, *this, *CUIndex);
-  return *CUIndex;
+  return State->getCUIndex();
 }
 
 const DWARFUnitIndex &DWARFContext::getTUIndex() {
-  if (TUIndex)
-    return *TUIndex;
-
-  DataExtractor TUIndexData(DObj->getTUIndexSection(), isLittleEndian(), 0);
-  TUIndex = std::make_unique<DWARFUnitIndex>(DW_SECT_EXT_TYPES);
-  bool isParseSuccessful = TUIndex->parse(TUIndexData);
-  // If we are parsing TU-index and for .debug_types section we don't need
-  // to do anything.
-  if (isParseSuccessful && TUIndex->getVersion() != 2)
-    fixupIndex(*DObj, *this, *TUIndex);
-  return *TUIndex;
+  return State->getTUIndex();
 }
 
 DWARFGdbIndex &DWARFContext::getGdbIndex() {
-  if (GdbIndex)
-    return *GdbIndex;
-
-  DataExtractor GdbIndexData(DObj->getGdbIndexSection(), true /*LE*/, 0);
-  GdbIndex = std::make_unique<DWARFGdbIndex>();
-  GdbIndex->parse(GdbIndexData);
-  return *GdbIndex;
+  return State->getGdbIndex();
 }
 
 const DWARFDebugAbbrev *DWARFContext::getDebugAbbrev() {
-  if (Abbrev)
-    return Abbrev.get();
-
-  DataExtractor abbrData(DObj->getAbbrevSection(), isLittleEndian(), 0);
-  Abbrev = std::make_unique<DWARFDebugAbbrev>(abbrData);
-  return Abbrev.get();
+  return State->getDebugAbbrev();
 }
 
 const DWARFDebugAbbrev *DWARFContext::getDebugAbbrevDWO() {
-  if (AbbrevDWO)
-    return AbbrevDWO.get();
-
-  DataExtractor abbrData(DObj->getAbbrevDWOSection(), isLittleEndian(), 0);
-  AbbrevDWO = std::make_unique<DWARFDebugAbbrev>(abbrData);
-  return AbbrevDWO.get();
+  return State->getDebugAbbrevDWO();
 }
 
 const DWARFDebugLoc *DWARFContext::getDebugLoc() {
-  if (Loc)
-    return Loc.get();
-
-  // Assume all units have the same address byte size.
-  auto LocData =
-      getNumCompileUnits()
-          ? DWARFDataExtractor(*DObj, DObj->getLocSection(), isLittleEndian(),
-                               getUnitAtIndex(0)->getAddressByteSize())
-          : DWARFDataExtractor("", isLittleEndian(), 0);
-  Loc.reset(new DWARFDebugLoc(std::move(LocData)));
-  return Loc.get();
+  return State->getDebugLoc();
 }
 
 const DWARFDebugAranges *DWARFContext::getDebugAranges() {
-  if (Aranges)
-    return Aranges.get();
-
-  Aranges.reset(new DWARFDebugAranges());
-  Aranges->generate(this);
-  return Aranges.get();
+  return State->getDebugAranges();
 }
 
 Expected<const DWARFDebugFrame *> DWARFContext::getDebugFrame() {
-  if (DebugFrame)
-    return DebugFrame.get();
-
-  const DWARFSection &DS = DObj->getFrameSection();
-
-  // There's a "bug" in the DWARFv3 standard with respect to the target address
-  // size within debug frame sections. While DWARF is supposed to be independent
-  // of its container, FDEs have fields with size being "target address size",
-  // which isn't specified in DWARF in general. It's only specified for CUs, but
-  // .eh_frame can appear without a .debug_info section. Follow the example of
-  // other tools (libdwarf) and extract this from the container (ObjectFile
-  // provides this information). This problem is fixed in DWARFv4
-  // See this dwarf-discuss discussion for more details:
-  // http://lists.dwarfstd.org/htdig.cgi/dwarf-discuss-dwarfstd.org/2011-December/001173.html
-  DWARFDataExtractor DebugFrameData(*DObj, DS, isLittleEndian(),
-                                    DObj->getAddressSize());
-  auto DF =
-      std::make_unique<DWARFDebugFrame>(getArch(), /*IsEH=*/false, DS.Address);
-  if (Error E = DF->parse(DebugFrameData))
-    return std::move(E);
-
-  DebugFrame.swap(DF);
-  return DebugFrame.get();
+  return State->getDebugFrame();
 }
 
 Expected<const DWARFDebugFrame *> DWARFContext::getEHFrame() {
-  if (EHFrame)
-    return EHFrame.get();
-
-  const DWARFSection &DS = DObj->getEHFrameSection();
-  DWARFDataExtractor DebugFrameData(*DObj, DS, isLittleEndian(),
-                                    DObj->getAddressSize());
-
-  auto DF =
-      std::make_unique<DWARFDebugFrame>(getArch(), /*IsEH=*/true, DS.Address);
-  if (Error E = DF->parse(DebugFrameData))
-    return std::move(E);
-  DebugFrame.swap(DF);
-  return DebugFrame.get();
+  return State->getEHFrame();
 }
 
 const DWARFDebugMacro *DWARFContext::getDebugMacro() {
-  if (!Macro)
-    Macro = parseMacroOrMacinfo(MacroSection);
-  return Macro.get();
+  return State->getDebugMacro();
 }
 
 const DWARFDebugMacro *DWARFContext::getDebugMacroDWO() {
-  if (!MacroDWO)
-    MacroDWO = parseMacroOrMacinfo(MacroDwoSection);
-  return MacroDWO.get();
+  return State->getDebugMacroDWO();
 }
 
 const DWARFDebugMacro *DWARFContext::getDebugMacinfo() {
-  if (!Macinfo)
-    Macinfo = parseMacroOrMacinfo(MacinfoSection);
-  return Macinfo.get();
+  return State->getDebugMacinfo();
 }
 
 const DWARFDebugMacro *DWARFContext::getDebugMacinfoDWO() {
-  if (!MacinfoDWO)
-    MacinfoDWO = parseMacroOrMacinfo(MacinfoDwoSection);
-  return MacinfoDWO.get();
+  return State->getDebugMacinfoDWO();
 }
 
-template <typename T>
-static T &getAccelTable(std::unique_ptr<T> &Cache, const DWARFObject &Obj,
-                        const DWARFSection &Section, StringRef StringSection,
-                        bool IsLittleEndian) {
-  if (Cache)
-    return *Cache;
-  DWARFDataExtractor AccelSection(Obj, Section, IsLittleEndian, 0);
-  DataExtractor StrData(StringSection, IsLittleEndian, 0);
-  Cache.reset(new T(AccelSection, StrData));
-  if (Error E = Cache->extract())
-    llvm::consumeError(std::move(E));
-  return *Cache;
-}
 
 const DWARFDebugNames &DWARFContext::getDebugNames() {
-  return getAccelTable(Names, *DObj, DObj->getNamesSection(),
-                       DObj->getStrSection(), isLittleEndian());
+  return State->getDebugNames();
 }
 
 const AppleAcceleratorTable &DWARFContext::getAppleNames() {
-  return getAccelTable(AppleNames, *DObj, DObj->getAppleNamesSection(),
-                       DObj->getStrSection(), isLittleEndian());
+  return State->getAppleNames();
 }
 
 const AppleAcceleratorTable &DWARFContext::getAppleTypes() {
-  return getAccelTable(AppleTypes, *DObj, DObj->getAppleTypesSection(),
-                       DObj->getStrSection(), isLittleEndian());
+  return State->getAppleTypes();
 }
 
 const AppleAcceleratorTable &DWARFContext::getAppleNamespaces() {
-  return getAccelTable(AppleNamespaces, *DObj,
-                       DObj->getAppleNamespacesSection(),
-                       DObj->getStrSection(), isLittleEndian());
+  return State->getAppleNamespaces();
 }
 
 const AppleAcceleratorTable &DWARFContext::getAppleObjC() {
-  return getAccelTable(AppleObjC, *DObj, DObj->getAppleObjCSection(),
-                       DObj->getStrSection(), isLittleEndian());
+  return State->getAppleObjC();
 }
 
 const DWARFDebugLine::LineTable *
 DWARFContext::getLineTableForUnit(DWARFUnit *U) {
   Expected<const DWARFDebugLine::LineTable *> ExpectedLineTable =
       getLineTableForUnit(U, WarningHandler);
   if (!ExpectedLineTable) {
     WarningHandler(ExpectedLineTable.takeError());
     return nullptr;
   }
   return *ExpectedLineTable;
 }
 
 Expected<const DWARFDebugLine::LineTable *> DWARFContext::getLineTableForUnit(
     DWARFUnit *U, function_ref<void(Error)> RecoverableErrorHandler) {
-  if (!Line)
-    Line.reset(new DWARFDebugLine);
-
-  auto UnitDIE = U->getUnitDIE();
-  if (!UnitDIE)
-    return nullptr;
-
-  auto Offset = toSectionOffset(UnitDIE.find(DW_AT_stmt_list));
-  if (!Offset)
-    return nullptr; // No line table for this compile unit.
-
-  uint64_t stmtOffset = *Offset + U->getLineTableOffset();
-  // See if the line table is cached.
-  if (const DWARFLineTable *lt = Line->getLineTable(stmtOffset))
-    return lt;
-
-  // Make sure the offset is good before we try to parse.
-  if (stmtOffset >= U->getLineSection().Data.size())
-    return nullptr;
-
-  // We have to parse it first.
-  DWARFDataExtractor lineData(*DObj, U->getLineSection(), isLittleEndian(),
-                              U->getAddressByteSize());
-  return Line->getOrParseLineTable(lineData, stmtOffset, *this, U,
-                                   RecoverableErrorHandler);
+  return State->getLineTableForUnit(U, RecoverableErrorHandler);
 }
 
 void DWARFContext::clearLineTableForUnit(DWARFUnit *U) {
-  if (!Line)
-    return;
-
-  auto UnitDIE = U->getUnitDIE();
-  if (!UnitDIE)
-    return;
-
-  auto Offset = toSectionOffset(UnitDIE.find(DW_AT_stmt_list));
-  if (!Offset)
-    return;
-
-  uint64_t stmtOffset = *Offset + U->getLineTableOffset();
-  Line->clearLineTable(stmtOffset);
+  return State->clearLineTableForUnit(U);
 }
 
-void DWARFContext::parseNormalUnits() {
-  if (!NormalUnits.empty())
-    return;
-  DObj->forEachInfoSections([&](const DWARFSection &S) {
-    NormalUnits.addUnitsForSection(*this, S, DW_SECT_INFO);
-  });
-  NormalUnits.finishedInfoUnits();
-  DObj->forEachTypesSections([&](const DWARFSection &S) {
-    NormalUnits.addUnitsForSection(*this, S, DW_SECT_EXT_TYPES);
-  });
-}
-
-void DWARFContext::parseDWOUnits(bool Lazy) {
-  if (!DWOUnits.empty())
-    return;
-  DObj->forEachInfoDWOSections([&](const DWARFSection &S) {
-    DWOUnits.addUnitsForDWOSection(*this, S, DW_SECT_INFO, Lazy);
-  });
-  DWOUnits.finishedInfoUnits();
-  DObj->forEachTypesDWOSections([&](const DWARFSection &S) {
-    DWOUnits.addUnitsForDWOSection(*this, S, DW_SECT_EXT_TYPES, Lazy);
-  });
+DWARFUnitVector &DWARFContext::getDWOUnits(bool Lazy) {
+  return State->getDWOUnits(Lazy);
 }
 
 DWARFCompileUnit *DWARFContext::getCompileUnitForOffset(uint64_t Offset) {
-  parseNormalUnits();
   return dyn_cast_or_null<DWARFCompileUnit>(
-      NormalUnits.getUnitForOffset(Offset));
+      State->getNormalUnits().getUnitForOffset(Offset));
 }
 
 DWARFCompileUnit *DWARFContext::getCompileUnitForCodeAddress(uint64_t Address) {
   uint64_t CUOffset = getDebugAranges()->findAddress(Address);
   return getCompileUnitForOffset(CUOffset);
 }
 
 DWARFCompileUnit *DWARFContext::getCompileUnitForDataAddress(uint64_t Address) {
for (uint32_t i = 0, n = InlinedChain.size(); i != n; i++) {
     }
     InliningInfo.addFrame(Frame);
   }
   return InliningInfo;
 }
 
 std::shared_ptr<DWARFContext>
 DWARFContext::getDWOContext(StringRef AbsolutePath) {
-  if (auto S = DWP.lock()) {
-    DWARFContext *Ctxt = S->Context.get();
-    return std::shared_ptr<DWARFContext>(std::move(S), Ctxt);
-  }
-
-  std::weak_ptr<DWOFile> *Entry = &DWOFiles[AbsolutePath];
-
-  if (auto S = Entry->lock()) {
-    DWARFContext *Ctxt = S->Context.get();
-    return std::shared_ptr<DWARFContext>(std::move(S), Ctxt);
-  }
-
-  Expected<OwningBinary<ObjectFile>> Obj = [&] {
-    if (!CheckedForDWP) {
-      SmallString<128> DWPName;
-      auto Obj = object::ObjectFile::createObjectFile(
-          this->DWPName.empty()
-              ? (DObj->getFileName() + ".dwp").toStringRef(DWPName)
-              : StringRef(this->DWPName));
-      if (Obj) {
-        Entry = &DWP;
-        return Obj;
-      } else {
-        CheckedForDWP = true;
-        // TODO: Should this error be handled (maybe in a high verbosity mode)
-        // before falling back to .dwo files?
-        consumeError(Obj.takeError());
-      }
-    }
-
-    return object::ObjectFile::createObjectFile(AbsolutePath);
-  }();
-
-  if (!Obj) {
-    // TODO: Actually report errors helpfully.
-    consumeError(Obj.takeError());
-    return nullptr;
-  }
-
-  auto S = std::make_shared<DWOFile>();
-  S->File = std::move(Obj.get());
-  S->Context = DWARFContext::create(*S->File.getBinary(),
-                                    ProcessDebugRelocations::Ignore);
-  *Entry = S;
-  auto *Ctxt = S->Context.get();
-  return std::shared_ptr<DWARFContext>(std::move(S), Ctxt);
+  return State->getDWOContext(AbsolutePath);
 }
 
 static Error createError(const Twine &Reason, llvm::Error E) {
   return make_error<StringError>(Reason + toString(std::move(E)),
                                  inconvertibleErrorCode());
 }
 
 /// SymInfo contains information about symbol: it's address

 };
 } // namespace
 
 std::unique_ptr<DWARFContext>
 DWARFContext::create(const object::ObjectFile &Obj,
                      ProcessDebugRelocations RelocAction,
                      const LoadedObjectInfo *L, std::string DWPName,
                      std::function<void(Error)> RecoverableErrorHandler,
-                     std::function<void(Error)> WarningHandler) {
+                     std::function<void(Error)> WarningHandler,
+                     bool ThreadSafe) {
   auto DObj = std::make_unique<DWARFObjInMemory>(
       Obj, L, RecoverableErrorHandler, WarningHandler, RelocAction);
-  return std::make_unique<DWARFContext>(std::move(DObj), std::move(DWPName),
+  return std::make_unique<DWARFContext>(std::move(DObj),
+                                        std::move(DWPName),
                                         RecoverableErrorHandler,
-                                        WarningHandler);
+                                        WarningHandler,
+                                        ThreadSafe);
 }
 
 std::unique_ptr<DWARFContext>
 DWARFContext::create(const StringMap<std::unique_ptr<MemoryBuffer>> &Sections,
                      uint8_t AddrSize, bool isLittleEndian,
                      std::function<void(Error)> RecoverableErrorHandler,
-                     std::function<void(Error)> WarningHandler) {
+                     std::function<void(Error)> WarningHandler,
+                     bool ThreadSafe) {
   auto DObj =
       std::make_unique<DWARFObjInMemory>(Sections, AddrSize, isLittleEndian);
   return std::make_unique<DWARFContext>(
-      std::move(DObj), "", RecoverableErrorHandler, WarningHandler);
+      std::move(DObj), "", RecoverableErrorHandler, WarningHandler, ThreadSafe);
 }
 
 uint8_t DWARFContext::getCUAddrSize() {
   // In theory, different compile units may have different address byte
   // sizes, but for simplicity we just use the address byte size of the
   // first compile unit. In practice the address size field is repeated across
   // various DWARF headers (at least in version 5) to make it easier to dump
   // them independently, not to enable varying the address size.
   auto CUs = compile_units();
   return CUs.empty() ? 0 : (*CUs.begin())->getAddressByteSize();
 }

llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp

for (const object::SectionRef &Sect : Obj.sections()) {
     const uint64_t Size = Sect.getSize();
     if (Size == 0)
       continue;
     const uint64_t StartAddr = Sect.getAddress();
     TextRanges.insert(AddressRange(StartAddr, StartAddr + Size));
   }
 
   // Make sure there is DWARF to convert first.
-  std::unique_ptr<DWARFContext> DICtx = DWARFContext::create(Obj);
+  std::unique_ptr<DWARFContext> DICtx = DWARFContext::create(
+      Obj,
+      /*RelocAction=*/DWARFContext::ProcessDebugRelocations::Process,
+      nullptr,
+      /*DWPName=*/"",
+      /*RecoverableErrorHandler=*/WithColor::defaultErrorHandler,
+      /*WarningHandler=*/WithColor::defaultWarningHandler,
+      /*ThreadSafe*/true);
   if (!DICtx)
     return createStringError(std::errc::invalid_argument,
                              "unable to create DWARF context");
 
   // Make a DWARF transformer object and populate the ranges of the code
   // so we don't end up adding invalid functions to GSYM data.
   DwarfTransformer DT(*DICtx, Gsym);
   if (!TextRanges.empty())