Switch from using member_clang_type.GetByteSize() to member_type->GetByteSize() in ParseSingleMember

[shafik]

Summary

We have a bug in which using member_clang_type.GetByteSize() triggers record layout and during this process since the record was not yet complete we ended up reaching a record that had not been layed out yet. Using member_type->GetByteSize() avoids this situation since it relies on size from DWARF and will not trigger record layout.

We were not able to reduce this to minimal reproducer. The crash goes away in this case when we have a dSYM and we have seen that ordering in symbol processing can change the behavior as well.

For reference: rdar://77293040

[aprantl]

The change itself seems good to me, I wonder what we can do to prevent a similar mistake in the future.

[aprantl]

I guess we could add least add a comment like this?

// By calling member_type instead of member_clang_type, the size comes from DWARF, which avoids premature record layout.

[teemperor]

I think this can just land as an optimisation. If we can read the size from DWARF (e.g., just returning the stored size int) then that's much better than computing the layout for some type.

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

if (tag == DW_TAG_member) {
           }
 
           last_field_info = this_field_info;
           last_field_info.SetIsBitfield(true);
         } else {
           last_field_info.bit_offset = field_bit_offset;
 
           if (llvm::Optional<uint64_t> clang_type_size =
-                  member_clang_type.GetByteSize(nullptr)) {
+                  member_type->GetByteSize(nullptr)) {
             last_field_info.bit_size = *clang_type_size * character_width;
           }
 
           last_field_info.SetIsBitfield(false);
         }
 
         if (!member_clang_type.IsCompleteType())
           member_clang_type.GetCompleteType();