[lldb] Support recursive record types in CTF

[JDevlieghere]

Summary

Support recursive record types in CTF. We are now more lazy when creating LLDB types. When encountering a record type (struct or union) we create a forward declaration and only complete it when requested.

[Michael137]

Generally LGTM, just some clarifications questions

lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp

508

Just to clarify the lifetimes. This ctf_record lives on m_ast while the m_compiler_types on the SymbolFile, so we're guaranteed that the ctf_record lives long enough?

524–529

Nit: would it make sense to just add classof methods to CTFType and use the llvm cast facilities?

Feel free to ignore since there's just one instance of such cast afaict

[JDevlieghere]

lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp

508

1. Yes, the ctf_record is owned by the m_ctf_types; whose lifetime is the same as the SymbolFile. We only need the CTF types until they're converted to LLDB types so we can be more memory efficient. I'll tackle that in a follow up.
2. Once we've completed a type, we should remove the compiler type from m_compiler_types. I'll include that in this patch.

524–529

I considered it too, but at this point I don't think it's worth it. If we need other casts that's definitely the way to go.

lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.h

public:
 
   Type *ResolveTypeUID(lldb::user_id_t type_uid) override;
   std::optional<ArrayInfo> GetDynamicArrayInfoForUID(
       lldb::user_id_t type_uid,
       const lldb_private::ExecutionContext *exe_ctx) override {
     return std::nullopt;
   }
 
-  bool CompleteType(CompilerType &compiler_type) override { return false; }
+  bool CompleteType(CompilerType &compiler_type) override;
 
   uint32_t ResolveSymbolContext(const lldb_private::Address &so_addr,
                                 lldb::SymbolContextItem resolve_scope,
                                 lldb_private::SymbolContext &sc) override;
 
   void AddSymbols(Symtab &symtab) override;
 
   void GetTypes(lldb_private::SymbolContextScope *sc_scope,
private:
 
   TypeSystemClang *m_ast;
   lldb::CompUnitSP m_comp_unit_sp;
 
   std::optional<ctf_header_t> m_header;
 
   std::vector<std::unique_ptr<CTFType>> m_ctf_types;
 
+  /// To complete types, we need a way to map (imcomplete) compiler types back
+  /// to parsed CTF types.
+  llvm::DenseMap<lldb::opaque_compiler_type_t, const CTFType *>
+      m_compiler_types;
+
   llvm::DenseMap<lldb::user_id_t, lldb::TypeSP> m_types;
 
   std::vector<lldb::FunctionSP> m_functions;
   std::vector<lldb::VariableSP> m_variables;
 
   static constexpr uint16_t g_ctf_magic = 0xcff1;
   static constexpr uint8_t g_ctf_version = 4;
   static constexpr uint32_t g_ctf_field_treshold = 8192;
 };
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_CTF_SYMBOLFILECTF_H

lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp

SymbolFileCTF::CreateFunction(const CTFFunction &ctf_function) {
   return MakeType(ctf_function.uid, ConstString(ctf_function.name), 0, nullptr,
                   LLDB_INVALID_UID, Type::eEncodingIsUID, decl, func_type,
                   lldb_private::Type::ResolveState::Full);
 }
 
 llvm::Expected<lldb::TypeSP>
 SymbolFileCTF::CreateRecord(const CTFRecord &ctf_record) {
   const clang::TagTypeKind tag_kind = TranslateRecordKind(ctf_record.kind);
-
   CompilerType record_type =
       m_ast->CreateRecordType(nullptr, OptionalClangModuleID(), eAccessPublic,
                               ctf_record.name.data(), tag_kind, eLanguageTypeC);
+  m_compiler_types[record_type.GetOpaqueQualType()] = &ctf_record;
+  Declaration decl;
+  return MakeType(ctf_record.uid, ConstString(ctf_record.name), ctf_record.size,
+                  nullptr, LLDB_INVALID_UID, lldb_private::Type::eEncodingIsUID,
+                  decl, record_type, lldb_private::Type::ResolveState::Forward);
+}
 
