Skip to content

[lldb/aarch64] Fix unwinding when signal interrupts a leaf function #91321

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 9, 2024
Merged

[lldb/aarch64] Fix unwinding when signal interrupts a leaf function #91321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
4 changes: 1 addition & 3 deletions lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,8 +424,6 @@ size_t UnwindAssemblyInstEmulation::WriteMemory(
log->PutString(strm.GetString());
}

const bool cant_replace = false;

switch (context.type) {
default:
case EmulateInstruction::eContextInvalid:
Expand Down Expand Up @@ -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);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be narrowed down so that it only overwrites the <same> rules, but I'm not sure it's necessary given that lines 464&465 ensure that the register can get pushed only once.

m_curr_row_modified = true;
}
}
Expand Down
6 changes: 3 additions & 3 deletions lldb/source/Target/RegisterContextUnwind.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1555,12 +1555,12 @@ RegisterContextUnwind::SavedLocationForRegister(
}

if (unwindplan_regloc.IsSame()) {
if (!IsFrameZero() &&
if (!m_all_registers_available &&
(regnum.GetAsKind(eRegisterKindGeneric) == LLDB_REGNUM_GENERIC_PC ||
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pc=<same> is probably still only valid for real frame zero, so we could make the m_all_registers_available check lr-only.

.. or drop the lr check entirely, since some non-ABI-respecting functions could actually preserve the value of lr even if they are not leaf functions.

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 {
Expand Down
15 changes: 15 additions & 0 deletions lldb/test/Shell/Unwind/Inputs/signal-in-leaf-function-aarch64.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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) { _exit(0); }

int main() {
signal(SIGILL, sigill_handler);
return signal_generating_add(42, 47);
}
24 changes: 24 additions & 0 deletions lldb/test/Shell/Unwind/signal-in-leaf-function-aarch64.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# 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

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
24 changes: 20 additions & 4 deletions lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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());
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
Loading