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 #51

Merged
merged 1 commit into from
May 1, 2020
Merged

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

merged 1 commit into from
May 1, 2020

Conversation

androm3da
Copy link

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

@androm3da
Copy link
Author

When I test this locally, CodeGen/AArch64/wineh-try-catch.ll fails. But it also fails on the baseline.

@androm3da
Copy link
Author

In fact, all these tests fail on baseline:

********************
Failing Tests (3):
    LLVM-Unit :: Analysis/./AnalysisTests/TargetLibraryInfoTest.ValidProto
    LLVM :: CodeGen/AArch64/wineh-try-catch.ll
    LLVM :: MC/RISCV/pcrel-fixups.s


@androm3da
Copy link
Author

@cuviper please let me know if this was the right branch to target for current rustc

@cuviper
Copy link
Member

cuviper commented May 1, 2020

Looks good, thanks!

@cuviper cuviper merged commit 94c0e0c into rust-lang:rustc/9.0-2019-12-19 May 1, 2020
vext01 added a commit to vext01/llvm-project that referenced this pull request Dec 21, 2022
51: Gate our changes to the `.llvmbbaddrmap` section. 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
2 participants