diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.h b/lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.h --- a/lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.h +++ b/lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.h @@ -12,6 +12,7 @@ #include "lldb/lldb-forward.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/DebugInfo/CodeView/CodeView.h" +#include namespace llvm { class APSInt; @@ -41,7 +42,7 @@ llvm::codeview::TypeIndex underlying_ti, llvm::pdb::TpiStream &tpi, const llvm::APSInt &constant, lldb::ModuleSP module); DWARFExpression MakeEnregisteredLocationExpressionForClass( - llvm::ArrayRef> + std::map> &members_info, lldb::ModuleSP module); } // namespace npdb diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp --- a/lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp +++ b/lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp @@ -252,11 +252,12 @@ } DWARFExpression lldb_private::npdb::MakeEnregisteredLocationExpressionForClass( - llvm::ArrayRef> &members_info, + std::map> &members_info, lldb::ModuleSP module) { return MakeLocationExpressionInternal( module, [&](Stream &stream, RegisterKind ®ister_kind) -> bool { - for (auto member_info : members_info) { + for (auto pair : members_info) { + std::pair member_info = pair.second; if (member_info.first != llvm::codeview::RegisterId::NONE) { uint32_t reg_num = GetRegisterNumber(module->GetArchitecture().GetMachine(), diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp --- a/lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp +++ b/lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp @@ -52,15 +52,18 @@ namespace { struct FindMembersSize : public TypeVisitorCallbacks { FindMembersSize( - llvm::SmallVectorImpl> &members_info, + std::map> &members_info, TpiStream &tpi) : members_info(members_info), tpi(tpi) {} - llvm::SmallVectorImpl> &members_info; + std::map> &members_info; TpiStream &tpi; llvm::Error visitKnownMember(CVMemberRecord &cvr, DataMemberRecord &member) override { - members_info.emplace_back(llvm::codeview::RegisterId::NONE, - GetSizeOfType(member.Type, tpi)); + auto it = members_info.insert( + {member.getFieldOffset(), + {llvm::codeview::RegisterId::NONE, GetSizeOfType(member.Type, tpi)}}); + if (!it.second) + return llvm::make_error(cv_error_code::corrupt_record); return llvm::Error::success(); } }; @@ -708,10 +711,11 @@ break; } case S_DEFRANGE_SUBFIELD_REGISTER: { - // A vector of register id and member size pairs. If the variable is a - // simple type, then we don't know the number of subfields. Otherwise, the - // vector size will be the number of subfields in the udt type. - llvm::SmallVector> members_info; + // A map from offset in parent to pair of register id and size. If the + // variable is a simple type, then we don't know the number of subfields. + // Otherwise, the size of the map should be greater than or equal to the + // number of sub field record. + std::map> members_info; bool is_simple_type = result.type.isSimple(); if (!is_simple_type) { CVType class_cvt = index.tpi().getType(result.type); @@ -725,8 +729,6 @@ } } - uint32_t prev_offset = 0; - uint32_t cur_offset = 0; size_t member_idx = 0; // Assuming S_DEFRANGE_SUBFIELD_REGISTER is followed only by // S_DEFRANGE_SUBFIELD_REGISTER, need to verify. @@ -748,26 +750,21 @@ } if (is_simple_type) { - // Fix last member's size. - if (!members_info.empty()) - members_info.back().second = - loc.Hdr.OffsetInParent - prev_offset; - members_info.emplace_back((RegisterId)(uint16_t)loc.Hdr.Register, 0); - prev_offset = loc.Hdr.OffsetInParent; - } else { - // Some fields maybe optimized away and have no - // S_DEFRANGE_SUBFIELD_REGISTER to describe them. Skip them. - while (loc.Hdr.OffsetInParent != cur_offset && - member_idx < members_info.size()) { - cur_offset += members_info[member_idx].second; - ++member_idx; + if (members_info.count(loc.Hdr.OffsetInParent)) { + // Malformed record. + result.ranges->Clear(); + return result; } - if (member_idx < members_info.size()) { - members_info[member_idx].first = - (RegisterId)(uint16_t)loc.Hdr.Register; - cur_offset += members_info[member_idx].second; - ++member_idx; + members_info[loc.Hdr.OffsetInParent] = { + (RegisterId)(uint16_t)loc.Hdr.Register, 0}; + } else { + if (!members_info.count(loc.Hdr.OffsetInParent)) { + // Malformed record. + result.ranges->Clear(); + return result; } + members_info[loc.Hdr.OffsetInParent].first = + (RegisterId)(uint16_t)loc.Hdr.Register; } // Go to next S_DEFRANGE_SUBFIELD_REGISTER. loc_specifier_id = PdbCompilandSymId( @@ -775,12 +772,23 @@ loc_specifier_id.offset + loc_specifier_cvs.RecordData.size()); loc_specifier_cvs = index.ReadSymbolRecord(loc_specifier_id); } - if (is_simple_type && !members_info.empty()) - members_info.back().second = - GetTypeSizeForSimpleKind(result.type.getSimpleKind()) - prev_offset; - auto member_info_ref = llvm::makeArrayRef(members_info); + // Fix size for simple type. + if (is_simple_type) { + auto cur = members_info.begin(); + auto end = members_info.end(); + auto next = cur; + ++next; + uint32_t size = 0; + while (next != end) { + cur->second.second = next->first - cur->first; + size += cur->second.second; + cur = next++; + } + cur->second.second = + GetTypeSizeForSimpleKind(result.type.getSimpleKind()) - size; + } result.location = - MakeEnregisteredLocationExpressionForClass(member_info_ref, module); + MakeEnregisteredLocationExpressionForClass(members_info, module); break; } default: diff --git a/lldb/test/Shell/SymbolFile/NativePDB/local-variables-registers.s b/lldb/test/Shell/SymbolFile/NativePDB/local-variables-registers.s --- a/lldb/test/Shell/SymbolFile/NativePDB/local-variables-registers.s +++ b/lldb/test/Shell/SymbolFile/NativePDB/local-variables-registers.s @@ -12,7 +12,7 @@ # int x; # char y; # }; -# +# # __attribute__((noinline)) S CreateS(int p1, char p2) { # S s; # s.x = p1 + 1; @@ -21,15 +21,15 @@ # ++s.y; # return s; # } -# +# # int main(int argc, char** argv) { # int local = argc * 2; # S s = CreateS(local, 'a'); # return s.x + s.y; # } -# FIXME: The following variable location have wrong register numbers due to -# https://github.com/llvm/llvm-project/issues/53575. Fix them after resolving +# FIXME: The following variable location have wrong register numbers due to +# https://github.com/llvm/llvm-project/issues/53575. Fix them after resolving # the issue. # CHECK: (lldb) image lookup -a 0x140001000 -v @@ -244,8 +244,9 @@ .asciz "s" .p2align 2 .Ltmp24: - .cv_def_range .Ltmp0 .Lfunc_end0, subfield_reg, 18, 0 - .cv_def_range .Ltmp1 .Lfunc_end0, subfield_reg, 3, 4 + # The following .cv_def_range order is inverted on purpose for testing. + .cv_def_range .Ltmp0 .Lfunc_end0, subfield_reg, 3, 4 + .cv_def_range .Ltmp1 .Lfunc_end0, subfield_reg,18, 0 .short 2 # Record length .short 4431 # Record kind: S_PROC_ID_END .Ltmp14: