diff --git a/libunwindstack/DwarfSection.cpp b/libunwindstack/DwarfSection.cpp index b8164c577..22921684a 100644 --- a/libunwindstack/DwarfSection.cpp +++ b/libunwindstack/DwarfSection.cpp @@ -107,7 +107,6 @@ bool DwarfSectionImpl::Eval(const DwarfCie* cie, Memory* regular_me return false; } - AddressType prev_pc = regs->pc(); AddressType prev_cfa = regs->sp(); AddressType cfa; @@ -233,8 +232,8 @@ bool DwarfSectionImpl::Eval(const DwarfCie* cie, Memory* regular_me *finished = (cur_regs->pc() == 0) ? true : false; cur_regs->set_sp(cfa); - // Return false if the unwind is not finished or the cfa and pc didn't change. - return *finished || prev_cfa != cfa || prev_pc != cur_regs->pc(); + + return true; } template diff --git a/libunwindstack/Unwinder.cpp b/libunwindstack/Unwinder.cpp index 2190711ec..2e46a115e 100644 --- a/libunwindstack/Unwinder.cpp +++ b/libunwindstack/Unwinder.cpp @@ -87,8 +87,10 @@ void Unwinder::Unwind(const std::vector* initial_map_names_to_skip, bool return_address_attempt = false; bool adjust_pc = false; for (; frames_.size() < max_frames_;) { - MapInfo* map_info = maps_->Find(regs_->pc()); + uint64_t cur_pc = regs_->pc(); + uint64_t cur_sp = regs_->sp(); + MapInfo* map_info = maps_->Find(regs_->pc()); uint64_t rel_pc; Elf* elf; if (map_info == nullptr) { @@ -138,6 +140,7 @@ void Unwinder::Unwind(const std::vector* initial_map_names_to_skip, } } } + if (!stepped) { if (return_address_attempt) { // Remove the speculative frame. @@ -157,6 +160,11 @@ void Unwinder::Unwind(const std::vector* initial_map_names_to_skip, } else { return_address_attempt = false; } + + // If the pc and sp didn't change, then consider everything stopped. + if (cur_pc == regs_->pc() && cur_sp == regs_->sp()) { + break; + } } } diff --git a/libunwindstack/tests/DwarfSectionImplTest.cpp b/libunwindstack/tests/DwarfSectionImplTest.cpp index 7e85bbbf8..5b9f3ee19 100644 --- a/libunwindstack/tests/DwarfSectionImplTest.cpp +++ b/libunwindstack/tests/DwarfSectionImplTest.cpp @@ -431,22 +431,6 @@ TYPED_TEST_P(DwarfSectionImplTest, Eval_reg_val_expr) { EXPECT_EQ(0x80000000U, regs.pc()); } -TYPED_TEST_P(DwarfSectionImplTest, Eval_same_cfa_same_pc) { - DwarfCie cie{.version = 3, .return_address_register = 5}; - RegsImplFake regs(10, 9); - dwarf_loc_regs_t loc_regs; - - regs.set_pc(0x100); - regs.set_sp(0x2000); - regs[5] = 0x100; - regs[8] = 0x2000; - loc_regs[CFA_REG] = DwarfLocation{DWARF_LOCATION_REGISTER, {8, 0}}; - bool finished; - ASSERT_FALSE(this->section_->Eval(&cie, &this->memory_, loc_regs, ®s, &finished)); - EXPECT_EQ(0x2000U, regs.sp()); - EXPECT_EQ(0x100U, regs.pc()); -} - TYPED_TEST_P(DwarfSectionImplTest, GetCie_fail_should_not_cache) { ASSERT_TRUE(this->section_->GetCie(0x4000) == nullptr); EXPECT_EQ(DWARF_ERROR_MEMORY_INVALID, this->section_->last_error()); @@ -872,9 +856,9 @@ REGISTER_TYPED_TEST_CASE_P( Eval_cfa_bad, Eval_cfa_register_prev, Eval_cfa_register_from_value, Eval_double_indirection, Eval_invalid_register, Eval_different_reg_locations, Eval_return_address_undefined, Eval_pc_zero, Eval_return_address, Eval_ignore_large_reg_loc, Eval_reg_expr, Eval_reg_val_expr, - Eval_same_cfa_same_pc, GetCie_fail_should_not_cache, GetCie_32_version_check, - GetCie_negative_data_alignment_factor, GetCie_64_no_augment, GetCie_augment, GetCie_version_3, - GetCie_version_4, GetFdeFromOffset_fail_should_not_cache, GetFdeFromOffset_32_no_augment, + GetCie_fail_should_not_cache, GetCie_32_version_check, GetCie_negative_data_alignment_factor, + GetCie_64_no_augment, GetCie_augment, GetCie_version_3, GetCie_version_4, + GetFdeFromOffset_fail_should_not_cache, GetFdeFromOffset_32_no_augment, GetFdeFromOffset_32_no_augment_non_zero_segment_size, GetFdeFromOffset_32_augment, GetFdeFromOffset_64_no_augment, GetFdeFromOffset_cached, GetCfaLocationInfo_cie_not_cached, GetCfaLocationInfo_cie_cached, Log); diff --git a/libunwindstack/tests/UnwinderTest.cpp b/libunwindstack/tests/UnwinderTest.cpp index 8a90baec0..869d1189e 100644 --- a/libunwindstack/tests/UnwinderTest.cpp +++ b/libunwindstack/tests/UnwinderTest.cpp @@ -127,6 +127,7 @@ class UnwinderTest : public ::testing::Test { void SetUp() override { ElfInterfaceFake::FakeClear(); regs_.FakeSetMachineType(EM_ARM); + regs_.FakeSetReturnAddressValid(false); } static MapsFake maps_; @@ -610,6 +611,71 @@ TEST_F(UnwinderTest, map_ignore_suffixes) { EXPECT_EQ(PROT_READ | PROT_WRITE, frame->map_flags); } +// Verify that an unwind stops when the sp and pc don't change. +TEST_F(UnwinderTest, sp_pc_do_not_change) { + ElfInterfaceFake::FakePushFunctionData(FunctionData("Frame0", 0)); + ElfInterfaceFake::FakePushFunctionData(FunctionData("Frame1", 1)); + ElfInterfaceFake::FakePushFunctionData(FunctionData("Frame2", 2)); + ElfInterfaceFake::FakePushFunctionData(FunctionData("Frame3", 3)); + ElfInterfaceFake::FakePushFunctionData(FunctionData("Frame4", 4)); + + regs_.FakeSetPc(0x1000); + regs_.FakeSetSp(0x10000); + ElfInterfaceFake::FakePushStepData(StepData(0x33402, 0x10010, false)); + ElfInterfaceFake::FakePushStepData(StepData(0x33502, 0x10020, false)); + ElfInterfaceFake::FakePushStepData(StepData(0x33502, 0x10020, false)); + ElfInterfaceFake::FakePushStepData(StepData(0x33502, 0x10020, false)); + ElfInterfaceFake::FakePushStepData(StepData(0x33502, 0x10020, false)); + ElfInterfaceFake::FakePushStepData(StepData(0, 0, true)); + + Unwinder unwinder(64, &maps_, ®s_, process_memory_); + unwinder.Unwind(); + + ASSERT_EQ(3U, unwinder.NumFrames()); + + auto* frame = &unwinder.frames()[0]; + EXPECT_EQ(0U, frame->num); + EXPECT_EQ(0U, frame->rel_pc); + EXPECT_EQ(0x1000U, frame->pc); + EXPECT_EQ(0x10000U, frame->sp); + EXPECT_EQ("Frame0", frame->function_name); + EXPECT_EQ(0U, frame->function_offset); + EXPECT_EQ("/system/fake/libc.so", frame->map_name); + EXPECT_EQ(0U, frame->map_offset); + EXPECT_EQ(0x1000U, frame->map_start); + EXPECT_EQ(0x8000U, frame->map_end); + EXPECT_EQ(0U, frame->map_load_bias); + EXPECT_EQ(PROT_READ | PROT_WRITE, frame->map_flags); + + frame = &unwinder.frames()[1]; + EXPECT_EQ(1U, frame->num); + EXPECT_EQ(0x400U, frame->rel_pc); + EXPECT_EQ(0x33400U, frame->pc); + EXPECT_EQ(0x10010U, frame->sp); + EXPECT_EQ("Frame1", frame->function_name); + EXPECT_EQ(1U, frame->function_offset); + EXPECT_EQ("/fake/compressed.so", frame->map_name); + EXPECT_EQ(0U, frame->map_offset); + EXPECT_EQ(0x33000U, frame->map_start); + EXPECT_EQ(0x34000U, frame->map_end); + EXPECT_EQ(0U, frame->map_load_bias); + EXPECT_EQ(PROT_READ | PROT_WRITE, frame->map_flags); + + frame = &unwinder.frames()[2]; + EXPECT_EQ(2U, frame->num); + EXPECT_EQ(0x500U, frame->rel_pc); + EXPECT_EQ(0x33500U, frame->pc); + EXPECT_EQ(0x10020U, frame->sp); + EXPECT_EQ("Frame2", frame->function_name); + EXPECT_EQ(2U, frame->function_offset); + EXPECT_EQ("/fake/compressed.so", frame->map_name); + EXPECT_EQ(0U, frame->map_offset); + EXPECT_EQ(0x33000U, frame->map_start); + EXPECT_EQ(0x34000U, frame->map_end); + EXPECT_EQ(0U, frame->map_load_bias); + EXPECT_EQ(PROT_READ | PROT_WRITE, frame->map_flags); +} + // Verify format frame code. TEST_F(UnwinderTest, format_frame_static) { FrameData frame;