From dff7d5afd51d7e831c44faf30f45f2d2ca02575b Mon Sep 17 00:00:00 2001 From: Brian Sheedy Date: Tue, 16 Mar 2021 17:39:32 +0000 Subject: [PATCH] Revert "arm: Allow the first function to use linked register as return pc" This reverts commit f2b3ab5e0af48f59754a96f0dbe95e857ba1abbd. Reason for revert: Causes symbolization errors on ARM ChromeOS devices crbug.com/1182948. Original change's description: > arm: Allow the first function to use linked register as return pc > > For a crash at the function entry with corrupted PC, the caller's PC > could be lying in the link register. Using the PC from link register > would be more effective than blindly scanning the stack immediately. > > Change-Id: I51673b7298e70faeeab2bfa97075e3c4793f94bc > Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2678992 > Reviewed-by: Mike Frysinger > Reviewed-by: Joshua Peraza Bug: 1182948 Change-Id: I2818b35ab1fb99012919cccc0fb80368e456ca15 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2765164 Reviewed-by: Joshua Peraza --- src/processor/microdump_processor_unittest.cc | 2 +- src/processor/stackwalker_arm.cc | 27 ------------------- src/processor/stackwalker_arm.h | 5 ---- 3 files changed, 1 insertion(+), 33 deletions(-) diff --git a/src/processor/microdump_processor_unittest.cc b/src/processor/microdump_processor_unittest.cc index 00f23f55..83bdef95 100644 --- a/src/processor/microdump_processor_unittest.cc +++ b/src/processor/microdump_processor_unittest.cc @@ -232,7 +232,7 @@ TEST_F(MicrodumpProcessorTest, TestProcessMultiple) { ASSERT_EQ("arm", state.system_info()->cpu); ASSERT_EQ("lge/p1_tmo_us/p1:6.0/MRA58K/1603210524c8d:user/release-keys", state.system_info()->os_version); - ASSERT_EQ(6U, state.threads()->at(0)->frames()->size()); + ASSERT_EQ(5U, state.threads()->at(0)->frames()->size()); } TEST_F(MicrodumpProcessorTest, TestProcessMips) { diff --git a/src/processor/stackwalker_arm.cc b/src/processor/stackwalker_arm.cc index f932ded9..f6f2c9bf 100644 --- a/src/processor/stackwalker_arm.cc +++ b/src/processor/stackwalker_arm.cc @@ -238,27 +238,6 @@ StackFrameARM* StackwalkerARM::GetCallerByFramePointer( return frame; } -StackFrameARM* StackwalkerARM::GetCallerByLinkRegister( - const vector& frames) { - StackFrameARM* last_frame = static_cast(frames.back()); - uint32_t last_lr = last_frame->context.iregs[MD_CONTEXT_ARM_REG_LR]; - - if (!(last_frame->context_validity & StackFrameARM::CONTEXT_VALID_LR) || - !InstructionAddressSeemsValid(last_lr)) { - return NULL; - } - - StackFrameARM* frame = new StackFrameARM(); - - frame->trust = StackFrame::FRAME_TRUST_SCAN; - frame->context = last_frame->context; - frame->context.iregs[MD_CONTEXT_ARM_REG_PC] = last_lr; - frame->context_validity = - context_frame_validity_ & (~StackFrameARM::CONTEXT_VALID_LR); - - return frame; -} - StackFrame* StackwalkerARM::GetCallerFrame(const CallStack* stack, bool stack_scan_allowed) { if (!memory_ || !stack) { @@ -285,12 +264,6 @@ StackFrame* StackwalkerARM::GetCallerFrame(const CallStack* stack, if (fp_register_ >= 0 && !frame.get()) frame.reset(GetCallerByFramePointer(frames)); - // For the first frame, return address may still be in the LR register at - // entry. Prefer to use LR register than scanning stack if LR register value - // points to a function range. - if (frames.size() == 1 && !frame.get()) - frame.reset(GetCallerByLinkRegister(frames)); - // If everuthing failed, fall back to stack scanning. if (stack_scan_allowed && !frame.get()) frame.reset(GetCallerByStackScan(frames)); diff --git a/src/processor/stackwalker_arm.h b/src/processor/stackwalker_arm.h index 9c386070..72ddddcc 100644 --- a/src/processor/stackwalker_arm.h +++ b/src/processor/stackwalker_arm.h @@ -82,11 +82,6 @@ class StackwalkerARM : public Stackwalker { // Return NULL on failure. StackFrameARM* GetCallerByFramePointer(const vector& frames); - // Use the link register if it seems to be a valid function adderss. - // The caller takes ownership of the returned frame. Return NULL on failure. - // This is useful when PC register is corrupted. - StackFrameARM* GetCallerByLinkRegister(const vector& frames); - // Scan the stack for plausible return addresses. The caller takes ownership // of the returned frame. Return NULL on failure. StackFrameARM* GetCallerByStackScan(const vector& frames);