Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle part-word LL/SC in atomic expansion pass #50

Merged
merged 1 commit into from
May 1, 2020
Merged

Handle part-word LL/SC in atomic expansion pass #50

merged 1 commit into from
May 1, 2020

Conversation

androm3da
Copy link

Differential Revision: https://reviews.llvm.org/D77213

@androm3da
Copy link
Author

This cherry-pick resolves an assertion I get when trying to build libstd for hexagon-unknown-linux-musl.

@cuviper
Copy link
Member

cuviper commented Apr 29, 2020

Can you try to redo the cherry-pick with the original commit metadata intact? e.g. git cherry-pick -x ID.

Edit: Oh sorry, you did actually preserve authorship and all. They just didn't keep the commit message the same as the review message, so I thought it was missing. I'll review this in the morning.

@nikic
Copy link

nikic commented Apr 29, 2020

This commit is against the 10.x branch, while Rust still uses 9.x. The patch looks pretty involved though, so might not be easy to backport.

@androm3da
Copy link
Author

The commit is technically on upstream master (11.x branch) but applies cleanly to 10.x. But if it's necessary to put it on 9.x, then I'll look into backporting it.

@cuviper
Copy link
Member

cuviper commented Apr 29, 2020

It's just that rust-lang/rust is still using the 9.x branch, so if you want this to be useful soon, it will have to be applied there. We'll switch to the 10.x branch once we've satisfied the current performance concerns (or we decide to eat that loss).

@androm3da
Copy link
Author

Sure, makes sense. Can we consider merging it to 10.x anyways? Even if it goes into 9.x I'll need it on 10.x branch for when rust upgrades.

It looks like it may not be too hard to backport to 9.x, in progress now.

@androm3da
Copy link
Author

Sure, makes sense. Can we consider merging it to 10.x anyways? Even if it goes into 9.x I'll need it on 10.x branch for when rust upgrades.

It looks like it may not be too hard to backport to 9.x, in progress now.

Created PR #51 -- is that the right 9.x branch?

@cuviper cuviper merged commit 6a831f0 into rust-lang:rustc/10.0-2020-02-05 May 1, 2020
vext01 added a commit to vext01/llvm-project that referenced this pull request Nov 28, 2022
50: Extend the .llvm_bb_addr_map section with PT decoding hints. r=ltratt a=vext01



Co-authored-by: Edd Barrett <vext01@gmail.com>
nikic pushed a commit to nikic/llvm-project that referenced this pull request May 14, 2023
…callback

The `TypeSystemMap::m_mutex` guards against concurrent modifications
of members of `TypeSystemMap`. In particular, `m_map`.

`TypeSystemMap::ForEach` iterates through the entire `m_map` calling
a user-specified callback for each entry. This is all done while
`m_mutex` is locked. However, there's nothing that guarantees that
the callback itself won't call back into `TypeSystemMap` APIs on the
same thread. This lead to double-locking `m_mutex`, which is undefined
behaviour. We've seen this cause a deadlock in the swift plugin with
following backtrace:

```

int main() {
    std::unique_ptr<int> up = std::make_unique<int>(5);

    volatile int val = *up;
    return val;
}

clang++ -std=c++2a -g -O1 main.cpp

./bin/lldb -o “br se -p return” -o run -o “v *up” -o “expr *up” -b
```

```
frame rust-lang#4: std::lock_guard<std::mutex>::lock_guard
frame rust-lang#5: lldb_private::TypeSystemMap::GetTypeSystemForLanguage <<<< Lock rust-lang#2
frame rust-lang#6: lldb_private::TypeSystemMap::GetTypeSystemForLanguage
frame rust-lang#7: lldb_private::Target::GetScratchTypeSystemForLanguage
...
frame rust-lang#26: lldb_private::SwiftASTContext::LoadLibraryUsingPaths
frame rust-lang#27: lldb_private::SwiftASTContext::LoadModule
frame rust-lang#30: swift::ModuleDecl::collectLinkLibraries
frame rust-lang#31: lldb_private::SwiftASTContext::LoadModule
frame rust-lang#34: lldb_private::SwiftASTContext::GetCompileUnitImportsImpl
frame rust-lang#35: lldb_private::SwiftASTContext::PerformCompileUnitImports
frame rust-lang#36: lldb_private::TypeSystemSwiftTypeRefForExpressions::GetSwiftASTContext
frame rust-lang#37: lldb_private::TypeSystemSwiftTypeRefForExpressions::GetPersistentExpressionState
frame rust-lang#38: lldb_private::Target::GetPersistentSymbol
frame rust-lang#41: lldb_private::TypeSystemMap::ForEach                 <<<< Lock #1
frame rust-lang#42: lldb_private::Target::GetPersistentSymbol
frame rust-lang#43: lldb_private::IRExecutionUnit::FindInUserDefinedSymbols
frame rust-lang#44: lldb_private::IRExecutionUnit::FindSymbol
frame rust-lang#45: lldb_private::IRExecutionUnit::MemoryManager::GetSymbolAddressAndPresence
frame rust-lang#46: lldb_private::IRExecutionUnit::MemoryManager::findSymbol
frame rust-lang#47: non-virtual thunk to lldb_private::IRExecutionUnit::MemoryManager::findSymbol
frame rust-lang#48: llvm::LinkingSymbolResolver::findSymbol
frame rust-lang#49: llvm::LegacyJITSymbolResolver::lookup
frame rust-lang#50: llvm::RuntimeDyldImpl::resolveExternalSymbols
frame rust-lang#51: llvm::RuntimeDyldImpl::resolveRelocations
frame rust-lang#52: llvm::MCJIT::finalizeLoadedModules
frame rust-lang#53: llvm::MCJIT::finalizeObject
frame rust-lang#54: lldb_private::IRExecutionUnit::ReportAllocations
frame rust-lang#55: lldb_private::IRExecutionUnit::GetRunnableInfo
frame rust-lang#56: lldb_private::ClangExpressionParser::PrepareForExecution
frame rust-lang#57: lldb_private::ClangUserExpression::TryParse
frame rust-lang#58: lldb_private::ClangUserExpression::Parse
```

Our solution is to simply iterate over a local copy of `m_map`.

**Testing**

* Confirmed on manual reproducer (would reproduce 100% of the time
  before the patch)

Differential Revision: https://reviews.llvm.org/D149949
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants