@JDevlieghere do you have any opinions on this?

Changed to regular mutex

@JDevlieghere I updated this to two shared mutexes because I'm assuming you can have more than one concurrent read and more than one concurrent write. If this is wrong I can go back to two regular mutexes.

Update to two shared mutexes

Lock earlier in operator=

In D157648#4578420, @JDevlieghere wrote:

Is the reported race specifically about the shared pointer being accessed concurrently or operations on the IOHandler?

I have one nit that I'll leave up to you whether it should be addressed now or not.

LGTM

On the description you mention unique_ptr but I only see shared_ptr here. Was that a typo or do you mean that users of this class kept it behind a unique_ptr?

Remove lock from DeepCopy

Addressed comments

Do all of these need to be protected with a mutex? In your description you're saying TSan is detecting data races. What piece of data are you observing the data race on?

Addressed comments

Ping

Personally I vote for the formatter kind on every line, so we don't need to jump around the logs when reading them, up to you though.

I've bumped into this problem before, so really like this change. It's a bit sad that you'll only print the kind of data formatter once though, so someone reading the logs might have to jump back and forth to understand what kind of formatter they're dealing with. There's also __PRETTY_FUNCTION__, which shows the generic type parameters by default, but I think it's not a standard macro (although it's implemented by both clang and gcc). What do you think of having a macro like:

Addressed comments

ping :)

@kastiglione I wonder if we should hint in all 3 command objects (expr, frame var and DWIM) or if we should limit this to DWIM instead (since DWIM is the one aliased to p and po and I don't think vo or expr -O are often used unintentionally).

Addressed comments

Remove failure check

Add test

Add radar

Add llvm_unreachable

In D150826#4351518, @bulbazord wrote:

Where do you plan on using this? Downstream I assume?

Changed implementation to GetCompileOptions

In D147748#4251543, @JDevlieghere wrote:

At a higher level I wonder if this is really the best interface. If you ever need all the compile options, you probably want something like Args SymbolFile::GetCompileOptions(). Wouldn't that be a more generic way to do the same thing here? Or do we expect that for DW_AT_APPLE_flags the only possible use case is to check whether a particular flag is set?

Addressed comments

Updated MetadataTest.cpp

@aaron.ballman thanks all the code reviews.

Added release note

Updated git commit message

Updated to debug_trampoline, and addressed comments

Abandoning in favor of https://reviews.llvm.org/D147292/.

In D147292#4238820, @JDevlieghere wrote:

There seems to be overlap in the code added in this patch and D146679. Does one supersede the other or is there a dependency? If it's the latter you should extract that into a separate patch and make it a parent revision of this and D146679.

In D146595#4235333, @aprantl wrote:

I hope I'm not kicking off a long bike-shedding thread, but I would propose to either call the attribute transparent to match the DWARF attribute name, or if we want to be more descriptive, debug_transparent, or transparentdebug to fit better with other attributes such as nodebug. My vote is on transparent.

@aaron.ballman @dblaikie I've changed the patch to introduce a more general "transparent_stepping" attribute, which doesn't take in a string (similar to what David suggested), please take a look and let me know what you think.

Updating to "transparent_stepping" attribute