Merge pull request #8719 from augusto2112/guard-ast

Introduce LockGuarded and make swift::ASTContext accesses thread safe

Author: adrian-prantl

lldb/source/Plugins/ExpressionParser/Swift/SwiftExpressionParser.cpp

@@ -13,9 +13,11 @@
 #ifndef liblldb_SwiftExpressionParser_h_
 #define liblldb_SwiftExpressionParser_h_
 
+#include "Plugins/TypeSystem/Swift/SwiftASTContext.h"
 #include "SwiftASTManipulator.h"
 
 #include "Plugins/ExpressionParser/Clang/IRForTarget.h"
+#include "SwiftPersistentExpressionState.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Expression/ExpressionParser.h"
@@ -24,13 +26,15 @@
 #include "lldb/Target/Target.h"
 #include "lldb/lldb-public.h"
 
+#include "swift/SIL/SILDebuggerClient.h"
 #include <string>
 #include <vector>
 
 namespace lldb_private {
 
 class IRExecutionUnit;
 class SwiftLanguageRuntime;
+class LLDBNameLookup;
 //----------------------------------------------------------------------
 /// @class SwiftExpressionParser SwiftExpressionParser.h
 /// "lldb/Expression/SwiftExpressionParser.h"
@@ -178,6 +182,27 @@ class SwiftExpressionParser : public ExpressionParser {
   typedef std::map<const char *, SILVariableInfo> SILVariableMap;
 
 private:
+  /// This holds the result of ParseAndImport.
+  struct ParsedExpression {
+    std::unique_ptr<SwiftASTManipulator> code_manipulator;
+    swift::ModuleDecl &module;
+    LLDBNameLookup &external_lookup;
+    swift::SourceFile &source_file;
+    std::string main_filename;
+    unsigned buffer_id;
+  };
+
+  /// Attempt to parse an expression and import all the Swift modules
+  /// the expression and its context depend on.
+  llvm::Expected<ParsedExpression>
+  ParseAndImport(SwiftASTContext::ScopedDiagnostics &expr_diagnostics,
+                 SwiftExpressionParser::SILVariableMap &variable_map,
+                 unsigned &buffer_id, DiagnosticManager &diagnostic_manager);
+
+  /// Initialize the SwiftASTContext and return the wrapped
+  /// ThreadSafeASTContext when successful.
+  ThreadSafeASTContext GetASTContext(DiagnosticManager &diagnostic_manager);
+
   /// The expression to be parsed.
   Expression &m_expr;
   /// The context to use for IR generation.
@@ -206,7 +231,41 @@ class SwiftExpressionParser : public ExpressionParser {
 
   /// Indicates whether the call to Parse of this type is cacheable.
   bool m_is_cacheable;
+
+  /// Once token to initialize the swift::ASTContext.
+  llvm::once_flag m_ast_init_once_flag;
+
+  /// Indicates if the ThreadSafeASTContext was initialized correctly.
+  bool m_ast_init_successful;
+};
+
+class LLDBNameLookup : public swift::SILDebuggerClient {
+public:
+  LLDBNameLookup(swift::SourceFile &source_file,
+                 SwiftExpressionParser::SILVariableMap &variable_map,
+                 SymbolContext &sc, ExecutionContextScope &exe_scope);
+
+  swift::SILValue emitLValueForVariable(swift::VarDecl *var,
+                                        swift::SILBuilder &builder) override;
+
+  SwiftPersistentExpressionState::SwiftDeclMap &GetStagedDecls();
+
+  void RegisterTypeAliases(
+      llvm::SmallVectorImpl<swift::TypeAliasDecl *> &type_aliases);
+
+protected:
+  Log *m_log;
+  swift::SourceFile &m_source_file;
+  SwiftExpressionParser::SILVariableMap &m_variable_map;
+  SymbolContext m_sc;
+  SwiftPersistentExpressionState *m_persistent_vars = nullptr;
+
+  // Subclasses stage globalized decls in this map. They get copied
+  // over to the SwiftPersistentVariable store if the parse succeeds.
+  SwiftPersistentExpressionState::SwiftDeclMap m_staged_decls;
+  llvm::SmallVector<swift::TypeAliasDecl *> m_type_aliases;
 };
+
 } // namespace lldb_private
 
 #endif // liblldb_SwiftExpressionParser_h_

lldb/source/Plugins/ExpressionParser/Swift/SwiftExpressionParser.h

@@ -574,7 +574,7 @@ void SwiftREPL::CompleteCode(const std::string &current_code,
   SwiftASTContextForExpressions *swift_ast = m_swift_ast;
 
   if (swift_ast) {
-    swift::ASTContext *ast = swift_ast->GetASTContext();
+    ThreadSafeASTContext ast = swift_ast->GetASTContext();
     swift::REPLCompletions completions;
     SourceModule completion_module_info;
     completion_module_info.path.push_back(ConstString("repl"));
@@ -587,7 +587,7 @@ void SwiftREPL::CompleteCode(const std::string &current_code,
       repl_module = swift_ast->CreateModule(completion_module_info, error,
                                             importInfo);
       std::optional<unsigned> bufferID;
-      swift::SourceFile *repl_source_file = new (*ast) swift::SourceFile(
+      swift::SourceFile *repl_source_file = new (**ast) swift::SourceFile(
           *repl_module, swift::SourceFileKind::Main, bufferID);
       repl_module->addFile(*repl_source_file);
       swift::performImportResolution(*repl_source_file);

lldb/source/Plugins/ExpressionParser/Swift/SwiftREPL.cpp

@@ -378,7 +378,7 @@ static llvm::Error AddVariableInfo(
       ss.flush();
       log->Printf("Adding injected self: type (%p) context(%p) is: %s",
                   static_cast<void *>(swift_type.getPointer()),
-                  static_cast<void *>(ast_context.GetASTContext()), s.c_str());
+                  static_cast<void *>(*ast_context.GetASTContext()), s.c_str());
     }
   // A one-off clone of variable_sp with the type replaced by target_type.
   auto patched_variable_sp = std::make_shared<lldb_private::Variable>(

lldb/source/Plugins/ExpressionParser/Swift/SwiftUserExpression.cpp

@@ -150,7 +150,8 @@ static std::string TranslateObjCNameToSwiftName(std::string className,
     }
   };
 
-  MyConsumer consumer(swift::ObjCSelector(*ctx->GetASTContext(), numArguments,
+  ThreadSafeASTContext ast_ctx = ctx->GetASTContext();
+  MyConsumer consumer(swift::ObjCSelector(**ast_ctx, numArguments,
                                           selectorIdentifiers));
   // FIXME(mracek): Switch to a new API that translates the Clang class name
   // to Swift class name, once this API exists. Now we assume they are the same.

lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp

@@ -0,0 +1,39 @@
+//===----------------- LockGuarded.h ----------------------------*- C++ -*-===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef liblldb_SwiftLockGuarded_h_
+#define liblldb_SwiftLockGuarded_h_
+
+#include <mutex>
+
+namespace lldb_private {
+/// A generic wrapper around a resource which holds a lock to ensure
+/// exclusive access.
+template <typename Resource> struct LockGuarded {
+  LockGuarded(Resource *resource, std::recursive_mutex &mutex)
+      : m_resource(resource), m_lock(mutex, std::adopt_lock) {}
+
+  LockGuarded() = default;
+
+  Resource *operator->() const { return m_resource; }
+
+  Resource *operator*() const { return m_resource; }
+
+  operator bool() const { return m_resource != nullptr; }
+
+private:
+  Resource *m_resource;
+  std::unique_lock<std::recursive_mutex> m_lock;
+};
+
+} // namespace lldb_private
+#endif

lldb/source/Plugins/LanguageRuntime/Swift/LockGuarded.h

@@ -24,6 +24,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Memory.h"
+#include "LockGuarded.h"
 
 namespace swift {
 namespace Demangle {
@@ -146,31 +147,6 @@ class ReflectionContextInterface {
   StripSignedPointer(swift::remote::RemoteAbsolutePointer pointer) = 0;
 };
 
-/// A wrapper around TargetReflectionContext, which holds a lock to ensure
-/// exclusive access.
-struct ThreadSafeReflectionContext {
-  ThreadSafeReflectionContext(ReflectionContextInterface *reflection_ctx,
-                              std::recursive_mutex &mutex)
-      : m_reflection_ctx(reflection_ctx), m_lock(mutex, std::adopt_lock) {}
-
-  static ThreadSafeReflectionContext MakeInvalid() {
-    // This exists so we can create an "empty" reflection context in the stub
-    // language runtime.
-    static std::recursive_mutex mutex;
-    return ThreadSafeReflectionContext(nullptr, mutex);
-  }
-
-  ReflectionContextInterface *operator->() const { return m_reflection_ctx; }
-
-  operator bool() const { return m_reflection_ctx != nullptr; }
-
-private:
-  ReflectionContextInterface *m_reflection_ctx;
-  // This lock operates on a recursive mutex because the initialization
-  // of ReflectionContext recursive calls itself (see
-  // SwiftLanguageRuntimeImpl::SetupReflection).
-  std::lock_guard<std::recursive_mutex> m_lock;
-};
-
+using ThreadSafeReflectionContext = LockGuarded<ReflectionContextInterface>;
 } // namespace lldb_private
 #endif

lldb/source/Plugins/LanguageRuntime/Swift/ReflectionContextInterface.h

@@ -235,7 +235,7 @@ class SwiftLanguageRuntimeStub {
 
   ThreadSafeReflectionContext GetReflectionContext() {
     STUB_LOG();
-    return ThreadSafeReflectionContext::MakeInvalid();
+    return ThreadSafeReflectionContext();
   }
 
   bool GetDynamicTypeAndAddress(ValueObject &in_value,

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

@@ -51,12 +51,15 @@ class TypeBase;
 } // namespace swift
 
 namespace lldb_private {
-struct ThreadSafeReflectionContext;
+template <typename T>
+struct LockGuarded;
 
 class SwiftLanguageRuntimeStub;
 class SwiftLanguageRuntimeImpl;
 class ReflectionContextInterface;
 
+using ThreadSafeReflectionContext = LockGuarded<ReflectionContextInterface>;
+
 class SwiftLanguageRuntime : public LanguageRuntime {
 protected:
   SwiftLanguageRuntime(Process *process);

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.h

@@ -59,17 +59,18 @@ swift::remoteAST::RemoteASTContext &
 SwiftLanguageRuntimeImpl::GetRemoteASTContext(SwiftASTContext &swift_ast_ctx) {
   // If we already have a remote AST context for this AST context,
   // return it.
-  auto known = m_remote_ast_contexts.find(swift_ast_ctx.GetASTContext());
+  ThreadSafeASTContext ast_ctx = swift_ast_ctx.GetASTContext();
+  auto known = m_remote_ast_contexts.find(*ast_ctx);
   if (known != m_remote_ast_contexts.end())
     return *known->second;
 
   // Initialize a new remote AST context.
   (void)GetReflectionContext();
   auto remote_ast_up = std::make_unique<swift::remoteAST::RemoteASTContext>(
-      *swift_ast_ctx.GetASTContext(), GetMemoryReader());
+      **ast_ctx, GetMemoryReader());
   auto &remote_ast = *remote_ast_up;
   m_remote_ast_contexts.insert(
-      {swift_ast_ctx.GetASTContext(), std::move(remote_ast_up)});
+      {*ast_ctx, std::move(remote_ast_up)});
   return remote_ast;
 }
 
@@ -521,7 +522,8 @@ SwiftLanguageRuntimeImpl::GetMetadataPromise(const SymbolContext *sc,
   if (addr == 0 || addr == LLDB_INVALID_ADDRESS)
     return nullptr;
 
-  auto key = std::make_pair(swift_ast_ctx->GetASTContext(), addr);
+  ThreadSafeASTContext ast_ctx = swift_ast_ctx->GetASTContext();
+  auto key = std::make_pair(*ast_ctx, addr);
   auto iter = m_promises_map.find(key);
   if (iter != m_promises_map.end())
     return iter->second;