[lldb] Implement SymbolFile::CopyType

[augusto2112]

Summary

SymbolFiles should be the only point of creation of Types to ensure
that they aren't destroyed prematurely by keeping them in the
SymbolFile's TypeList. This patch hides the copy constructor of Types,
and adds a new CopyType function to SymbolFile, so Types can still be
copied safely.

[clayborg]

We don't allow types to be copied into another SymbolFile. There is a strict rule in LLDB that SymbolFile objects only create types from their own data. There are many reasons we don't want this:

• Type objects have a m_symbol_file which points to the SymbolFile that created it. This pointer can and will go bad if the original symbol file is freed.
• Type objects have a symbol context member variable ( "SymbolContextScope *m_context;"), same issue as above
• Type objects might have a valid encoding type member variable ("Type *m_encoding_type;") that is non NULL that points to a type that is owned by the original symbol file
• Type objects might have a encoding User ID and encoding type that point to the original symbol file ( lldb::user_id_t m_encoding_uid = LLDB_INVALID_UID; EncodingDataType m_encoding_uid_type = eEncodingInvalid;) and the user_id_t only makes sense in the original encoding type.

[augusto2112]

@clayborg the intended usage here is to create a copy of the type in the same symbol file. I could add a sanity check that makes sure we're not creating a copy of something that isn't in the symbol file's type list in debug mode. Would that be enough?

[clayborg]

In D142052#4067177, @augusto2112 wrote:

@clayborg the intended usage here is to create a copy of the type in the same symbol file. I could add a sanity check that makes sure we're not creating a copy of something that isn't in the symbol file's type list in debug mode. Would that be enough?

When would you need to copy a type that is already in SymbolFile::m_type_list?

[augusto2112]

@clayborg we have one instance downstream where we need to keep two types around. One for the original type and one for a slightly modified one (here). This patch is just a way to make sure any new type we copy is inserted in the type list as well.

[clayborg]

Gotcha, makes sense now. Thanks for the extra info. We do need to make sure the m_symbol_file values match and return an empty TypeSP if they don't. Once that is done, this is good to go.

lldb/include/lldb/Symbol/SymbolFile.h

527

We have to check if other_type has the same symbol file and return nothing if it doesn't match.

[augusto2112]

Added check for same symbol file

lldb/include/lldb/Symbol/SymbolFile.h

public:
   MakeType(lldb::user_id_t uid, ConstString name,
            std::optional<uint64_t> byte_size, SymbolContextScope *context,
            lldb::user_id_t encoding_uid,
            Type::EncodingDataType encoding_uid_type, const Declaration &decl,
            const CompilerType &compiler_qual_type,
            Type::ResolveState compiler_type_resolve_state,
            uint32_t opaque_payload = 0) = 0;
 
+  virtual lldb::TypeSP CopyType(const lldb::TypeSP &other_type) = 0;
+
 protected:
   void AssertModuleLock();
 
 private:
   SymbolFile(const SymbolFile &) = delete;
   const SymbolFile &operator=(const SymbolFile &) = delete;
 };
 
lldb::TypeSP MakeType(lldb::user_id_t uid, ConstString name,
      lldb::TypeSP type_sp (new Type(
          uid, this, name, byte_size, context, encoding_uid,
          encoding_uid_type, decl, compiler_qual_type,
          compiler_type_resolve_state, opaque_payload));
      m_type_list.Insert(type_sp);
      return type_sp;
   }
 
+  lldb::TypeSP CopyType(const lldb::TypeSP &other_type) override {
+     // Make sure the real symbol file matches when copying types.
+     if (GetBackingSymbolFile() != other_type->GetSymbolFile())
+      return lldb::TypeSP();
+     lldb::TypeSP type_sp(new Type(*other_type));
+     m_type_list.Insert(type_sp);
+     return type_sp;
+  }
+
 protected:
   virtual uint32_t CalculateNumCompileUnits() = 0;
   virtual lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t idx) = 0;
   virtual TypeList &GetTypeList() { return m_type_list; }
   void SetCompileUnitAtIndex(uint32_t idx, const lldb::CompUnitSP &cu_sp);
 
   lldb::ObjectFileSP m_objfile_sp; // Keep a reference to the object file in
                                    // case it isn't the same as the module

lldb/include/lldb/Symbol/SymbolFileOnDemand.h

lldb::TypeSP MakeType(lldb::user_id_t uid, ConstString name,
                         const CompilerType &compiler_qual_type,
                         Type::ResolveState compiler_type_resolve_state,
                         uint32_t opaque_payload = 0) override {
     return m_sym_file_impl->MakeType(
         uid, name, byte_size, context, encoding_uid, encoding_uid_type, decl,
         compiler_qual_type, compiler_type_resolve_state, opaque_payload);
   }
 
+  lldb::TypeSP CopyType(const lldb::TypeSP &other_type) override {
+    return m_sym_file_impl->CopyType(other_type);
+  }
+
 private:
   Log *GetLog() const { return ::lldb_private::GetLog(LLDBLog::OnDemand); }
 
   ConstString GetSymbolFileName() {
     return GetObjectFile()->GetFileSpec().GetFilename();
   }
 
 private:
   bool m_debug_info_enabled = false;
   bool m_preload_symbols = false;
   std::unique_ptr<SymbolFile> m_sym_file_impl;
 };
 } // namespace lldb_private
 
 #endif // LLDB_SYMBOL_SYMBOLFILEONDEMAND_H

lldb/include/lldb/Symbol/Type.h

Type(lldb::user_id_t uid, SymbolFile *symbol_file, ConstString name,
        std::optional<uint64_t> byte_size, SymbolContextScope *context,
        lldb::user_id_t encoding_uid, EncodingDataType encoding_uid_type,
        const Declaration &decl, const CompilerType &compiler_qual_type,
        ResolveState compiler_type_resolve_state, uint32_t opaque_payload = 0);
 
   // This makes an invalid type.  Used for functions that return a Type when
   // they get an error.
   Type();
+
+  Type(Type &t) = default;
+
+  Type(Type &&t) = default;
+
+  Type &operator=(const Type &t) = default;
+
+  Type &operator=(Type &&t) = default;
 };
 
 // the two classes here are used by the public API as a backend to the SBType
 // and SBTypeList classes
 
 class TypeImpl {
 public:
   TypeImpl() = default;