Return a shared_ptr from ScratchTypeSystemClang::GetForTarget()

The current interface theoretically could lead to a use-after-free when a client holds on to the returned pointer. Fix this by returning a shared_ptr to the scratch typesystem. rdar://103619233 Differential Revision: https://reviews.llvm.org/D141100
llvm · Jan 9, 2023 · f8d7ab8 · f8d7ab8
1 parent 6b90f67
commit f8d7ab8
Show file tree

Hide file tree

Showing 29 changed files with 172 additions and 158 deletions.
diff --git a/lldb/include/lldb/lldb-forward.h b/lldb/include/lldb/lldb-forward.h
@@ -258,6 +258,7 @@ class TypeNameSpecifierImpl;
 class TypeSummaryImpl;
 class TypeSummaryOptions;
 class TypeSystem;
+class TypeSystemClang;
 class UUID;
 class UnixSignals;
 class Unwind;
@@ -432,6 +433,7 @@ typedef std::shared_ptr<lldb_private::TypeMemberFunctionImpl>
 typedef std::shared_ptr<lldb_private::TypeEnumMemberImpl> TypeEnumMemberImplSP;
 typedef std::shared_ptr<lldb_private::TypeFilterImpl> TypeFilterImplSP;
 typedef std::shared_ptr<lldb_private::TypeSystem> TypeSystemSP;
+typedef std::shared_ptr<lldb_private::TypeSystemClang> TypeSystemClangSP;
 typedef std::weak_ptr<lldb_private::TypeSystem> TypeSystemWP;
 typedef std::shared_ptr<lldb_private::TypeFormatImpl> TypeFormatImplSP;
 typedef std::shared_ptr<lldb_private::TypeNameSpecifierImpl>

diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -1112,14 +1112,14 @@ DynamicLoaderDarwin::GetThreadLocalData(const lldb::ModuleSP module_sp,
         }
         StackFrameSP frame_sp = thread_sp->GetStackFrameAtIndex(0);
         if (frame_sp) {
-          TypeSystemClang *clang_ast_context =
+          TypeSystemClangSP scratch_ts_sp =
               ScratchTypeSystemClang::GetForTarget(target);
 
-          if (!clang_ast_context)
+          if (!scratch_ts_sp)
             return LLDB_INVALID_ADDRESS;
 
           CompilerType clang_void_ptr_type =
-              clang_ast_context->GetBasicType(eBasicTypeVoid).GetPointerType();
+              scratch_ts_sp->GetBasicType(eBasicTypeVoid).GetPointerType();
           Address pthread_getspecific_addr = GetPthreadSetSpecificAddress();
           if (pthread_getspecific_addr.IsValid()) {
             EvaluateExpressionOptions options;

diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
@@ -260,9 +260,9 @@ bool DynamicLoaderMacOS::NotifyBreakpointHit(void *baton,
     // Build up the value array to store the three arguments given above, then
     // get the values from the ABI:
 
-    TypeSystemClang *clang_ast_context =
+    TypeSystemClangSP scratch_ts_sp =
         ScratchTypeSystemClang::GetForTarget(process->GetTarget());
-    if (!clang_ast_context)
+    if (!scratch_ts_sp)
       return false;
 
     ValueList argument_values;
@@ -273,13 +273,13 @@ bool DynamicLoaderMacOS::NotifyBreakpointHit(void *baton,
     Value headers_value; // uint64_t machHeaders[] (aka void*)
 
     CompilerType clang_void_ptr_type =
-        clang_ast_context->GetBasicType(eBasicTypeVoid).GetPointerType();
+        scratch_ts_sp->GetBasicType(eBasicTypeVoid).GetPointerType();
     CompilerType clang_uint32_type =
-        clang_ast_context->GetBuiltinTypeForEncodingAndBitSize(
-            lldb::eEncodingUint, 32);
+        scratch_ts_sp->GetBuiltinTypeForEncodingAndBitSize(lldb::eEncodingUint,
+                                                           32);
     CompilerType clang_uint64_type =
-        clang_ast_context->GetBuiltinTypeForEncodingAndBitSize(
-            lldb::eEncodingUint, 32);
+        scratch_ts_sp->GetBuiltinTypeForEncodingAndBitSize(lldb::eEncodingUint,
+                                                           32);
 
     mode_value.SetValueType(Value::ValueType::Scalar);
     mode_value.SetCompilerType(clang_uint32_type);

diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
@@ -357,19 +357,19 @@ bool DynamicLoaderMacOSXDYLD::NotifyBreakpointHit(
     // Build up the value array to store the three arguments given above, then
     // get the values from the ABI:
 
-    TypeSystemClang *clang_ast_context =
+    TypeSystemClangSP scratch_ts_sp =
         ScratchTypeSystemClang::GetForTarget(process->GetTarget());
-    if (!clang_ast_context)
+    if (!scratch_ts_sp)
       return false;
 
     ValueList argument_values;
     Value input_value;
 
     CompilerType clang_void_ptr_type =
-        clang_ast_context->GetBasicType(eBasicTypeVoid).GetPointerType();
+        scratch_ts_sp->GetBasicType(eBasicTypeVoid).GetPointerType();
     CompilerType clang_uint32_type =
-        clang_ast_context->GetBuiltinTypeForEncodingAndBitSize(
-            lldb::eEncodingUint, 32);
+        scratch_ts_sp->GetBuiltinTypeForEncodingAndBitSize(lldb::eEncodingUint,
+                                                           32);
     input_value.SetValueType(Value::ValueType::Scalar);
     input_value.SetCompilerType(clang_uint32_type);
     //        input_value.SetContext (Value::eContextTypeClangType,

diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
@@ -470,15 +470,15 @@ void ASTResultSynthesizer::CommitPersistentDecls() {
 
   auto *persistent_vars = llvm::cast<ClangPersistentVariables>(state);
 
-  TypeSystemClang *scratch_ctx = ScratchTypeSystemClang::GetForTarget(
+  lldb::TypeSystemClangSP scratch_ts_sp = ScratchTypeSystemClang::GetForTarget(
       m_target, m_ast_context->getLangOpts());
 
   for (clang::NamedDecl *decl : m_decls) {
     StringRef name = decl->getName();
     ConstString name_cs(name.str().c_str());
 
     Decl *D_scratch = persistent_vars->GetClangASTImporter()->DeportDecl(
-        &scratch_ctx->getASTContext(), decl);
+        &scratch_ts_sp->getASTContext(), decl);
 
     if (!D_scratch) {
       Log *log = GetLog(LLDBLog::Expressions);
@@ -497,7 +497,7 @@ void ASTResultSynthesizer::CommitPersistentDecls() {
 
     if (NamedDecl *NamedDecl_scratch = dyn_cast<NamedDecl>(D_scratch))
       persistent_vars->RegisterPersistentDecl(name_cs, NamedDecl_scratch,
-                                              scratch_ctx);
+                                              scratch_ts_sp);
   }
 }
 

diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -78,14 +78,14 @@ ClangASTSource::~ClangASTSource() {
   // query the deleted ASTContext for additional type information.
   // We unregister from *all* scratch ASTContexts in case a type got exported
   // to a scratch AST that isn't the best fitting scratch ASTContext.
-  TypeSystemClang *scratch_ast = ScratchTypeSystemClang::GetForTarget(
+  lldb::TypeSystemClangSP scratch_ts_sp = ScratchTypeSystemClang::GetForTarget(
       *m_target, ScratchTypeSystemClang::DefaultAST, false);
 
-  if (!scratch_ast)
+  if (!scratch_ts_sp)
     return;
 
   ScratchTypeSystemClang *default_scratch_ast =
-      llvm::cast<ScratchTypeSystemClang>(scratch_ast);
+      llvm::cast<ScratchTypeSystemClang>(scratch_ts_sp.get());
   // Unregister from the default scratch AST (and all sub-ASTs).
   default_scratch_ast->ForgetSource(m_ast_context, *m_ast_importer_sp);
 }

diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -205,7 +205,7 @@ ClangExpressionDeclMap::TargetInfo ClangExpressionDeclMap::GetTargetInfo() {
 TypeFromUser ClangExpressionDeclMap::DeportType(TypeSystemClang &target,
                                                 TypeSystemClang &source,
                                                 TypeFromParser parser_type) {
-  assert(&target == GetScratchContext(*m_target));
+  assert(&target == GetScratchContext(*m_target).get());
   assert((TypeSystem *)&source ==
          parser_type.GetTypeSystem().GetSharedPointer().get());
   assert(&source.getASTContext() == m_ast_context);
@@ -242,7 +242,7 @@ bool ClangExpressionDeclMap::AddPersistentVariable(const NamedDecl *decl,
     if (target == nullptr)
       return false;
 
-    auto *clang_ast_context = GetScratchContext(*target);
+    auto clang_ast_context = GetScratchContext(*target);
     if (!clang_ast_context)
       return false;
 
@@ -280,7 +280,7 @@ bool ClangExpressionDeclMap::AddPersistentVariable(const NamedDecl *decl,
   if (target == nullptr)
     return false;
 
-  TypeSystemClang *context = GetScratchContext(*target);
+  auto context = GetScratchContext(*target);
   if (!context)
     return false;
 
@@ -1728,7 +1728,7 @@ void ClangExpressionDeclMap::AddOneGenericVariable(NameSearchContext &context,
   if (target == nullptr)
     return;
 
-  TypeSystemClang *scratch_ast_context = GetScratchContext(*target);
+  auto scratch_ast_context = GetScratchContext(*target);
   if (!scratch_ast_context)
     return;
 

diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
@@ -12,6 +12,7 @@
 #include <csignal>
 #include <cstdint>
 
+#include <memory>
 #include <vector>
 
 #include "ClangASTSource.h"
@@ -377,7 +378,7 @@ class ClangExpressionDeclMap : public ClangASTSource {
   /// Deallocate struct variables
   void DisableStructVars() { m_struct_vars.reset(); }
 
-  TypeSystemClang *GetScratchContext(Target &target) {
+  lldb::TypeSystemClangSP GetScratchContext(Target &target) {
     return ScratchTypeSystemClang::GetForTarget(target,
                                                 m_ast_context->getLangOpts());
   }

diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
@@ -21,6 +21,7 @@
 
 #include "llvm/ADT/StringMap.h"
 #include <optional>
+#include <memory>
 
 using namespace lldb;
 using namespace lldb_private;
@@ -85,15 +86,15 @@ ClangPersistentVariables::GetCompilerTypeFromPersistentDecl(
   return std::nullopt;
 }
 
-void ClangPersistentVariables::RegisterPersistentDecl(ConstString name,
-                                                      clang::NamedDecl *decl,
-                                                      TypeSystemClang *ctx) {
-  PersistentDecl p = {decl, ctx->weak_from_this()};
+void ClangPersistentVariables::RegisterPersistentDecl(
+    ConstString name, clang::NamedDecl *decl,
+    std::shared_ptr<TypeSystemClang> ctx) {
+  PersistentDecl p = {decl, ctx};
   m_persistent_decls.insert(std::make_pair(name.GetCString(), p));
 
   if (clang::EnumDecl *enum_decl = llvm::dyn_cast<clang::EnumDecl>(decl)) {
     for (clang::EnumConstantDecl *enumerator_decl : enum_decl->enumerators()) {
-      p = {enumerator_decl, ctx->weak_from_this()};
+      p = {enumerator_decl, ctx};
       m_persistent_decls.insert(std::make_pair(
           ConstString(enumerator_decl->getNameAsString()).GetCString(), p));
     }

diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h
@@ -70,7 +70,7 @@ class ClangPersistentVariables : public PersistentExpressionState {
   GetCompilerTypeFromPersistentDecl(ConstString type_name) override;
 
   void RegisterPersistentDecl(ConstString name, clang::NamedDecl *decl,
-                              TypeSystemClang *ctx);
+                              std::shared_ptr<TypeSystemClang> ctx);
 
   clang::NamedDecl *GetPersistentDecl(ConstString name);
 

diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -853,13 +853,13 @@ LibcxxWStringSummaryProvider(ValueObject &valobj, Stream &stream,
     return false;
 
   // std::wstring::size() is measured in 'characters', not bytes
-  TypeSystemClang *ast_context =
+  TypeSystemClangSP scratch_ts_sp =
       ScratchTypeSystemClang::GetForTarget(*valobj.GetTargetSP());
-  if (!ast_context)
+  if (!scratch_ts_sp)
     return false;
 
   auto wchar_t_size =
-      ast_context->GetBasicType(lldb::eBasicTypeWChar).GetByteSize(nullptr);
+      scratch_ts_sp->GetBasicType(lldb::eBasicTypeWChar).GetByteSize(nullptr);
   if (!wchar_t_size)
     return false;
 

diff --git a/lldb/source/Plugins/Language/ObjC/NSArray.cpp b/lldb/source/Plugins/Language/ObjC/NSArray.cpp
@@ -463,12 +463,12 @@ lldb_private::formatters::NSArrayMSyntheticFrontEndBase::
     NSArrayMSyntheticFrontEndBase(lldb::ValueObjectSP valobj_sp)
     : SyntheticChildrenFrontEnd(*valobj_sp), m_exe_ctx_ref(), m_id_type() {
   if (valobj_sp) {
-    auto *clang_ast_context = ScratchTypeSystemClang::GetForTarget(
+    TypeSystemClangSP scratch_ts_sp = ScratchTypeSystemClang::GetForTarget(
         *valobj_sp->GetExecutionContextRef().GetTargetSP());
-    if (clang_ast_context)
+    if (scratch_ts_sp)
       m_id_type = CompilerType(
-          clang_ast_context->weak_from_this(),
-          clang_ast_context->getASTContext().ObjCBuiltinIdTy.getAsOpaquePtr());
+          scratch_ts_sp->weak_from_this(),
+          scratch_ts_sp->getASTContext().ObjCBuiltinIdTy.getAsOpaquePtr());
     if (valobj_sp->GetProcessSP())
       m_ptr_size = valobj_sp->GetProcessSP()->GetAddressByteSize();
   }
@@ -609,11 +609,11 @@ lldb_private::formatters::GenericNSArrayISyntheticFrontEnd<D32, D64, Inline>::
   if (valobj_sp) {
     CompilerType type = valobj_sp->GetCompilerType();
     if (type) {
-      auto *clang_ast_context = ScratchTypeSystemClang::GetForTarget(
+      TypeSystemClangSP scratch_ts_sp = ScratchTypeSystemClang::GetForTarget(
           *valobj_sp->GetExecutionContextRef().GetTargetSP());
-      if (clang_ast_context)
-        m_id_type = clang_ast_context->GetType(
-            clang_ast_context->getASTContext().ObjCBuiltinIdTy);
+      if (scratch_ts_sp)
+        m_id_type = scratch_ts_sp->GetType(
+            scratch_ts_sp->getASTContext().ObjCBuiltinIdTy);
     }
   }
 }
@@ -776,11 +776,10 @@ lldb_private::formatters::NSArray1SyntheticFrontEnd::GetChildAtIndex(
   static const ConstString g_zero("[0]");
 
   if (idx == 0) {
-    auto *clang_ast_context =
+    TypeSystemClangSP scratch_ts_sp =
         ScratchTypeSystemClang::GetForTarget(*m_backend.GetTargetSP());
-    if (clang_ast_context) {
-      CompilerType id_type(
-          clang_ast_context->GetBasicType(lldb::eBasicTypeObjCID));
+    if (scratch_ts_sp) {
+      CompilerType id_type(scratch_ts_sp->GetBasicType(lldb::eBasicTypeObjCID));
       return m_backend.GetSyntheticChildAtOffset(
           m_backend.GetProcessSP()->GetAddressByteSize(), id_type, true,
           g_zero);

diff --git a/lldb/source/Plugins/Language/ObjC/NSDictionary.cpp b/lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
@@ -66,26 +66,25 @@ NSDictionary_Additionals::GetAdditionalSynthetics() {
 static CompilerType GetLLDBNSPairType(TargetSP target_sp) {
   CompilerType compiler_type;
 
-  TypeSystemClang *target_ast_context =
+  TypeSystemClangSP scratch_ts_sp =
       ScratchTypeSystemClang::GetForTarget(*target_sp);
 
-  if (target_ast_context) {
+  if (scratch_ts_sp) {
     ConstString g_lldb_autogen_nspair("__lldb_autogen_nspair");
 
-    compiler_type =
-        target_ast_context->GetTypeForIdentifier<clang::CXXRecordDecl>(
-            g_lldb_autogen_nspair);
+    compiler_type = scratch_ts_sp->GetTypeForIdentifier<clang::CXXRecordDecl>(
+        g_lldb_autogen_nspair);
 
     if (!compiler_type) {
-      compiler_type = target_ast_context->CreateRecordType(
+      compiler_type = scratch_ts_sp->CreateRecordType(
           nullptr, OptionalClangModuleID(), lldb::eAccessPublic,
           g_lldb_autogen_nspair.GetCString(), clang::TTK_Struct,
           lldb::eLanguageTypeC);
 
       if (compiler_type) {
         TypeSystemClang::StartTagDeclarationDefinition(compiler_type);
         CompilerType id_compiler_type =
-            target_ast_context->GetBasicType(eBasicTypeObjCID);
+            scratch_ts_sp->GetBasicType(eBasicTypeObjCID);
         TypeSystemClang::AddFieldToRecordType(
             compiler_type, "key", id_compiler_type, lldb::eAccessPublic, 0);
         TypeSystemClang::AddFieldToRecordType(

diff --git a/lldb/source/Plugins/Language/ObjC/NSError.cpp b/lldb/source/Plugins/Language/ObjC/NSError.cpp
@@ -83,13 +83,15 @@ bool lldb_private::formatters::NSError_SummaryProvider(
   }
 
   InferiorSizedWord isw(domain_str_value, *process_sp);
+  TypeSystemClangSP scratch_ts_sp =
+      ScratchTypeSystemClang::GetForTarget(process_sp->GetTarget());
 
+  if (!scratch_ts_sp)
+    return false;
   ValueObjectSP domain_str_sp = ValueObject::CreateValueObjectFromData(
       "domain_str", isw.GetAsData(process_sp->GetByteOrder()),
       valobj.GetExecutionContextRef(),
-      ScratchTypeSystemClang::GetForTarget(process_sp->GetTarget())
-          ->GetBasicType(lldb::eBasicTypeVoid)
-          .GetPointerType());
+      scratch_ts_sp->GetBasicType(lldb::eBasicTypeVoid).GetPointerType());
 
   if (!domain_str_sp)
     return false;
@@ -153,11 +155,14 @@ class NSErrorSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
     if (userinfo == LLDB_INVALID_ADDRESS || error.Fail())
       return false;
     InferiorSizedWord isw(userinfo, *process_sp);
+    TypeSystemClangSP scratch_ts_sp =
+        ScratchTypeSystemClang::GetForTarget(process_sp->GetTarget());
+    if (!scratch_ts_sp)
+      return false;
     m_child_sp = CreateValueObjectFromData(
         "_userInfo", isw.GetAsData(process_sp->GetByteOrder()),
         m_backend.GetExecutionContextRef(),
-        ScratchTypeSystemClang::GetForTarget(process_sp->GetTarget())
-            ->GetBasicType(lldb::eBasicTypeObjCID));
+        scratch_ts_sp->GetBasicType(lldb::eBasicTypeObjCID));
     return false;
   }
 

diff --git a/lldb/source/Plugins/Language/ObjC/NSException.cpp b/lldb/source/Plugins/Language/ObjC/NSException.cpp
@@ -69,13 +69,13 @@ static bool ExtractFields(ValueObject &valobj, ValueObjectSP *name_sp,
   InferiorSizedWord userinfo_isw(userinfo, *process_sp);
   InferiorSizedWord reserved_isw(reserved, *process_sp);
 
-  auto *clang_ast_context =
+  TypeSystemClangSP scratch_ts_sp =
       ScratchTypeSystemClang::GetForTarget(process_sp->GetTarget());
-  if (!clang_ast_context)
+  if (!scratch_ts_sp)
     return false;
 
   CompilerType voidstar =
-      clang_ast_context->GetBasicType(lldb::eBasicTypeVoid).GetPointerType();
+      scratch_ts_sp->GetBasicType(lldb::eBasicTypeVoid).GetPointerType();
 
   if (name_sp)
     *name_sp = ValueObject::CreateValueObjectFromData(

diff --git a/lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp b/lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
@@ -53,7 +53,7 @@ class NSIndexPathSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
     if (!type_system)
       return false;
 
-    TypeSystemClang *ast = ScratchTypeSystemClang::GetForTarget(
+    auto ast = ScratchTypeSystemClang::GetForTarget(
         *m_backend.GetExecutionContextRef().GetTargetSP());
     if (!ast)
       return false;