-  m_ast->StartTagDeclarationDefinition(record_type);
-  for (const CTFRecord::Field &field : ctf_record.fields) {
-    if (Type *field_type = ResolveTypeUID(field.type)) {
-      const uint32_t field_size = field_type->GetByteSize(nullptr).value_or(0);
-      TypeSystemClang::AddFieldToRecordType(record_type, field.name,
-                                            field_type->GetFullCompilerType(),
-                                            eAccessPublic, field_size);
-    }
-  }
-  m_ast->CompleteTagDeclarationDefinition(record_type);
+bool SymbolFileCTF::CompleteType(CompilerType &compiler_type) {
+  // Check if we have a CTF type for the given incomplete compiler type.
+  auto it = m_compiler_types.find(compiler_type.GetOpaqueQualType());
+  if (it == m_compiler_types.end())
+    return false;
+
+  const CTFType *ctf_type = it->second;
+  assert(ctf_type && "m_compiler_types should only contain valid CTF types");
+
+  // We only support resolving record types.
+  assert(ctf_type->kind == CTFType::Kind::eStruct ||
+         ctf_type->kind == CTFType::Kind::eUnion);
+
+  // Cast to the appropriate CTF type.
+  const CTFRecord *ctf_record = static_cast<const CTFRecord *>(ctf_type);
+
+  // If any of the fields are incomplete, we cannot complete the type.
+  for (const CTFRecord::Field &field : ctf_record->fields) {
+    if (!ResolveTypeUID(field.type)) {
+      LLDB_LOG(GetLog(LLDBLog::Symbols),
+               "Cannot complete type {0} because field {1} is incomplete",
+               ctf_type->uid, field.type);
+      return false;
+    }
+  }
+
+  // Complete the record type.
+  m_ast->StartTagDeclarationDefinition(compiler_type);
+  for (const CTFRecord::Field &field : ctf_record->fields) {
+    Type *field_type = ResolveTypeUID(field.type);
+    assert(field_type && "field must be complete");
+    TypeSystemClang::AddFieldToRecordType(compiler_type, field.name,
+  m_ast->CompleteTagDeclarationDefinition(compiler_type);
 
-  Declaration decl;
-  return MakeType(ctf_record.uid, ConstString(ctf_record.name), ctf_record.size,
-                  nullptr, LLDB_INVALID_UID, lldb_private::Type::eEncodingIsUID,
-                  decl, record_type, lldb_private::Type::ResolveState::Full);
+  return true;
 }
 
 llvm::Expected<lldb::TypeSP>
 SymbolFileCTF::CreateForward(const CTFForward &ctf_forward) {
   CompilerType forward_compiler_type = m_ast->CreateRecordType(
       nullptr, OptionalClangModuleID(), eAccessPublic, ctf_forward.name,
       clang::TTK_Struct, eLanguageTypeC);
   Declaration decl;
lldb_private::Type *SymbolFileCTF::ResolveTypeUID(lldb::user_id_t type_uid) {
 
   if (type_uid == 0 || type_uid > m_ctf_types.size())
     return nullptr;
 
   CTFType *ctf_type = m_ctf_types[type_uid - 1].get();
   if (!ctf_type)
     return nullptr;
 
-  m_types[type_uid] = TypeSP();
   Log *log = GetLog(LLDBLog::Symbols);
 
   llvm::Expected<TypeSP> type_or_error = CreateType(ctf_type);
   if (!type_or_error) {
     LLDB_LOG_ERROR(log, type_or_error.takeError(),
                    "Failed to create type for {1}: {0}", ctf_type->uid);
     return {};
   }

lldb/test/API/macosx/ctf/TestCTF.py

def do_test(self):
         self.expect(
             "type lookup LargeStruct",
             substrs=[
                 "struct LargeStruct",
                 "char buffer[9000]",
                 "int b ",
             ],
         )
->>>>>>> Stashed changes
+
+        self.expect("type lookup RecursiveStruct", substrs=["RecursiveStruct *n;"])

lldb/test/API/macosx/ctf/test.c

typedef struct MyStruct {
   void (*f)(int);
 } MyStructT;
 
 struct LargeStruct {
   char buffer[9000];
   int b;
 };
 
+struct RecursiveStruct {
+  struct RecursiveStruct *n;
+};
+
 MyStructT foo;
 struct ForwardDecl *forward;
 struct LargeStruct bar;
+struct RecursiveStruct ke;
 
 void populate(MyInt i) {
   foo.n.i = i;
   foo.n.s = "foo";
   foo.n.c = 'c';
   foo.n.a[0] = 'a';
   foo.n.a[1] = 'b';
   foo.n.a[2] = 'c';
   foo.n.a[3] = 'd';
   foo.n.e = eOne;
   foo.f = NULL;
   forward = NULL;
   bar.b = i;
+  ke.n = NULL;
 }
 
 int main(int argc, char** argv) {
   populate(argc);
   printf("foo is at address: %p\n", (void*)&foo); // Break here
   return 0;
 }