Move sp/pc not changing check into Unwinder.

Remove this check from the DwarfSection class.

Rather than have every step function make the check, doing it at the
top level avoids having every function do the same check.

Bug: 68167269

Test: New unit tests, ran debuggerd -b on processes.
Change-Id: I23b7c799faaf26c93c1b72848df18c78de6c42fb

libunwindstack/DwarfSection.cpp

@@ -107,7 +107,6 @@ bool DwarfSectionImpl<AddressType>::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<AddressType>::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 <typename AddressType>

libunwindstack/Unwinder.cpp

@@ -87,8 +87,10 @@ void Unwinder::Unwind(const std::vector<std::string>* 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<std::string>* 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<std::string>* 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;
+    }
   }
 }
 

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<TypeParam> 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, &regs, &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);

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_, &regs_, 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;