Skip to content

Commit 1b3b686

Browse files
[lldb] Fix HeapCFA comparison to be symmetric
A swift async frame A is younger than frame B if the async ctx of B can be reached from the async ctx of A. This is implemented today. But we fail to check the opposite: A swift async frame A is _older_ than frame B if the async ctx of A can be reached from the async ctx of B. Because we lacked the opposite check, we were falling back to "vanilla" CFA comparison, and were just being lucky that the vast majority of cases are only interested in the "Younger" case.
1 parent 34622b4 commit 1b3b686

File tree

2 files changed

+32
-0
lines changed

2 files changed

+32
-0
lines changed

lldb/source/Target/StackID.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,11 @@ CompareHeapCFAs(const StackID &lhs, const StackID &rhs, Process &process) {
139139
LLDB_LOG_ERROR(GetLog(LLDBLog::Unwind), std::move(E), "{0}");
140140
else if (*lhs_younger)
141141
return HeapCFAComparisonResult::Younger;
142+
llvm::Expected<bool> lhs_older = IsReachableParent(rhs_cfa, lhs_cfa, process);
143+
if (auto E = lhs_older.takeError())
144+
LLDB_LOG_ERROR(GetLog(LLDBLog::Unwind), std::move(E), "{0}");
145+
else if (*lhs_older)
146+
return HeapCFAComparisonResult::Older;
142147
return HeapCFAComparisonResult::NoOpinion;
143148
}
144149
// END SWIFT

lldb/unittests/StackID/StackIDTest.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,3 +123,30 @@ TEST_F(StackIDTest, HeapHeapCFAComparison) {
123123
EXPECT_TRUE(StackID::IsYounger(middle_cfa, oldest_cfa, process));
124124
EXPECT_FALSE(StackID::IsYounger(oldest_cfa, middle_cfa, process));
125125
}
126+
127+
TEST_F(StackIDTest, HeapHeapCFAComparisonDecreasing) {
128+
// Create a mock async continuation chain:
129+
// 100 -> 90 -> 80 -> 0
130+
// This should be read as:
131+
// "Async context whose address is 100 has a continuation context whose
132+
// address is 90", etc.
133+
llvm::DenseMap<addr_t, addr_t> memory_map;
134+
memory_map[100] = 90;
135+
memory_map[90] = 80;
136+
memory_map[80] = 0;
137+
auto process = MockProcess(m_target_sp, Listener::MakeListener("dummy"),
138+
std::move(memory_map));
139+
140+
MockStackID oldest_cfa(/*cfa*/ 80, OnStack::No);
141+
MockStackID middle_cfa(/*cfa*/ 90, OnStack::No);
142+
MockStackID youngest_cfa(/*cfa*/ 100, OnStack::No);
143+
144+
EXPECT_TRUE(StackID::IsYounger(youngest_cfa, oldest_cfa, process));
145+
EXPECT_FALSE(StackID::IsYounger(oldest_cfa, youngest_cfa, process));
146+
147+
EXPECT_TRUE(StackID::IsYounger(youngest_cfa, middle_cfa, process));
148+
EXPECT_FALSE(StackID::IsYounger(middle_cfa, youngest_cfa, process));
149+
150+
EXPECT_TRUE(StackID::IsYounger(middle_cfa, oldest_cfa, process));
151+
EXPECT_FALSE(StackID::IsYounger(oldest_cfa, middle_cfa, process));
152+
}

0 commit comments

Comments
 (0)