Skip to content

Commit 349d906

Browse files
[lldb] Make StackID comparison symmetric
Currently, if we're comparing a StackID on the stack with one StackID on the heap, we perform a special kind of IsYounger comparison. However, if `IsYounger(A, b)` returns true, it must be the case that `IsYounger(B, A)` returns false. But this is not necessarily the case today because, before this patch, we would fallback to the traditional "stack" comparison, which is wrong if one of the StackIDs is on the heap.
1 parent 57757e9 commit 349d906

File tree

2 files changed

+7
-9
lines changed

2 files changed

+7
-9
lines changed

lldb/source/Target/StackID.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,14 @@ IsYoungerHeapCFAs(const StackID &lhs, const StackID &rhs, Process &process) {
8686
return HeapCFAComparisonResult::NoOpinion;
8787

8888
// FIXME: rdar://76119439
89-
// At the boundary between an async parent frame calling a regular child
90-
// frame, the CFA of the parent async function is a heap addresses, and the
91-
// CFA of concrete child function is a stack address. Therefore, if lhs is
92-
// on stack, and rhs is not, lhs is considered less than rhs, independent of
93-
// address values.
89+
// If one of the frames has a CFA on the stack and the other doesn't, we are
90+
// at the boundary between an asynchronous and a synchronous function.
91+
// Synchronous functions cannot call asynchronous functions, therefore the
92+
// synchronous frame is always younger.
9493
if (lhs_cfa_on_stack && !rhs_cfa_on_stack)
9594
return HeapCFAComparisonResult::Younger;
95+
if (!lhs_cfa_on_stack && rhs_cfa_on_stack)
96+
return HeapCFAComparisonResult::Older;
9697
return HeapCFAComparisonResult::NoOpinion;
9798
}
9899
// END SWIFT

lldb/unittests/StackID/StackIDTest.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,5 @@ TEST_F(StackIDTest, StackHeapCFAComparison) {
8686
MockStackID cfa_on_heap(/*cfa*/ 10, OnStack::No);
8787

8888
EXPECT_TRUE(StackID::IsYounger(cfa_on_stack, cfa_on_heap, process));
89-
90-
// FIXME: if the above returned true, swapping the arguments **must** return
91-
// false. And yet it doesn't.
92-
// EXPECT_FALSE(StackID::IsYounger(cfa_on_heap, cfa_on_stack, process));
89+
EXPECT_FALSE(StackID::IsYounger(cfa_on_heap, cfa_on_stack, process));
9390
}

0 commit comments

Comments
 (0)