Use elf offset in signal step check.
The function StepIfSignalHandler assumed that the rel_pc passed
to it was actually an elf offset. A new version of clang created a libc.so
that has a load bias, so tests unwinding through a signal handler
would fail on arm. On other ABIs, there is unwind information that could
be used instead, so the unwind still worked.
The fix is to subtract the load bias from the rel_pc to get an elf
offset to pass to the Register StepIfSignalHandler functions. Change all
of the Register funtions to make it clear what the first parameter means.
Add a unit test for this new code. Also, add an offline test for
this case.
Bug: 145683525
Test: Ran unit tests using the new clang and the old clang.
Change-Id: I3e249653b79bcad6d3a56411a7911fde4888e9d6
diff --git a/libunwindstack/Android.bp b/libunwindstack/Android.bp
index 512c962..9cd3d65 100644
--- a/libunwindstack/Android.bp
+++ b/libunwindstack/Android.bp
@@ -251,6 +251,7 @@
"tests/files/offline/shared_lib_in_apk_arm64/*",
"tests/files/offline/shared_lib_in_apk_memory_only_arm64/*",
"tests/files/offline/shared_lib_in_apk_single_map_arm64/*",
+ "tests/files/offline/signal_load_bias_arm/*",
"tests/files/offline/straddle_arm/*",
"tests/files/offline/straddle_arm64/*",
],
diff --git a/libunwindstack/Elf.cpp b/libunwindstack/Elf.cpp
index 6627787..f01b092 100644
--- a/libunwindstack/Elf.cpp
+++ b/libunwindstack/Elf.cpp
@@ -173,7 +173,12 @@
if (!valid_) {
return false;
}
- return regs->StepIfSignalHandler(rel_pc, this, process_memory);
+
+ // Convert the rel_pc to an elf_offset.
+ if (rel_pc < static_cast<uint64_t>(load_bias_)) {
+ return false;
+ }
+ return regs->StepIfSignalHandler(rel_pc - load_bias_, this, process_memory);
}
// The relative pc is always relative to the start of the map from which it comes.
diff --git a/libunwindstack/RegsArm.cpp b/libunwindstack/RegsArm.cpp
index 885dc94..1b1f7eb 100644
--- a/libunwindstack/RegsArm.cpp
+++ b/libunwindstack/RegsArm.cpp
@@ -127,12 +127,12 @@
return regs;
}
-bool RegsArm::StepIfSignalHandler(uint64_t rel_pc, Elf* elf, Memory* process_memory) {
+bool RegsArm::StepIfSignalHandler(uint64_t elf_offset, Elf* elf, Memory* process_memory) {
uint32_t data;
Memory* elf_memory = elf->memory();
// Read from elf memory since it is usually more expensive to read from
// process memory.
- if (!elf_memory->ReadFully(rel_pc, &data, sizeof(data))) {
+ if (!elf_memory->ReadFully(elf_offset, &data, sizeof(data))) {
return false;
}
diff --git a/libunwindstack/RegsArm64.cpp b/libunwindstack/RegsArm64.cpp
index 2e8af20..1df1dff 100644
--- a/libunwindstack/RegsArm64.cpp
+++ b/libunwindstack/RegsArm64.cpp
@@ -126,12 +126,12 @@
return regs;
}
-bool RegsArm64::StepIfSignalHandler(uint64_t rel_pc, Elf* elf, Memory* process_memory) {
+bool RegsArm64::StepIfSignalHandler(uint64_t elf_offset, Elf* elf, Memory* process_memory) {
uint64_t data;
Memory* elf_memory = elf->memory();
// Read from elf memory since it is usually more expensive to read from
// process memory.
- if (!elf_memory->ReadFully(rel_pc, &data, sizeof(data))) {
+ if (!elf_memory->ReadFully(elf_offset, &data, sizeof(data))) {
return false;
}
diff --git a/libunwindstack/RegsMips.cpp b/libunwindstack/RegsMips.cpp
index f330fe0..ebefe42 100644
--- a/libunwindstack/RegsMips.cpp
+++ b/libunwindstack/RegsMips.cpp
@@ -129,13 +129,13 @@
return regs;
}
-bool RegsMips::StepIfSignalHandler(uint64_t rel_pc, Elf* elf, Memory* process_memory) {
+bool RegsMips::StepIfSignalHandler(uint64_t elf_offset, Elf* elf, Memory* process_memory) {
uint64_t data;
uint64_t offset = 0;
Memory* elf_memory = elf->memory();
// Read from elf memory since it is usually more expensive to read from
// process memory.
- if (!elf_memory->ReadFully(rel_pc, &data, sizeof(data))) {
+ if (!elf_memory->ReadFully(elf_offset, &data, sizeof(data))) {
return false;
}
diff --git a/libunwindstack/RegsMips64.cpp b/libunwindstack/RegsMips64.cpp
index 3f67d92..be2fd22 100644
--- a/libunwindstack/RegsMips64.cpp
+++ b/libunwindstack/RegsMips64.cpp
@@ -127,12 +127,12 @@
return regs;
}
-bool RegsMips64::StepIfSignalHandler(uint64_t rel_pc, Elf* elf, Memory* process_memory) {
+bool RegsMips64::StepIfSignalHandler(uint64_t elf_offset, Elf* elf, Memory* process_memory) {
uint64_t data;
Memory* elf_memory = elf->memory();
// Read from elf memory since it is usually more expensive to read from
// process memory.
- if (!elf_memory->Read(rel_pc, &data, sizeof(data))) {
+ if (!elf_memory->Read(elf_offset, &data, sizeof(data))) {
return false;
}
diff --git a/libunwindstack/RegsX86.cpp b/libunwindstack/RegsX86.cpp
index 9bb39d1..5538fc0 100644
--- a/libunwindstack/RegsX86.cpp
+++ b/libunwindstack/RegsX86.cpp
@@ -119,12 +119,12 @@
return regs;
}
-bool RegsX86::StepIfSignalHandler(uint64_t rel_pc, Elf* elf, Memory* process_memory) {
+bool RegsX86::StepIfSignalHandler(uint64_t elf_offset, Elf* elf, Memory* process_memory) {
uint64_t data;
Memory* elf_memory = elf->memory();
// Read from elf memory since it is usually more expensive to read from
// process memory.
- if (!elf_memory->ReadFully(rel_pc, &data, sizeof(data))) {
+ if (!elf_memory->ReadFully(elf_offset, &data, sizeof(data))) {
return false;
}
diff --git a/libunwindstack/RegsX86_64.cpp b/libunwindstack/RegsX86_64.cpp
index 74cd1cb..5b9aa58 100644
--- a/libunwindstack/RegsX86_64.cpp
+++ b/libunwindstack/RegsX86_64.cpp
@@ -139,17 +139,17 @@
return regs;
}
-bool RegsX86_64::StepIfSignalHandler(uint64_t rel_pc, Elf* elf, Memory* process_memory) {
+bool RegsX86_64::StepIfSignalHandler(uint64_t elf_offset, Elf* elf, Memory* process_memory) {
uint64_t data;
Memory* elf_memory = elf->memory();
// Read from elf memory since it is usually more expensive to read from
// process memory.
- if (!elf_memory->ReadFully(rel_pc, &data, sizeof(data)) || data != 0x0f0000000fc0c748) {
+ if (!elf_memory->ReadFully(elf_offset, &data, sizeof(data)) || data != 0x0f0000000fc0c748) {
return false;
}
uint16_t data2;
- if (!elf_memory->ReadFully(rel_pc + 8, &data2, sizeof(data2)) || data2 != 0x0f05) {
+ if (!elf_memory->ReadFully(elf_offset + 8, &data2, sizeof(data2)) || data2 != 0x0f05) {
return false;
}
diff --git a/libunwindstack/include/unwindstack/Regs.h b/libunwindstack/include/unwindstack/Regs.h
index 1c2a81c..4f761b4 100644
--- a/libunwindstack/include/unwindstack/Regs.h
+++ b/libunwindstack/include/unwindstack/Regs.h
@@ -66,7 +66,7 @@
virtual uint64_t GetPcAdjustment(uint64_t rel_pc, Elf* elf) = 0;
- virtual bool StepIfSignalHandler(uint64_t rel_pc, Elf* elf, Memory* process_memory) = 0;
+ virtual bool StepIfSignalHandler(uint64_t elf_offset, Elf* elf, Memory* process_memory) = 0;
virtual bool SetPcFromReturnAddress(Memory* process_memory) = 0;
diff --git a/libunwindstack/include/unwindstack/RegsArm.h b/libunwindstack/include/unwindstack/RegsArm.h
index 44f6744..aa029be 100644
--- a/libunwindstack/include/unwindstack/RegsArm.h
+++ b/libunwindstack/include/unwindstack/RegsArm.h
@@ -40,7 +40,7 @@
bool SetPcFromReturnAddress(Memory* process_memory) override;
- bool StepIfSignalHandler(uint64_t rel_pc, Elf* elf, Memory* process_memory) override;
+ bool StepIfSignalHandler(uint64_t elf_offset, Elf* elf, Memory* process_memory) override;
void IterateRegisters(std::function<void(const char*, uint64_t)>) override final;
diff --git a/libunwindstack/include/unwindstack/RegsArm64.h b/libunwindstack/include/unwindstack/RegsArm64.h
index a72f91f..5cd7e5b 100644
--- a/libunwindstack/include/unwindstack/RegsArm64.h
+++ b/libunwindstack/include/unwindstack/RegsArm64.h
@@ -40,7 +40,7 @@
bool SetPcFromReturnAddress(Memory* process_memory) override;
- bool StepIfSignalHandler(uint64_t rel_pc, Elf* elf, Memory* process_memory) override;
+ bool StepIfSignalHandler(uint64_t elf_offset, Elf* elf, Memory* process_memory) override;
void IterateRegisters(std::function<void(const char*, uint64_t)>) override final;
diff --git a/libunwindstack/include/unwindstack/RegsMips.h b/libunwindstack/include/unwindstack/RegsMips.h
index c9dd202..8164a15 100644
--- a/libunwindstack/include/unwindstack/RegsMips.h
+++ b/libunwindstack/include/unwindstack/RegsMips.h
@@ -40,7 +40,7 @@
bool SetPcFromReturnAddress(Memory* process_memory) override;
- bool StepIfSignalHandler(uint64_t rel_pc, Elf* elf, Memory* process_memory) override;
+ bool StepIfSignalHandler(uint64_t elf_offset, Elf* elf, Memory* process_memory) override;
void IterateRegisters(std::function<void(const char*, uint64_t)>) override final;
diff --git a/libunwindstack/include/unwindstack/RegsMips64.h b/libunwindstack/include/unwindstack/RegsMips64.h
index 7c42812..c982542 100644
--- a/libunwindstack/include/unwindstack/RegsMips64.h
+++ b/libunwindstack/include/unwindstack/RegsMips64.h
@@ -40,7 +40,7 @@
bool SetPcFromReturnAddress(Memory* process_memory) override;
- bool StepIfSignalHandler(uint64_t rel_pc, Elf* elf, Memory* process_memory) override;
+ bool StepIfSignalHandler(uint64_t elf_offset, Elf* elf, Memory* process_memory) override;
void IterateRegisters(std::function<void(const char*, uint64_t)>) override final;
diff --git a/libunwindstack/include/unwindstack/RegsX86.h b/libunwindstack/include/unwindstack/RegsX86.h
index d19e449..2323a4f 100644
--- a/libunwindstack/include/unwindstack/RegsX86.h
+++ b/libunwindstack/include/unwindstack/RegsX86.h
@@ -41,7 +41,7 @@
bool SetPcFromReturnAddress(Memory* process_memory) override;
- bool StepIfSignalHandler(uint64_t rel_pc, Elf* elf, Memory* process_memory) override;
+ bool StepIfSignalHandler(uint64_t elf_offset, Elf* elf, Memory* process_memory) override;
void SetFromUcontext(x86_ucontext_t* ucontext);
diff --git a/libunwindstack/include/unwindstack/RegsX86_64.h b/libunwindstack/include/unwindstack/RegsX86_64.h
index dc9a220..3e919a4 100644
--- a/libunwindstack/include/unwindstack/RegsX86_64.h
+++ b/libunwindstack/include/unwindstack/RegsX86_64.h
@@ -41,7 +41,7 @@
bool SetPcFromReturnAddress(Memory* process_memory) override;
- bool StepIfSignalHandler(uint64_t rel_pc, Elf* elf, Memory* process_memory) override;
+ bool StepIfSignalHandler(uint64_t elf_offset, Elf* elf, Memory* process_memory) override;
void SetFromUcontext(x86_64_ucontext_t* ucontext);
diff --git a/libunwindstack/tests/ElfTest.cpp b/libunwindstack/tests/ElfTest.cpp
index d227b60..4866345 100644
--- a/libunwindstack/tests/ElfTest.cpp
+++ b/libunwindstack/tests/ElfTest.cpp
@@ -104,6 +104,8 @@
memory_->SetMemory(0x100, &phdr, sizeof(phdr));
}
+ void VerifyStepIfSignalHandler(uint64_t load_bias);
+
MemoryFake* memory_;
};
@@ -281,7 +283,7 @@
ASSERT_EQ(0x101U, elf.GetRelPc(0x1101, &map_info));
}
-TEST_F(ElfTest, step_in_signal_map) {
+void ElfTest::VerifyStepIfSignalHandler(uint64_t load_bias) {
ElfFake elf(memory_);
RegsArm regs;
@@ -290,6 +292,7 @@
ElfInterfaceFake* interface = new ElfInterfaceFake(memory_);
elf.FakeSetInterface(interface);
+ elf.FakeSetLoadBias(load_bias);
memory_->SetData32(0x3000, 0xdf0027ad);
MemoryFake process_memory;
@@ -299,12 +302,20 @@
}
elf.FakeSetValid(true);
- ASSERT_TRUE(elf.StepIfSignalHandler(0x3000, ®s, &process_memory));
+ ASSERT_TRUE(elf.StepIfSignalHandler(0x3000 + load_bias, ®s, &process_memory));
EXPECT_EQ(ERROR_NONE, elf.GetLastErrorCode());
EXPECT_EQ(15U, regs.pc());
EXPECT_EQ(13U, regs.sp());
}
+TEST_F(ElfTest, step_in_signal_map) {
+ VerifyStepIfSignalHandler(0);
+}
+
+TEST_F(ElfTest, step_in_signal_map_non_zero_load_bias) {
+ VerifyStepIfSignalHandler(0x1000);
+}
+
class ElfInterfaceMock : public ElfInterface {
public:
ElfInterfaceMock(Memory* memory) : ElfInterface(memory) {}
diff --git a/libunwindstack/tests/UnwindOfflineTest.cpp b/libunwindstack/tests/UnwindOfflineTest.cpp
index 0d58c09..364101a 100644
--- a/libunwindstack/tests/UnwindOfflineTest.cpp
+++ b/libunwindstack/tests/UnwindOfflineTest.cpp
@@ -1629,4 +1629,72 @@
EXPECT_EQ(0xfffe1d74ULL, unwinder.frames()[10].sp);
}
+TEST_F(UnwindOfflineTest, signal_load_bias_arm) {
+ ASSERT_NO_FATAL_FAILURE(Init("signal_load_bias_arm/", ARCH_ARM));
+
+ Unwinder unwinder(128, maps_.get(), regs_.get(), process_memory_);
+ unwinder.Unwind();
+
+ std::string frame_info(DumpFrames(unwinder));
+ ASSERT_EQ(17U, unwinder.NumFrames()) << "Unwind:\n" << frame_info;
+ EXPECT_EQ(
+ " #00 pc 0029ef9e libunwindstack_unit_test (SignalInnerFunction+10)\n"
+ " #01 pc 0029efa7 libunwindstack_unit_test (SignalMiddleFunction+2)\n"
+ " #02 pc 0029efaf libunwindstack_unit_test (SignalOuterFunction+2)\n"
+ " #03 pc 002a280b libunwindstack_unit_test (unwindstack::SignalCallerHandler(int, "
+ "siginfo*, void*)+10)\n"
+ " #04 pc 00058bd4 libc.so (__restore)\n"
+ " #05 pc 0029f01e libunwindstack_unit_test (InnerFunction+106)\n"
+ " #06 pc 0029f633 libunwindstack_unit_test (MiddleFunction+16)\n"
+ " #07 pc 0029f64b libunwindstack_unit_test (OuterFunction+16)\n"
+ " #08 pc 002a1711 libunwindstack_unit_test (unwindstack::RemoteThroughSignal(int, unsigned "
+ "int)+260)\n"
+ " #09 pc 002a1603 libunwindstack_unit_test "
+ "(unwindstack::UnwindTest_remote_through_signal_Test::TestBody()+10)\n"
+ " #10 pc 002c8fe3 libunwindstack_unit_test (testing::Test::Run()+130)\n"
+ " #11 pc 002c9b25 libunwindstack_unit_test (testing::TestInfo::Run()+184)\n"
+ " #12 pc 002c9e27 libunwindstack_unit_test (testing::TestSuite::Run()+202)\n"
+ " #13 pc 002d193d libunwindstack_unit_test "
+ "(testing::internal::UnitTestImpl::RunAllTests()+660)\n"
+ " #14 pc 002d160b libunwindstack_unit_test (testing::UnitTest::Run()+134)\n"
+ " #15 pc 002de035 libunwindstack_unit_test (IsolateMain+680)\n"
+ " #16 pc 00058155 libc.so (__libc_init+68)\n",
+ frame_info);
+
+ EXPECT_EQ(0xb6955f9eULL, unwinder.frames()[0].pc);
+ EXPECT_EQ(0xf2790ce8ULL, unwinder.frames()[0].sp);
+ EXPECT_EQ(0xb6955fa7ULL, unwinder.frames()[1].pc);
+ EXPECT_EQ(0xf2790ce8ULL, unwinder.frames()[1].sp);
+ EXPECT_EQ(0xb6955fafULL, unwinder.frames()[2].pc);
+ EXPECT_EQ(0xf2790cf0ULL, unwinder.frames()[2].sp);
+ EXPECT_EQ(0xb695980bULL, unwinder.frames()[3].pc);
+ EXPECT_EQ(0xf2790cf8ULL, unwinder.frames()[3].sp);
+ EXPECT_EQ(0xf23febd4ULL, unwinder.frames()[4].pc);
+ EXPECT_EQ(0xf2790d10ULL, unwinder.frames()[4].sp);
+ EXPECT_EQ(0xb695601eULL, unwinder.frames()[5].pc);
+ EXPECT_EQ(0xffe67798ULL, unwinder.frames()[5].sp);
+ EXPECT_EQ(0xb6956633ULL, unwinder.frames()[6].pc);
+ EXPECT_EQ(0xffe67890ULL, unwinder.frames()[6].sp);
+ EXPECT_EQ(0xb695664bULL, unwinder.frames()[7].pc);
+ EXPECT_EQ(0xffe678a0ULL, unwinder.frames()[7].sp);
+ EXPECT_EQ(0xb6958711ULL, unwinder.frames()[8].pc);
+ EXPECT_EQ(0xffe678b0ULL, unwinder.frames()[8].sp);
+ EXPECT_EQ(0xb6958603ULL, unwinder.frames()[9].pc);
+ EXPECT_EQ(0xffe67ac8ULL, unwinder.frames()[9].sp);
+ EXPECT_EQ(0xb697ffe3ULL, unwinder.frames()[10].pc);
+ EXPECT_EQ(0xffe67ad8ULL, unwinder.frames()[10].sp);
+ EXPECT_EQ(0xb6980b25ULL, unwinder.frames()[11].pc);
+ EXPECT_EQ(0xffe67ae8ULL, unwinder.frames()[11].sp);
+ EXPECT_EQ(0xb6980e27ULL, unwinder.frames()[12].pc);
+ EXPECT_EQ(0xffe67b18ULL, unwinder.frames()[12].sp);
+ EXPECT_EQ(0xb698893dULL, unwinder.frames()[13].pc);
+ EXPECT_EQ(0xffe67b48ULL, unwinder.frames()[13].sp);
+ EXPECT_EQ(0xb698860bULL, unwinder.frames()[14].pc);
+ EXPECT_EQ(0xffe67bb0ULL, unwinder.frames()[14].sp);
+ EXPECT_EQ(0xb6995035ULL, unwinder.frames()[15].pc);
+ EXPECT_EQ(0xffe67bd0ULL, unwinder.frames()[15].sp);
+ EXPECT_EQ(0xf23fe155ULL, unwinder.frames()[16].pc);
+ EXPECT_EQ(0xffe67d10ULL, unwinder.frames()[16].sp);
+}
+
} // namespace unwindstack
diff --git a/libunwindstack/tests/files/offline/signal_load_bias_arm/libc.so b/libunwindstack/tests/files/offline/signal_load_bias_arm/libc.so
new file mode 100644
index 0000000..f046624
--- /dev/null
+++ b/libunwindstack/tests/files/offline/signal_load_bias_arm/libc.so
Binary files differ
diff --git a/libunwindstack/tests/files/offline/signal_load_bias_arm/libunwindstack_unit_test b/libunwindstack/tests/files/offline/signal_load_bias_arm/libunwindstack_unit_test
new file mode 100644
index 0000000..f460dd6
--- /dev/null
+++ b/libunwindstack/tests/files/offline/signal_load_bias_arm/libunwindstack_unit_test
Binary files differ
diff --git a/libunwindstack/tests/files/offline/signal_load_bias_arm/maps.txt b/libunwindstack/tests/files/offline/signal_load_bias_arm/maps.txt
new file mode 100644
index 0000000..165ae49
--- /dev/null
+++ b/libunwindstack/tests/files/offline/signal_load_bias_arm/maps.txt
@@ -0,0 +1,4 @@
+b66b7000-b670c000 r--p 0 00:00 0 libunwindstack_unit_test
+b670c000-b69a8000 r-xp 54000 00:00 0 libunwindstack_unit_test
+f23a6000-f23d0000 r--p 0 00:00 0 libc.so
+f23d0000-f2451000 r-xp 29000 00:00 0 libc.so
diff --git a/libunwindstack/tests/files/offline/signal_load_bias_arm/regs.txt b/libunwindstack/tests/files/offline/signal_load_bias_arm/regs.txt
new file mode 100644
index 0000000..e03f8fd
--- /dev/null
+++ b/libunwindstack/tests/files/offline/signal_load_bias_arm/regs.txt
@@ -0,0 +1,16 @@
+r0: b69b7c84
+r1: 1
+r2: 1
+r3: 1
+r4: f1e52bd0
+r5: f1e11000
+r6: f1e52bd0
+r7: f1e52a38
+r8: f1e11000
+r9: 5de82a8f
+r10: f1e06030
+r11: f1e6d080
+ip: ffe67a88
+sp: f2790ce8
+lr: b6955fab
+pc: b6955f9e
diff --git a/libunwindstack/tests/files/offline/signal_load_bias_arm/stack0.data b/libunwindstack/tests/files/offline/signal_load_bias_arm/stack0.data
new file mode 100644
index 0000000..d9f23f8
--- /dev/null
+++ b/libunwindstack/tests/files/offline/signal_load_bias_arm/stack0.data
Binary files differ
diff --git a/libunwindstack/tests/files/offline/signal_load_bias_arm/stack1.data b/libunwindstack/tests/files/offline/signal_load_bias_arm/stack1.data
new file mode 100644
index 0000000..6011883
--- /dev/null
+++ b/libunwindstack/tests/files/offline/signal_load_bias_arm/stack1.data
Binary files differ