https://github.com/labath created
https://github.com/llvm/llvm-project/pull/92503
…unction (#91321)"
This reapplies fd1bd53ba5a06f344698a55578f6a5d79c457e30, 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.
>From 47858edddfbccf989213d61f3761fcdb04814839 Mon Sep 17 00:00:00 2001
From: Pavel Labath <pa...@labath.sk>
Date: Fri, 17 May 2024 06:34:08 +0000
Subject: [PATCH] Reapply "[lldb/aarch64] Fix unwinding when signal interrupts
a leaf function (#91321)"
This reapplies fd1bd53ba5a06f344698a55578f6a5d79c457e30, 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.
---
.../ARM64/EmulateInstructionARM64.cpp | 2 ++
.../UnwindAssemblyInstEmulation.cpp | 4 +--
lldb/source/Target/RegisterContextUnwind.cpp | 6 ++---
.../API/functionalities/bt-interrupt/main.c | 1 +
.../Inputs/signal-in-leaf-function-aarch64.c | 15 +++++++++++
.../signal-in-leaf-function-aarch64.test | 27 +++++++++++++++++++
.../ARM64/TestArm64InstEmulation.cpp | 24 ++++++++++++++---
7 files changed, 69 insertions(+), 10 deletions(-)
create mode 100644
lldb/test/Shell/Unwind/Inputs/signal-in-leaf-function-aarch64.c
create mode 100644 lldb/test/Shell/Unwind/signal-in-leaf-function-aarch64.test
diff --git a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
index 6ca4fb052457e..62ecac3e0831d 100644
--- a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
+++ b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
@@ -444,6 +444,8 @@ bool EmulateInstructionARM64::CreateFunctionEntryUnwind(
// Our previous Call Frame Address is the stack pointer
row->GetCFAValue().SetIsRegisterPlusOffset(gpr_sp_arm64, 0);
+ row->SetRegisterLocationToSame(gpr_lr_arm64, /*must_replace=*/false);
+ row->SetRegisterLocationToSame(gpr_fp_arm64, /*must_replace=*/false);
unwind_plan.AppendRow(row);
unwind_plan.SetSourceName("EmulateInstructionARM64");
diff --git
a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
index c4a171ec7d01b..49edd40544e32 100644
---
a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
+++
b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
@@ -424,8 +424,6 @@ size_t UnwindAssemblyInstEmulation::WriteMemory(
log->PutString(strm.GetString());
}
- const bool cant_replace = false;
-
switch (context.type) {
default:
case EmulateInstruction::eContextInvalid:
@@ -467,7 +465,7 @@ size_t UnwindAssemblyInstEmulation::WriteMemory(
m_pushed_regs[reg_num] = addr;
const int32_t offset = addr - m_initial_sp;
m_curr_row->SetRegisterLocationToAtCFAPlusOffset(reg_num, offset,
- cant_replace);
+ /*can_replace=*/true);
m_curr_row_modified = true;
}
}
diff --git a/lldb/source/Target/RegisterContextUnwind.cpp
b/lldb/source/Target/RegisterContextUnwind.cpp
index 13e101413a477..e2d712cb72eae 100644
--- a/lldb/source/Target/RegisterContextUnwind.cpp
+++ b/lldb/source/Target/RegisterContextUnwind.cpp
@@ -1555,12 +1555,12 @@ RegisterContextUnwind::SavedLocationForRegister(
}
if (unwindplan_regloc.IsSame()) {
- if (!IsFrameZero() &&
+ if (!m_all_registers_available &&
(regnum.GetAsKind(eRegisterKindGeneric) == LLDB_REGNUM_GENERIC_PC ||
regnum.GetAsKind(eRegisterKindGeneric) == LLDB_REGNUM_GENERIC_RA)) {
UnwindLogMsg("register %s (%d) is marked as 'IsSame' - it is a pc or "
- "return address reg on a non-zero frame -- treat as if we "
- "have no information",
+ "return address reg on a frame which does not have all "
+ "registers available -- treat as if we have no information",
regnum.GetName(), regnum.GetAsKind(eRegisterKindLLDB));
return UnwindLLDB::RegisterSearchResult::eRegisterNotFound;
} else {
diff --git a/lldb/test/API/functionalities/bt-interrupt/main.c
b/lldb/test/API/functionalities/bt-interrupt/main.c
index bdaf423d334ef..14a9eb6ffc856 100644
--- a/lldb/test/API/functionalities/bt-interrupt/main.c
+++ b/lldb/test/API/functionalities/bt-interrupt/main.c
@@ -12,6 +12,7 @@ struct Foo {
int
forgot_termination(int input, struct Foo my_foo) {
+ char frame_increasing_buffer[0x1000]; // To blow the stack sooner.
return forgot_termination(++input, my_foo);
}
diff --git a/lldb/test/Shell/Unwind/Inputs/signal-in-leaf-function-aarch64.c
b/lldb/test/Shell/Unwind/Inputs/signal-in-leaf-function-aarch64.c
new file mode 100644
index 0000000000000..fe020affcad0f
--- /dev/null
+++ b/lldb/test/Shell/Unwind/Inputs/signal-in-leaf-function-aarch64.c
@@ -0,0 +1,15 @@
+#include <signal.h>
+#include <unistd.h>
+
+int __attribute__((naked)) signal_generating_add(int a, int b) {
+ asm("add w0, w1, w0\n\t"
+ "udf #0xdead\n\t"
+ "ret");
+}
+
+void sigill_handler(int signo) { _exit(0); }
+
+int main() {
+ signal(SIGILL, sigill_handler);
+ return signal_generating_add(42, 47);
+}
diff --git a/lldb/test/Shell/Unwind/signal-in-leaf-function-aarch64.test
b/lldb/test/Shell/Unwind/signal-in-leaf-function-aarch64.test
new file mode 100644
index 0000000000000..09f17c174bbfd
--- /dev/null
+++ b/lldb/test/Shell/Unwind/signal-in-leaf-function-aarch64.test
@@ -0,0 +1,27 @@
+# REQUIRES: target-aarch64 && native
+# UNSUPPORTED: system-windows
+
+# RUN: %clang_host %S/Inputs/signal-in-leaf-function-aarch64.c -o %t
+# RUN: %lldb -s %s -o exit %t | FileCheck %s
+
+# Convert EXC_BAD_INSTRUCTION to SIGILL on darwin
+settings set platform.plugin.darwin.ignored-exceptions EXC_BAD_INSTRUCTION
+
+breakpoint set -n sigill_handler
+# CHECK: Breakpoint 1: where = {{.*}}`sigill_handler
+
+run
+# CHECK: thread #1, {{.*}} stop reason = signal SIGILL
+
+thread backtrace
+# CHECK: frame #0: [[ADD:0x[0-9a-fA-F]*]] {{.*}}`signal_generating_add
+# CHECK: frame #1: [[MAIN:0x[0-9a-fA-F]*]] {{.*}}`main
+
+continue
+# CHECK: thread #1, {{.*}} stop reason = breakpoint 1
+
+thread backtrace
+# CHECK: frame #0: {{.*}}`sigill_handler
+# Unknown number of signal trampoline frames
+# CHECK: frame #{{[0-9]+}}: [[ADD]] {{.*}}`signal_generating_add
+# CHECK: frame #{{[0-9]+}}: [[MAIN]] {{.*}}`main
diff --git a/lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp
b/lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp
index 80abeb8fae9e5..9303d6f5f3c6e 100644
--- a/lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp
+++ b/lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp
@@ -77,7 +77,7 @@ TEST_F(TestArm64InstEmulation, TestSimpleDarwinFunction) {
// UnwindPlan we expect:
- // row[0]: 0: CFA=sp +0 =>
+ // row[0]: 0: CFA=sp +0 => fp= <same> lr= <same>
// row[1]: 4: CFA=sp+16 => fp=[CFA-16] lr=[CFA-8]
// row[2]: 8: CFA=fp+16 => fp=[CFA-16] lr=[CFA-8]
// row[2]: 16: CFA=sp+16 => fp=[CFA-16] lr=[CFA-8]
@@ -88,13 +88,19 @@ TEST_F(TestArm64InstEmulation, TestSimpleDarwinFunction) {
EXPECT_TRUE(engine->GetNonCallSiteUnwindPlanFromAssembly(
sample_range, data, sizeof(data), unwind_plan));
- // CFA=sp +0
+ // CFA=sp +0 => fp= <same> lr= <same>
row_sp = unwind_plan.GetRowForFunctionOffset(0);
EXPECT_EQ(0ull, row_sp->GetOffset());
EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64);
EXPECT_TRUE(row_sp->GetCFAValue().IsRegisterPlusOffset() == true);
EXPECT_EQ(0, row_sp->GetCFAValue().GetOffset());
+ EXPECT_TRUE(row_sp->GetRegisterInfo(gpr_fp_arm64, regloc));
+ EXPECT_TRUE(regloc.IsSame());
+
+ EXPECT_TRUE(row_sp->GetRegisterInfo(gpr_lr_arm64, regloc));
+ EXPECT_TRUE(regloc.IsSame());
+
// CFA=sp+16 => fp=[CFA-16] lr=[CFA-8]
row_sp = unwind_plan.GetRowForFunctionOffset(4);
EXPECT_EQ(4ull, row_sp->GetOffset());
@@ -146,6 +152,12 @@ TEST_F(TestArm64InstEmulation, TestSimpleDarwinFunction) {
EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64);
EXPECT_TRUE(row_sp->GetCFAValue().IsRegisterPlusOffset() == true);
EXPECT_EQ(0, row_sp->GetCFAValue().GetOffset());
+
+ EXPECT_TRUE(row_sp->GetRegisterInfo(gpr_fp_arm64, regloc));
+ EXPECT_TRUE(regloc.IsSame());
+
+ EXPECT_TRUE(row_sp->GetRegisterInfo(gpr_lr_arm64, regloc));
+ EXPECT_TRUE(regloc.IsSame());
}
TEST_F(TestArm64InstEmulation, TestMediumDarwinFunction) {
@@ -381,8 +393,12 @@ TEST_F(TestArm64InstEmulation,
TestFramelessThreeEpilogueFunction) {
EXPECT_FALSE(row_sp->GetRegisterInfo(gpr_x26_arm64, regloc));
EXPECT_FALSE(row_sp->GetRegisterInfo(gpr_x27_arm64, regloc));
EXPECT_FALSE(row_sp->GetRegisterInfo(gpr_x28_arm64, regloc));
- EXPECT_FALSE(row_sp->GetRegisterInfo(gpr_fp_arm64, regloc));
- EXPECT_FALSE(row_sp->GetRegisterInfo(gpr_lr_arm64, regloc));
+
+ EXPECT_TRUE(row_sp->GetRegisterInfo(gpr_fp_arm64, regloc));
+ EXPECT_TRUE(regloc.IsSame());
+
+ EXPECT_TRUE(row_sp->GetRegisterInfo(gpr_lr_arm64, regloc));
+ EXPECT_TRUE(regloc.IsSame());
row_sp = unwind_plan.GetRowForFunctionOffset(36);
EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
