Skip to content

Commit 6423afe

Browse files
yoneymeta-codesync[bot]
authored andcommitted
Decouple self stack-walk tests from GoogleTest frame pointers
Summary: The OSS build enables frame pointers for stack_walk_test.cpp, but walkSelf() can inline into TestBody(), making its first caller GoogleTest code compiled without frame pointers. Add fixture-owned, non-inlined frames around self-walks so the tests reliably exercise the intended behavior. Reviewed By: DinoV Differential Revision: D117194593 fbshipit-source-id: 8c01052242a112d96ab764b748def18658908b20
1 parent 1e7d273 commit 6423afe

1 file changed

Lines changed: 38 additions & 10 deletions

File tree

cinderx/RuntimeTests/stack_walk_test.cpp

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,24 @@ PyThreadState fakeThreadState(StackWalk::ThreadId thread) {
199199
return tstate;
200200
}
201201

202+
// Keeps self-walks below enough frames built as part of this file to prove the
203+
// walker follows a real chain cheaply. In OSS builds, the GoogleTest caller
204+
// above TestBody may not have frame pointers of its own.
205+
constexpr size_t kSelfWalkFixtureDepth = 8;
206+
207+
template <typename F>
208+
__attribute__((noinline)) WalkResult
209+
walkBelowKnownFrames(size_t remaining, F&& walk) {
210+
if (remaining == 0) {
211+
return std::forward<F>(walk)();
212+
}
213+
WalkResult result =
214+
walkBelowKnownFrames(remaining - 1, std::forward<F>(walk));
215+
// Stops this being a tail call, which would collapse the fixture chain.
216+
asm volatile("");
217+
return result;
218+
}
219+
202220
// The frames a walk of `stack` should produce: the innermost record paired with
203221
// the interrupted PC, then every caller paired with the address its callee
204222
// returns to.
@@ -606,9 +624,11 @@ TEST(StackWalkThreadTest, WalkOfOwnThreadStateWalksSelfRatherThanFailing) {
606624
PyThreadState tstate = fakeThreadState(::pthread_self());
607625

608626
size_t frames = 0;
609-
const WalkResult walked = sw.walk(&tstate, [&](const void*, const void*) {
610-
frames++;
611-
return true;
627+
const WalkResult walked = walkBelowKnownFrames(kSelfWalkFixtureDepth, [&] {
628+
return sw.walk(&tstate, [&](const void*, const void*) {
629+
frames++;
630+
return true;
631+
});
612632
});
613633

614634
EXPECT_EQ(walked, WalkResult::Completed);
@@ -668,17 +688,21 @@ TEST(StackWalkThreadTest, WalkingOurOwnStackCostsAFewSafeReadsNotOnePerFrame) {
668688
const uint64_t before = StackWalk::safeReadCount();
669689
size_t frames = 0;
670690
ASSERT_EQ(
671-
StackWalk::walkSelf([&](const void*, const void*) {
672-
frames++;
673-
return true;
674-
}),
691+
walkBelowKnownFrames(
692+
kSelfWalkFixtureDepth,
693+
[&] {
694+
return StackWalk::walkSelf([&](const void*, const void*) {
695+
frames++;
696+
return true;
697+
});
698+
}),
675699
WalkResult::Completed);
676700
const uint64_t reads = StackWalk::safeReadCount() - before;
677701

678702
EXPECT_LE(reads, 2u) << "walked " << frames << " frames but spent " << reads
679703
<< " safe reads";
680-
EXPECT_GT(frames, reads)
681-
<< "the walk did not find more frames than it read through the fallback";
704+
EXPECT_GT(frames, kSelfWalkFixtureDepth)
705+
<< "the walk did not reach every recursive fixture frame";
682706
}
683707

684708
TEST(StackWalkThreadTest, CallbackReturningFalseStopsACrossThreadWalk) {
@@ -1140,7 +1164,11 @@ TEST(StackWalkSignalDiscoveryTest, NoFreeSignalLeavesOnlySelfWalksWorking) {
11401164
// Walking the calling thread needs no signal at all, so it is unaffected -
11411165
// which is why this is a degraded walker rather than no walker.
11421166
size_t own = 0;
1143-
EXPECT_EQ(walkCounting(sw, ::pthread_self(), own), WalkResult::Completed);
1167+
EXPECT_EQ(
1168+
walkBelowKnownFrames(
1169+
kSelfWalkFixtureDepth,
1170+
[&] { return walkCounting(sw, ::pthread_self(), own); }),
1171+
WalkResult::Completed);
11441172
EXPECT_GT(own, 0u);
11451173
}
11461174

0 commit comments

Comments
 (0)