[Lldb-commits] [lldb] 2fa2734 - [LLDB][NativePDB] Fix the case when S_DEFRANGE_SUBFIELD_REGISTERs are out of order.
Zequan Wu via lldb-commits
lldb-commits at lists.llvm.org
Wed Apr 20 10:33:08 PDT 2022
Author: Zequan Wu
Date: 2022-04-20T10:33:00-07:00
New Revision: 2fa2734690ff6debf2d678dc9f94fa14e0cbd227
URL: https://github.com/llvm/llvm-project/commit/2fa2734690ff6debf2d678dc9f94fa14e0cbd227
DIFF: https://github.com/llvm/llvm-project/commit/2fa2734690ff6debf2d678dc9f94fa14e0cbd227.diff
LOG: [LLDB][NativePDB] Fix the case when S_DEFRANGE_SUBFIELD_REGISTERs are out of order.
Previously, I was assuming that S_DEFRANGE_SUBFIELD_REGISTERs are always in the
increasing order of offset_in_parent until I saw a counter example.
Using `std::map` so that they are sorted by offset_in_parent.
Differential Revision: https://reviews.llvm.org/D124061
Added:
Modified:
lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.h
lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
lldb/test/Shell/SymbolFile/NativePDB/local-variables-registers.s
Removed:
################################################################################
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
index 43ee9e1285b29..3ba0079c96e6a 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
@@ -252,11 +252,12 @@ DWARFExpression lldb_private::npdb::MakeConstantLocationExpression(
}
DWARFExpression lldb_private::npdb::MakeEnregisteredLocationExpressionForClass(
- llvm::ArrayRef<std::pair<RegisterId, uint32_t>> &members_info,
+ std::map<uint64_t, std::pair<RegisterId, uint32_t>> &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<RegisterId, uint32_t> 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/DWARFLocationExpression.h b/lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.h
index 1f948452ad280..1ed6c55fea74b 100644
--- 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 <map>
namespace llvm {
class APSInt;
@@ -41,7 +42,7 @@ DWARFExpression MakeConstantLocationExpression(
llvm::codeview::TypeIndex underlying_ti, llvm::pdb::TpiStream &tpi,
const llvm::APSInt &constant, lldb::ModuleSP module);
DWARFExpression MakeEnregisteredLocationExpressionForClass(
- llvm::ArrayRef<std::pair<llvm::codeview::RegisterId, uint32_t>>
+ std::map<uint64_t, std::pair<llvm::codeview::RegisterId, uint32_t>>
&members_info,
lldb::ModuleSP module);
} // namespace npdb
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
index 1f0ca3f3d5d80..8ef4f21a69694 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
@@ -52,15 +52,18 @@ MakeRangeList(const PdbIndex &index, const LocalVariableAddrRange &range,
namespace {
struct FindMembersSize : public TypeVisitorCallbacks {
FindMembersSize(
- llvm::SmallVectorImpl<std::pair<RegisterId, uint32_t>> &members_info,
+ std::map<uint64_t, std::pair<RegisterId, uint32_t>> &members_info,
TpiStream &tpi)
: members_info(members_info), tpi(tpi) {}
- llvm::SmallVectorImpl<std::pair<RegisterId, uint32_t>> &members_info;
+ std::map<uint64_t, std::pair<RegisterId, uint32_t>> &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<CodeViewError>(cv_error_code::corrupt_record);
return llvm::Error::success();
}
};
@@ -708,10 +711,11 @@ VariableInfo lldb_private::npdb::GetVariableLocationInfo(
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<std::pair<RegisterId, uint32_t>> 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<uint64_t, std::pair<RegisterId, uint32_t>> 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 @@ VariableInfo lldb_private::npdb::GetVariableLocationInfo(
}
}
- 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 @@ VariableInfo lldb_private::npdb::GetVariableLocationInfo(
}
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 @@ VariableInfo lldb_private::npdb::GetVariableLocationInfo(
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
index 35dda0a3c1c64..ad524dd98c46a 100644
--- 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 @@ main: # @main
.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:
More information about the lldb-commits
mailing list