Skip to content

Commit bbd54e0

Browse files
authored
Reapply "[lldb/aarch64] Fix unwinding when signal interrupts a leaf f… (#92503)
…unction (#91321)" This reapplies fd1bd53, which was reverted due to a test failure on aarch64/windows. The failure was caused by a combination of several factors: - clang targeting aarch64-windows (unlike msvc, and unlike clang targeting other aarch64 platforms) defaults to -fomit-frame-pointers - lldb's code for looking up register values for `<same>` unwind rules is recursive - the test binary creates a very long chain of fp-less function frames (it manages to fit about 22k frames before it blows its stack) Together, these things have caused lldb to recreate the same deep recursion when unwinding through this, and blow its own stack as well. Since lldb frames are larger, about 4k frames like this was sufficient to trigger the stack overflow. This version of the patch works around this problem by increasing the frame size of the test binary, thereby causing it to blow its stack sooner. This doesn't fix the issue -- the same problem can occur with a real binary -- but it's not very likely, as it requires an infinite recursion in a simple (so it doesn't use the frame pointer) function with a very small frame (so you can fit a lot of them on the stack). A more principled fix would be to make lldb's lookup code non-recursive, but I believe that's out of scope for this patch. The original patch description follows: A leaf function may not store the link register to stack, but we it can still end up being a non-zero frame if it gets interrupted by a signal. Currently, we were unable to unwind past this function because we could not read the link register value. To make this work, this patch: - changes the function-entry unwind plan to include the `fp|lr = <same>` rules. This in turn necessitated an adjustment in the generic instruction emulation logic to ensure that `lr=[sp-X]` can override the `<same>` rule. - allows the `<same>` rule for pc and lr in all `m_all_registers_available` frames (and not just frame zero). The test verifies that we can unwind in a situation like this, and that the backtrace matches the one we computed before getting a signal.
1 parent ee76f1e commit bbd54e0

File tree

7 files changed

+72
-10
lines changed

7 files changed

+72
-10
lines changed

lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,8 @@ bool EmulateInstructionARM64::CreateFunctionEntryUnwind(
444444

445445
// Our previous Call Frame Address is the stack pointer
446446
row->GetCFAValue().SetIsRegisterPlusOffset(gpr_sp_arm64, 0);
447+
row->SetRegisterLocationToSame(gpr_lr_arm64, /*must_replace=*/false);
448+
row->SetRegisterLocationToSame(gpr_fp_arm64, /*must_replace=*/false);
447449

448450
unwind_plan.AppendRow(row);
449451
unwind_plan.SetSourceName("EmulateInstructionARM64");

lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -424,8 +424,6 @@ size_t UnwindAssemblyInstEmulation::WriteMemory(
424424
log->PutString(strm.GetString());
425425
}
426426

427-
const bool cant_replace = false;
428-
429427
switch (context.type) {
430428
default:
431429
case EmulateInstruction::eContextInvalid:
@@ -467,7 +465,7 @@ size_t UnwindAssemblyInstEmulation::WriteMemory(
467465
m_pushed_regs[reg_num] = addr;
468466
const int32_t offset = addr - m_initial_sp;
469467
m_curr_row->SetRegisterLocationToAtCFAPlusOffset(reg_num, offset,
470-
cant_replace);
468+
/*can_replace=*/true);
471469
m_curr_row_modified = true;
472470
}
473471
}

lldb/source/Target/RegisterContextUnwind.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1555,12 +1555,12 @@ RegisterContextUnwind::SavedLocationForRegister(
15551555
}
15561556

15571557
if (unwindplan_regloc.IsSame()) {
1558-
if (!IsFrameZero() &&
1558+
if (!m_all_registers_available &&
15591559
(regnum.GetAsKind(eRegisterKindGeneric) == LLDB_REGNUM_GENERIC_PC ||
15601560
regnum.GetAsKind(eRegisterKindGeneric) == LLDB_REGNUM_GENERIC_RA)) {
15611561
UnwindLogMsg("register %s (%d) is marked as 'IsSame' - it is a pc or "
1562-
"return address reg on a non-zero frame -- treat as if we "
1563-
"have no information",
1562+
"return address reg on a frame which does not have all "
1563+
"registers available -- treat as if we have no information",
15641564
regnum.GetName(), regnum.GetAsKind(eRegisterKindLLDB));
15651565
return UnwindLLDB::RegisterSearchResult::eRegisterNotFound;
15661566
} else {

lldb/test/API/functionalities/bt-interrupt/main.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ struct Foo {
1212

1313
int
1414
forgot_termination(int input, struct Foo my_foo) {
15+
char frame_increasing_buffer[0x1000]; // To blow the stack sooner.
1516
return forgot_termination(++input, my_foo);
1617
}
1718

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#include <signal.h>
2+
#include <unistd.h>
3+
4+
int __attribute__((naked)) signal_generating_add(int a, int b) {
5+
asm("add w0, w1, w0\n\t"
6+
"udf #0xdead\n\t"
7+
"ret");
8+
}
9+
10+
void sigill_handler(int signo) { _exit(0); }
11+
12+
int main() {
13+
signal(SIGILL, sigill_handler);
14+
return signal_generating_add(42, 47);
15+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# REQUIRES: target-aarch64 && native
2+
# UNSUPPORTED: system-windows
3+
# llvm.org/pr91610, rdar://128031075
4+
# XFAIL: system-darwin
5+
6+
7+
# RUN: %clang_host %S/Inputs/signal-in-leaf-function-aarch64.c -o %t
8+
# RUN: %lldb -s %s -o exit %t | FileCheck %s
9+
10+
# Convert EXC_BAD_INSTRUCTION to SIGILL on darwin
11+
settings set platform.plugin.darwin.ignored-exceptions EXC_BAD_INSTRUCTION
12+
13+
breakpoint set -n sigill_handler
14+
# CHECK: Breakpoint 1: where = {{.*}}`sigill_handler
15+
16+
run
17+
# CHECK: thread #1, {{.*}} stop reason = signal SIGILL
18+
19+
thread backtrace
20+
# CHECK: frame #0: [[ADD:0x[0-9a-fA-F]*]] {{.*}}`signal_generating_add
21+
# CHECK: frame #1: [[MAIN:0x[0-9a-fA-F]*]] {{.*}}`main
22+
23+
continue
24+
# CHECK: thread #1, {{.*}} stop reason = breakpoint 1
25+
26+
thread backtrace
27+
# CHECK: frame #0: {{.*}}`sigill_handler
28+
# Unknown number of signal trampoline frames
29+
# CHECK: frame #{{[0-9]+}}: [[ADD]] {{.*}}`signal_generating_add
30+
# CHECK: frame #{{[0-9]+}}: [[MAIN]] {{.*}}`main

lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ TEST_F(TestArm64InstEmulation, TestSimpleDarwinFunction) {
7777

7878
// UnwindPlan we expect:
7979

80-
// row[0]: 0: CFA=sp +0 =>
80+
// row[0]: 0: CFA=sp +0 => fp= <same> lr= <same>
8181
// row[1]: 4: CFA=sp+16 => fp=[CFA-16] lr=[CFA-8]
8282
// row[2]: 8: CFA=fp+16 => fp=[CFA-16] lr=[CFA-8]
8383
// row[2]: 16: CFA=sp+16 => fp=[CFA-16] lr=[CFA-8]
@@ -88,13 +88,19 @@ TEST_F(TestArm64InstEmulation, TestSimpleDarwinFunction) {
8888
EXPECT_TRUE(engine->GetNonCallSiteUnwindPlanFromAssembly(
8989
sample_range, data, sizeof(data), unwind_plan));
9090

91-
// CFA=sp +0
91+
// CFA=sp +0 => fp= <same> lr= <same>
9292
row_sp = unwind_plan.GetRowForFunctionOffset(0);
9393
EXPECT_EQ(0ull, row_sp->GetOffset());
9494
EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64);
9595
EXPECT_TRUE(row_sp->GetCFAValue().IsRegisterPlusOffset() == true);
9696
EXPECT_EQ(0, row_sp->GetCFAValue().GetOffset());
9797

98+
EXPECT_TRUE(row_sp->GetRegisterInfo(gpr_fp_arm64, regloc));
99+
EXPECT_TRUE(regloc.IsSame());
100+
101+
EXPECT_TRUE(row_sp->GetRegisterInfo(gpr_lr_arm64, regloc));
102+
EXPECT_TRUE(regloc.IsSame());
103+
98104
// CFA=sp+16 => fp=[CFA-16] lr=[CFA-8]
99105
row_sp = unwind_plan.GetRowForFunctionOffset(4);
100106
EXPECT_EQ(4ull, row_sp->GetOffset());
@@ -146,6 +152,12 @@ TEST_F(TestArm64InstEmulation, TestSimpleDarwinFunction) {
146152
EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64);
147153
EXPECT_TRUE(row_sp->GetCFAValue().IsRegisterPlusOffset() == true);
148154
EXPECT_EQ(0, row_sp->GetCFAValue().GetOffset());
155+
156+
EXPECT_TRUE(row_sp->GetRegisterInfo(gpr_fp_arm64, regloc));
157+
EXPECT_TRUE(regloc.IsSame());
158+
159+
EXPECT_TRUE(row_sp->GetRegisterInfo(gpr_lr_arm64, regloc));
160+
EXPECT_TRUE(regloc.IsSame());
149161
}
150162

151163
TEST_F(TestArm64InstEmulation, TestMediumDarwinFunction) {
@@ -381,8 +393,12 @@ TEST_F(TestArm64InstEmulation, TestFramelessThreeEpilogueFunction) {
381393
EXPECT_FALSE(row_sp->GetRegisterInfo(gpr_x26_arm64, regloc));
382394
EXPECT_FALSE(row_sp->GetRegisterInfo(gpr_x27_arm64, regloc));
383395
EXPECT_FALSE(row_sp->GetRegisterInfo(gpr_x28_arm64, regloc));
384-
EXPECT_FALSE(row_sp->GetRegisterInfo(gpr_fp_arm64, regloc));
385-
EXPECT_FALSE(row_sp->GetRegisterInfo(gpr_lr_arm64, regloc));
396+
397+
EXPECT_TRUE(row_sp->GetRegisterInfo(gpr_fp_arm64, regloc));
398+
EXPECT_TRUE(regloc.IsSame());
399+
400+
EXPECT_TRUE(row_sp->GetRegisterInfo(gpr_lr_arm64, regloc));
401+
EXPECT_TRUE(regloc.IsSame());
386402

387403
row_sp = unwind_plan.GetRowForFunctionOffset(36);
388404
EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64);

0 commit comments

Comments
 (0)