[Lldb-commits] [PATCH] D141318: [lldb] Store shared pointers in DieToTypePtr map instead of raw pointers

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 added a comment.

In D141318#4045372 <https://reviews.llvm.org/D141318#4045372>, @clayborg wrote:

> Each SymbolFile has its own type list that keeps the shared pointers to all types. So there should be no need for any of these changes here unless someone isn't correctly storing a newly created type in the SymbolFile's type list. You can see the types being stored in code like:
>
>   TypeSP type_sp(... /* create type here*/ ...);
>   dwarf->GetTypeList().Insert(type_sp); // Store the share reference in the symbol file's type list 

What triggered the use after free for me was this snippet in `DWARFASTParserClang::ParseTypeModifier`:

    ...
    TypeSP lldb_function_type_sp = ParseTypeFromDWARF(
        sc, function_type, &function_type_is_new_pointer);
  
    if (lldb_function_type_sp) {
      clang_type = m_ast.CreateBlockPointerType(
          lldb_function_type_sp->GetForwardCompilerType());
      encoding_data_type = Type::eEncodingIsUID;
      attrs.type.Clear();
      resolve_state = Type::ResolveState::Full;
    }
  }

`lldb_function_type_sp` is created, the underlying pointer is placed in the map, the SP is destroyed at the end of the scope, and any lookups for that type will get back the dangling pointer.

I could update this particular instance, but this just seems like a really easy mistake to make... There's nothing to suggest to callers that they must update the `m_type_list` when they ask for a new type. Wouldn't it be better we prevent this somehow, by updating the raw pointers of the map by being either unique, shared or weak pointers?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141318/new/

https://reviews.llvm.org/D141318