Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 065f1561 authored by Christopher Ferris's avatar Christopher Ferris
Browse files

Do not remove speculative frames in all cases.

If the first frame of an unwind is a totally invalid pc that's not in
any map, a speculative frame is added. Rather than deleting this frame
if no more unwinding is possible, leave it. This fixes a case where
the only frame you get is an invalid one, but the speculative frame
winds up in a shared library or somewhere else and gets removed.

Bug: 120505086

Test: New unit tests to catch this case pass.
Test: Verified original crashing program now emits two backtrace lines.
Change-Id: I088dff21c057386dcdaeb3fc2578b24322683bd0
parent 583ce2de
Loading
Loading
Loading
Loading
+8 −2
Original line number Original line Diff line number Diff line
@@ -239,8 +239,14 @@ void Unwinder::Unwind(const std::vector<std::string>* initial_map_names_to_skip,


    if (!stepped) {
    if (!stepped) {
      if (return_address_attempt) {
      if (return_address_attempt) {
        // Only remove the speculative frame if there are more than two frames
        // or the pc in the first frame is in a valid map.
        // This allows for a case where the code jumps into the middle of
        // nowhere, but there is no other unwind information after that.
        if (frames_.size() != 2 || maps_->Find(frames_[0].pc) != nullptr) {
          // Remove the speculative frame.
          // Remove the speculative frame.
          frames_.pop_back();
          frames_.pop_back();
        }
        break;
        break;
      } else if (in_device_map) {
      } else if (in_device_map) {
        // Do not attempt any other unwinding, pc or sp is in a device
        // Do not attempt any other unwinding, pc or sp is in a device
+63 −1
Original line number Original line Diff line number Diff line
@@ -659,6 +659,54 @@ TEST_F(UnwinderTest, speculative_frame_removed) {
  ElfInterfaceFake::FakePushFunctionData(FunctionData("Frame0", 0));
  ElfInterfaceFake::FakePushFunctionData(FunctionData("Frame0", 0));
  ElfInterfaceFake::FakePushFunctionData(FunctionData("Frame1", 1));
  ElfInterfaceFake::FakePushFunctionData(FunctionData("Frame1", 1));


  // Fake as if code called a nullptr function.
  regs_.set_pc(0x20000);
  regs_.set_sp(0x10000);
  ElfInterfaceFake::FakePushStepData(StepData(0, 0x10010, false));
  regs_.FakeSetReturnAddress(0x12);
  regs_.FakeSetReturnAddressValid(true);

  Unwinder unwinder(64, &maps_, &regs_, process_memory_);
  unwinder.Unwind();
  EXPECT_EQ(ERROR_INVALID_MAP, unwinder.LastErrorCode());

  ASSERT_EQ(2U, unwinder.NumFrames());

  auto* frame = &unwinder.frames()[0];
  EXPECT_EQ(0U, frame->num);
  EXPECT_EQ(0U, frame->rel_pc);
  EXPECT_EQ(0x20000U, frame->pc);
  EXPECT_EQ(0x10000U, frame->sp);
  EXPECT_EQ("Frame0", frame->function_name);
  EXPECT_EQ(0U, frame->function_offset);
  EXPECT_EQ("/system/fake/libunwind.so", frame->map_name);
  EXPECT_EQ(0U, frame->map_offset);
  EXPECT_EQ(0x20000U, frame->map_start);
  EXPECT_EQ(0x22000U, 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(0U, frame->rel_pc);
  EXPECT_EQ(0U, frame->pc);
  EXPECT_EQ(0x10010U, frame->sp);
  EXPECT_EQ("", frame->function_name);
  EXPECT_EQ(0U, frame->function_offset);
  EXPECT_EQ("", frame->map_name);
  EXPECT_EQ(0U, frame->map_offset);
  EXPECT_EQ(0U, frame->map_start);
  EXPECT_EQ(0U, frame->map_end);
  EXPECT_EQ(0U, frame->map_load_bias);
  EXPECT_EQ(0, frame->map_flags);
}

// Verify that a speculative frame is added and left if there are only
// two frames and the pc is in the middle nowhere.
TEST_F(UnwinderTest, speculative_frame_not_removed_pc_bad) {
  ElfInterfaceFake::FakePushFunctionData(FunctionData("Frame0", 0));
  ElfInterfaceFake::FakePushFunctionData(FunctionData("Frame1", 1));

  // Fake as if code called a nullptr function.
  // Fake as if code called a nullptr function.
  regs_.set_pc(0);
  regs_.set_pc(0);
  regs_.set_sp(0x10000);
  regs_.set_sp(0x10000);
@@ -669,7 +717,7 @@ TEST_F(UnwinderTest, speculative_frame_removed) {
  unwinder.Unwind();
  unwinder.Unwind();
  EXPECT_EQ(ERROR_NONE, unwinder.LastErrorCode());
  EXPECT_EQ(ERROR_NONE, unwinder.LastErrorCode());


  ASSERT_EQ(1U, unwinder.NumFrames());
  ASSERT_EQ(2U, unwinder.NumFrames());


  auto* frame = &unwinder.frames()[0];
  auto* frame = &unwinder.frames()[0];
  EXPECT_EQ(0U, frame->num);
  EXPECT_EQ(0U, frame->num);
@@ -684,6 +732,20 @@ TEST_F(UnwinderTest, speculative_frame_removed) {
  EXPECT_EQ(0U, frame->map_end);
  EXPECT_EQ(0U, frame->map_end);
  EXPECT_EQ(0U, frame->map_load_bias);
  EXPECT_EQ(0U, frame->map_load_bias);
  EXPECT_EQ(0, frame->map_flags);
  EXPECT_EQ(0, frame->map_flags);

  frame = &unwinder.frames()[1];
  EXPECT_EQ(1U, frame->num);
  EXPECT_EQ(0x200U, frame->rel_pc);
  EXPECT_EQ(0x1200U, 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);
}
}


// Verify that an unwind stops when a frame is in given suffix.
// Verify that an unwind stops when a frame is in given suffix.