From 48a13da168f6a82d9edf63a22769cb42a660996c Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Fri, 14 Oct 2016 10:28:47 -0700 Subject: Provide initial EBX value to FPO frame data evaluator EBX is sometimes used in "WIN FRAME 4" programs. Not providing the initial value was causing the evaluation in some frames of ntdll, resulting in a fallback to scanning and a failed stack walk. R=mark@chromium.org BUG=chromium:651453 Change-Id: I94a8184e1eed72b0d0e3212fe323fbdd10d56da5 Reviewed-on: https://chromium-review.googlesource.com/398059 Reviewed-by: Mark Mentovai --- src/processor/stackwalker_x86.cc | 18 +++- src/processor/stackwalker_x86_unittest.cc | 141 +++++++++++++++++++++++++++++- 2 files changed, 153 insertions(+), 6 deletions(-) diff --git a/src/processor/stackwalker_x86.cc b/src/processor/stackwalker_x86.cc index 29d98e4b..46e26cf9 100644 --- a/src/processor/stackwalker_x86.cc +++ b/src/processor/stackwalker_x86.cc @@ -200,13 +200,15 @@ StackFrameX86* StackwalkerX86::GetCallerByWindowsFrameInfo( } } - // Set up the dictionary for the PostfixEvaluator. %ebp and %esp are used - // in each program string, and their previous values are known, so set them - // here. + // Set up the dictionary for the PostfixEvaluator. %ebp, %esp, and sometimes + // %ebx are used in program strings, and their previous values are known, so + // set them here. PostfixEvaluator::DictionaryType dictionary; // Provide the current register values. dictionary["$ebp"] = last_frame->context.ebp; dictionary["$esp"] = last_frame->context.esp; + if (last_frame->context_validity & StackFrameX86::CONTEXT_VALID_EBX) + dictionary["$ebx"] = last_frame->context.ebx; // Provide constants from the debug info for last_frame and its callee. // .cbCalleeParams is a Breakpad extension that allows us to use the // PostfixEvaluator engine when certain types of debugging information @@ -330,11 +332,19 @@ StackFrameX86* StackwalkerX86::GetCallerByWindowsFrameInfo( // evaluation. The stack will not be examined to locate a saved // %ebp value, because these frames do not save (or use) %ebp. // + // We also propagate %ebx through, as it is commonly unmodifed after + // calling simple forwarding functions in ntdll (that are this non-EBP + // using type). It's not clear that this is always correct, but it is + // important for some functions to get a correct walk. + // // %eip_new = *(%esp_old + callee_params + saved_regs + locals) // %esp_new = %esp_old + callee_params + saved_regs + locals + 4 // %ebp_new = %ebp_old + // %ebx_new = %ebx_old // If available. program_string = "$eip .raSearchStart ^ = " - "$esp .raSearchStart 4 + ="; + "$esp .raSearchStart 4 + ="; + if (last_frame->context_validity & StackFrameX86::CONTEXT_VALID_EBX) + program_string += " $ebx $ebx ="; recover_ebp = false; } diff --git a/src/processor/stackwalker_x86_unittest.cc b/src/processor/stackwalker_x86_unittest.cc index d4c61c8c..6685b456 100644 --- a/src/processor/stackwalker_x86_unittest.cc +++ b/src/processor/stackwalker_x86_unittest.cc @@ -979,7 +979,8 @@ TEST_F(GetCallerFrame, WindowsFPOUnchangedEBP) { EXPECT_EQ(StackFrame::FRAME_TRUST_CFI, frame1->trust); ASSERT_EQ((StackFrameX86::CONTEXT_VALID_EIP | StackFrameX86::CONTEXT_VALID_ESP - | StackFrameX86::CONTEXT_VALID_EBP), + | StackFrameX86::CONTEXT_VALID_EBP + | StackFrameX86::CONTEXT_VALID_EBX), frame1->context_validity); EXPECT_EQ(0x40009b5bU, frame1->instruction + 1); EXPECT_EQ(0x40009b5bU, frame1->context.eip); @@ -1074,6 +1075,141 @@ TEST_F(GetCallerFrame, WindowsFPOUsedEBP) { } } +// This is a regression test for FPO code that references the initial value of +// EBX. +TEST_F(GetCallerFrame, WindowsFPOReferencesEBX) { + MockCodeModule ntdll(0x776e0000, 0x142000, "ntdll", "ntdllver"); + MockCodeModule kernel32(0x77250000, 0xd5000, "kernel32", "kernel32ver"); + MockCodeModule chrome_child(0x5a710000, 0x2b7c000, "chrome_child", + "chrome_childver"); + MockCodeModules modules; + modules.Add(&ntdll); + modules.Add(&kernel32); + modules.Add(&chrome_child); + SetModuleSymbols(&ntdll, + "PUBLIC 46bf4 0 KiFastSystemCallRet\n" + "STACK WIN 0 46bf4 1 0 0 0 0 0 0 0 0\n" + + "PUBLIC 46550 10 NtWaitForKeyedEvent\n" + "STACK WIN 0 46550 f 0 0 10 0 0 0 0 0\n" + + "PUBLIC 4965 10 RtlSleepConditionVariableSRW\n" + + "STACK WIN 4 4965 23 23 0 10 8 38 0 1 $T0 $ebx = " + "$eip $T0 4 + ^ = $ebx $T0 ^ = $esp $T0 8 + = " + "$ebp $ebp ^ = $L $ebp = $P $T0 8 + .cbParams + =\n" + + "STACK WIN 4 4988 e3 0 0 10 8 38 0 1 $T0 $ebx = " + "$eip $T0 4 + ^ = $ebx $T0 ^ = $esp $T0 8 + = " + "$ebp $ebp ^ = $L $ebp = $P $T0 8 + .cbParams + =\n" + + "STACK WIN 4 4b2e b 0 0 10 8 38 0 1 $T0 $ebx = " + "$eip $T0 4 + ^ = $ebx $T0 ^ = $esp $T0 8 + = " + "$ebp $ebp ^ = $L $ebp = $P $T0 8 + .cbParams + =\n"); + + SetModuleSymbols(&kernel32, + "PUBLIC 3217a 10 SleepConditionVariableSRW\n" + "STACK WIN 4 3217a 40 8 0 10 0 8 0 1 $T0 $ebp = $eip $T0 4 " + "+ ^ = $ebp $T0 ^ = $esp $T0 8 + = $L $T0 .cbSavedRegs - = " + "$P $T0 8 + .cbParams + =\n"); + + SetModuleSymbols(&chrome_child, + "FUNC 4f4851 20 0 base::ConditionVariable::TimedWait\n"); + + stack_section.start() = 0x0026c048; + stack_section + .D32(0x7772655c) + .D32(0x776e4a3f) + .D32(0x00000000) + .D32(0x0026c070) + .D32(0x00000000) + .D32(0x00000000) + .D32(0x00000001) + .D32(0x0026c168) + .D32(0x00000000) + .D32(0x00000000) + .D32(0x00000000) + .D32(0x0026c070) + .D32(0x00000000) + .D32(0x00000000) + .D32(0x00000000) + .D32(0x0026c164) + .D32(0x00be0000) + .D32(0x0026fc60) + .D32(0x0026c164) + .D32(0x00000000) + .D32(0xfffffffe) + .D32(0x00000000) + .D32(0x0026c0d0) + .D32(0x7728219e) + .D32(0x000003e8) + .D32(0x00000000) + .D32(0x7728219e) + .D32(0x0026c168) + .D32(0x0026c164) + .D32(0x00000000) + .D32(0x00000000) + .D32(0x0026c168) + .D32(0x000003e8) + .D32(0x000003e8) + .D32(0x0026c0ec) + .D32(0x5ac0486c) + .D32(0x0026c168) + .D32(0x0026c108) + .D32(0x5ac0484c) + .D32(0x0026c100) + .D32(0x0026c160); + + RegionFromSection(); + raw_context.eip = 0x77726bf4; // in ntdll!KiFastSystemCallRet + raw_context.esp = stack_section.start().Value(); + raw_context.ebp = 0x26c0a0; + raw_context.ebx = 0x26c0ac; + + StackFrameSymbolizer frame_symbolizer(&supplier, &resolver); + StackwalkerX86 walker(&system_info, &raw_context, &stack_region, &modules, + &frame_symbolizer); + vector modules_without_symbols; + vector modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); + ASSERT_EQ(0U, modules_without_symbols.size()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); + frames = call_stack.frames(); + + ASSERT_EQ(5U, frames->size()); + { + const StackFrameX86 &frame = *static_cast(frames->at(0)); + EXPECT_EQ(StackFrame::FRAME_TRUST_CONTEXT, frame.trust); + EXPECT_EQ(0x77726bf4U, frame.context.eip); + EXPECT_EQ("KiFastSystemCallRet", frame.function_name); + } + { + const StackFrameX86 &frame = *static_cast(frames->at(1)); + EXPECT_EQ(StackFrame::FRAME_TRUST_CFI, frame.trust); + EXPECT_EQ(0x7772655cU, frame.context.eip); + EXPECT_EQ("NtWaitForKeyedEvent", frame.function_name); + } + { + const StackFrameX86 &frame = *static_cast(frames->at(2)); + EXPECT_EQ(StackFrame::FRAME_TRUST_CFI, frame.trust); + EXPECT_EQ(0x776e4a3fU, frame.context.eip); + EXPECT_EQ("RtlSleepConditionVariableSRW", frame.function_name); + } + { + const StackFrameX86 &frame = *static_cast(frames->at(3)); + EXPECT_EQ(StackFrame::FRAME_TRUST_CFI, frame.trust); + EXPECT_EQ(0x7728219eU, frame.context.eip); + EXPECT_EQ("SleepConditionVariableSRW", frame.function_name); + } + { + const StackFrameX86 &frame = *static_cast(frames->at(4)); + EXPECT_EQ(StackFrame::FRAME_TRUST_CFI, frame.trust); + EXPECT_EQ(0x5ac0486cU, frame.context.eip); + EXPECT_EQ("base::ConditionVariable::TimedWait", frame.function_name); + } +} + // This is a regression unit test which covers a bug which has to do with // FPO-optimized Windows system call stubs in the context frame. There is // a more recent Windows system call dispatch mechanism which differs from @@ -1205,7 +1341,8 @@ TEST_F(GetCallerFrame, WindowsFPOSystemCall) { EXPECT_EQ(StackFrame::FRAME_TRUST_CFI, frame1->trust); ASSERT_EQ((StackFrameX86::CONTEXT_VALID_EIP | StackFrameX86::CONTEXT_VALID_ESP - | StackFrameX86::CONTEXT_VALID_EBP), + | StackFrameX86::CONTEXT_VALID_EBP + | StackFrameX86::CONTEXT_VALID_EBX), frame1->context_validity); EXPECT_EQ(0x75fa0a91U, frame1->instruction + 1); EXPECT_EQ(0x75fa0a91U, frame1->context.eip); -- cgit v1.2.1