From 02fdb569f65f85730cca514311fd0965dec0b6a1 Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Fri, 8 Dec 2017 15:04:49 -0800 Subject: [PATCH] Do not check arch for format check. Use a generic check if the address is 32 bits when using the default formating of a backtrace line instead of an arch check. Test: New unit tests pass. Change-Id: Id609abc037d7b437a02d52763aa91fbefe5f4d5b --- libunwindstack/Unwinder.cpp | 2 +- libunwindstack/include/unwindstack/Regs.h | 4 ++ libunwindstack/tests/RegsFake.h | 2 + libunwindstack/tests/UnwinderTest.cpp | 84 +++++++++++++++++------ 4 files changed, 71 insertions(+), 21 deletions(-) diff --git a/libunwindstack/Unwinder.cpp b/libunwindstack/Unwinder.cpp index 4ae365d59..a83f85b9c 100644 --- a/libunwindstack/Unwinder.cpp +++ b/libunwindstack/Unwinder.cpp @@ -174,7 +174,7 @@ std::string Unwinder::FormatFrame(size_t frame_num) { if (frame_num >= frames_.size()) { return ""; } - return FormatFrame(frames_[frame_num], regs_->Arch() == ARCH_ARM || regs_->Arch() == ARCH_X86); + return FormatFrame(frames_[frame_num], regs_->Format32Bit()); } std::string Unwinder::FormatFrame(const FrameData& frame, bool bits32) { diff --git a/libunwindstack/include/unwindstack/Regs.h b/libunwindstack/include/unwindstack/Regs.h index 7025fcf12..613682ffd 100644 --- a/libunwindstack/include/unwindstack/Regs.h +++ b/libunwindstack/include/unwindstack/Regs.h @@ -51,6 +51,8 @@ class Regs { virtual ArchEnum Arch() = 0; + virtual bool Format32Bit() = 0; + virtual void* RawData() = 0; virtual uint64_t pc() = 0; virtual uint64_t sp() = 0; @@ -92,6 +94,8 @@ class RegsImpl : public Regs { void set_pc(AddressType pc) { pc_ = pc; } void set_sp(AddressType sp) { sp_ = sp; } + bool Format32Bit() override { return sizeof(AddressType) == sizeof(uint32_t); } + inline AddressType& operator[](size_t reg) { return regs_[reg]; } void* RawData() override { return regs_.data(); } diff --git a/libunwindstack/tests/RegsFake.h b/libunwindstack/tests/RegsFake.h index 3c5af4a2b..b81b2ca5d 100644 --- a/libunwindstack/tests/RegsFake.h +++ b/libunwindstack/tests/RegsFake.h @@ -45,6 +45,8 @@ class RegsFake : public Regs { void IterateRegisters(std::function) override {} + bool Format32Bit() { return false; } + uint64_t GetAdjustedPc(uint64_t rel_pc, Elf*) override { return rel_pc - 2; } bool StepIfSignalHandler(uint64_t, Elf*, Memory*) override { return false; } diff --git a/libunwindstack/tests/UnwinderTest.cpp b/libunwindstack/tests/UnwinderTest.cpp index 71103b4fb..2034191e2 100644 --- a/libunwindstack/tests/UnwinderTest.cpp +++ b/libunwindstack/tests/UnwinderTest.cpp @@ -28,6 +28,10 @@ #include #include #include +#include +#include +#include +#include #include #include "ElfFake.h" @@ -53,7 +57,7 @@ class UnwinderTest : public ::testing::Test { static void SetUpTestCase() { maps_.FakeClear(); MapInfo* info = new MapInfo(0x1000, 0x8000, 0, PROT_READ | PROT_WRITE, "/system/fake/libc.so"); - ElfFake* elf = new ElfFake(nullptr); + ElfFake* elf = new ElfFake(new MemoryFake); info->elf = elf; elf->FakeSetInterface(new ElfInterfaceFake(nullptr)); maps_.FakeAddMapInfo(info); @@ -66,31 +70,33 @@ class UnwinderTest : public ::testing::Test { maps_.FakeAddMapInfo(info); info = new MapInfo(0x20000, 0x22000, 0, PROT_READ | PROT_WRITE, "/system/fake/libunwind.so"); - elf = new ElfFake(nullptr); + elf = new ElfFake(new MemoryFake); info->elf = elf; elf->FakeSetInterface(new ElfInterfaceFake(nullptr)); maps_.FakeAddMapInfo(info); info = new MapInfo(0x23000, 0x24000, 0, PROT_READ | PROT_WRITE, "/fake/libanother.so"); - elf = new ElfFake(nullptr); + elf = new ElfFake(new MemoryFake); info->elf = elf; elf->FakeSetInterface(new ElfInterfaceFake(nullptr)); maps_.FakeAddMapInfo(info); info = new MapInfo(0x33000, 0x34000, 0, PROT_READ | PROT_WRITE, "/fake/compressed.so"); - elf = new ElfFake(nullptr); + elf = new ElfFake(new MemoryFake); info->elf = elf; elf->FakeSetInterface(new ElfInterfaceFake(nullptr)); maps_.FakeAddMapInfo(info); info = new MapInfo(0x43000, 0x44000, 0x1d000, PROT_READ | PROT_WRITE, "/fake/fake.apk"); - elf = new ElfFake(nullptr); + elf = new ElfFake(new MemoryFake); info->elf = elf; elf->FakeSetInterface(new ElfInterfaceFake(nullptr)); maps_.FakeAddMapInfo(info); info = new MapInfo(0x53000, 0x54000, 0, PROT_READ | PROT_WRITE, "/fake/fake.oat"); maps_.FakeAddMapInfo(info); + + process_memory_.reset(new MemoryFake); } void SetUp() override { @@ -690,29 +696,67 @@ TEST_F(UnwinderTest, format_frame_static) { EXPECT_EQ(" #01 pc 00001000 ", Unwinder::FormatFrame(frame, true)); } +static std::string ArchToString(ArchEnum arch) { + if (arch == ARCH_ARM) { + return "Arm"; + } else if (arch == ARCH_ARM64) { + return "Arm64"; + } else if (arch == ARCH_X86) { + return "X86"; + } else if (arch == ARCH_X86_64) { + return "X86_64"; + } else { + return "Unknown"; + } +} + // Verify format frame code. TEST_F(UnwinderTest, format_frame) { - ElfInterfaceFake::FakePushFunctionData(FunctionData("Frame0", 10)); + std::vector reg_list; + RegsArm* arm = new RegsArm; + arm->set_pc(0x2300); + arm->set_sp(0x10000); + reg_list.push_back(arm); - regs_.FakeSetPc(0x2300); - regs_.FakeSetSp(0x10000); + RegsArm64* arm64 = new RegsArm64; + arm64->set_pc(0x2300); + arm64->set_sp(0x10000); + reg_list.push_back(arm64); - Unwinder unwinder(64, &maps_, ®s_, process_memory_); - unwinder.Unwind(); + RegsX86* x86 = new RegsX86; + x86->set_pc(0x2300); + x86->set_sp(0x10000); + reg_list.push_back(x86); - ASSERT_EQ(1U, unwinder.NumFrames()); + RegsX86_64* x86_64 = new RegsX86_64; + x86_64->set_pc(0x2300); + x86_64->set_sp(0x10000); + reg_list.push_back(x86_64); - regs_.FakeSetArch(ARCH_ARM); - EXPECT_EQ(" #00 pc 00001300 /system/fake/libc.so (Frame0+10)", unwinder.FormatFrame(0)); - regs_.FakeSetArch(ARCH_X86); - EXPECT_EQ(" #00 pc 00001300 /system/fake/libc.so (Frame0+10)", unwinder.FormatFrame(0)); + for (auto regs : reg_list) { + ElfInterfaceFake::FakePushFunctionData(FunctionData("Frame0", 10)); - regs_.FakeSetArch(ARCH_ARM64); - EXPECT_EQ(" #00 pc 0000000000001300 /system/fake/libc.so (Frame0+10)", unwinder.FormatFrame(0)); - regs_.FakeSetArch(ARCH_X86_64); - EXPECT_EQ(" #00 pc 0000000000001300 /system/fake/libc.so (Frame0+10)", unwinder.FormatFrame(0)); + Unwinder unwinder(64, &maps_, regs, process_memory_); + unwinder.Unwind(); - EXPECT_EQ("", unwinder.FormatFrame(1)); + ASSERT_EQ(1U, unwinder.NumFrames()); + std::string expected; + switch (regs->Arch()) { + case ARCH_ARM: + case ARCH_X86: + expected = " #00 pc 00001300 /system/fake/libc.so (Frame0+10)"; + break; + case ARCH_ARM64: + case ARCH_X86_64: + expected = " #00 pc 0000000000001300 /system/fake/libc.so (Frame0+10)"; + break; + default: + expected = ""; + } + EXPECT_EQ(expected, unwinder.FormatFrame(0)) + << "Mismatch of frame format for regs arch " << ArchToString(regs->Arch()); + delete regs; + } } } // namespace unwindstack