From 074fc3b2f08092174a0802945b93c2e107db11d1 Mon Sep 17 00:00:00 2001 From: martin Date: Fri, 24 Apr 2020 20:08:11 +0000 Subject: [PATCH] FpDebug: Fix "stepped out" detection for step-over/in. Do not stop an "leave" command. git-svn-id: trunk@63061 - --- components/fpdebug/fpdbgclasses.pp | 72 ++++++++++++++--- components/fpdebug/fpdbgcontroller.pas | 7 +- components/fpdebug/fpdbgdisasx86.pp | 103 ++++++++++++++++++++++++- 3 files changed, 165 insertions(+), 17 deletions(-) diff --git a/components/fpdebug/fpdbgclasses.pp b/components/fpdebug/fpdbgclasses.pp index 128fa47371..7a02e69393 100644 --- a/components/fpdebug/fpdbgclasses.pp +++ b/components/fpdebug/fpdbgclasses.pp @@ -140,17 +140,21 @@ type This default assumes an Intel like stack, with StackPointer and FrameBase. This default assumes the stack grows by decreasing addresses. } + TDbgStackFrameInfo = class private FThread: TDbgThread; FStoredStackFrame, FStoredStackPointer: TDBGPtr; FHasSteppedOut: Boolean; + FProcessAfterRun: Boolean; + FLeaveState: (lsNone, lsWasAtLeave1, lsWasAtLeave2, lsLeaveDone); + Procedure DoAfterRun; protected - procedure DoCheckNextInstruction(ANextInstruction: TDbgAsmInstruction); virtual; + procedure DoCheckNextInstruction(ANextInstruction: TDbgAsmInstruction; NextIsSingleStep: Boolean); virtual; function CalculateHasSteppedOut: Boolean; virtual; public constructor Create(AThread: TDbgThread); - procedure CheckNextInstruction(ANextInstruction: TDbgAsmInstruction); inline; + procedure CheckNextInstruction(ANextInstruction: TDbgAsmInstruction; NextIsSingleStep: Boolean); inline; function HasSteppedOut: Boolean; inline; procedure FlagAsSteppedOut; inline; @@ -430,6 +434,8 @@ type function IsCallInstruction: boolean; virtual; function IsReturnInstruction: boolean; virtual; function IsLeaveStackFrame: boolean; virtual; + //function ModifiesBasePointer: boolean; virtual; + function ModifiesStackPointer: boolean; virtual; function IsJumpInstruction(IncludeConditional: Boolean = True; IncludeUncoditional: Boolean = True): boolean; virtual; function InstructionLength: Integer; virtual; end; @@ -1320,6 +1326,11 @@ begin Result := False; end; +function TDbgAsmInstruction.ModifiesStackPointer: boolean; +begin + Result := False; +end; + function TDbgAsmInstruction.IsJumpInstruction(IncludeConditional: Boolean; IncludeUncoditional: Boolean): boolean; begin @@ -2223,27 +2234,70 @@ end; { TDbgStackFrameInfo } -procedure TDbgStackFrameInfo.DoCheckNextInstruction( - ANextInstruction: TDbgAsmInstruction); +procedure TDbgStackFrameInfo.DoAfterRun; +var + CurStackFrame: TDBGPtr; begin - if ANextInstruction.IsReturnInstruction then + FProcessAfterRun := False; + case FLeaveState of + lsWasAtLeave1: begin + CurStackFrame := FThread.GetStackBasePointerRegisterValue; + FStoredStackPointer := FThread.GetStackPointerRegisterValue; + if CurStackFrame <> FStoredStackFrame then + FLeaveState := lsLeaveDone // real leave + else + FLeaveState := lsWasAtLeave2; // lea rsp,[rbp+$00] / pop ebp // epb in next command + end; + lsWasAtLeave2: begin + // TODO: maybe check, if stackpointer only goes down by sizeof(pointer) "Pop bp" + FStoredStackFrame := FThread.GetStackBasePointerRegisterValue; + FStoredStackPointer := FThread.GetStackPointerRegisterValue; + FLeaveState := lsLeaveDone; + end; + end; +end; + +procedure TDbgStackFrameInfo.DoCheckNextInstruction( + ANextInstruction: TDbgAsmInstruction; NextIsSingleStep: Boolean); +begin + if FProcessAfterRun then + DoAfterRun; + + if not NextIsSingleStep then begin + if FLeaveState = lsWasAtLeave2 then + FLeaveState := lsLeaveDone; + exit; + end; + + if ANextInstruction.IsReturnInstruction then begin FHasSteppedOut := True; + FLeaveState := lsLeaveDone; + end + else if FLeaveState = lsNone then begin + if ANextInstruction.IsLeaveStackFrame then + FLeaveState := lsWasAtLeave1; + end; + + FProcessAfterRun := FLeaveState in [lsWasAtLeave1, lsWasAtLeave2]; end; function TDbgStackFrameInfo.CalculateHasSteppedOut: Boolean; var CurBp, CurSp: TDBGPtr; begin + if FProcessAfterRun then + DoAfterRun; + Result := False; CurBp := FThread.GetStackBasePointerRegisterValue; if FStoredStackFrame < CurBp then begin CurSp := FThread.GetStackPointerRegisterValue; if FStoredStackPointer >= CurSp then // this happens, if current was recorded before the BP frame was set up // a finally handle may then fake an outer frame exit; - {$PUSH}{$Q-}{$R-} +// {$PUSH}{$Q-}{$R-} // if CurSp = FStoredStackPointer + FThread.Process.PointerSize then // exit; // Still in proc, but passed asm "leave" (BP has been popped, but IP not yet) - {$POP} +// {$POP} Result := True; debugln(FPDBG_COMMANDS, ['BreakStepBaseCmd.GetIsSteppedOut: Has stepped out Stored-BP=', FStoredStackFrame, ' < BP=', CurBp, ' / SP', CurSp]); end; @@ -2257,10 +2311,10 @@ begin end; procedure TDbgStackFrameInfo.CheckNextInstruction( - ANextInstruction: TDbgAsmInstruction); + ANextInstruction: TDbgAsmInstruction; NextIsSingleStep: Boolean); begin if not FHasSteppedOut then - DoCheckNextInstruction(ANextInstruction); + DoCheckNextInstruction(ANextInstruction, NextIsSingleStep); end; function TDbgStackFrameInfo.HasSteppedOut: Boolean; diff --git a/components/fpdebug/fpdbgcontroller.pas b/components/fpdebug/fpdbgcontroller.pas index 8f74e1feea..41bfa5e367 100644 --- a/components/fpdebug/fpdbgcontroller.pas +++ b/components/fpdebug/fpdbgcontroller.pas @@ -472,11 +472,10 @@ begin FStackFrameInfo := FThread.GetCurrentStackFrameInfo; end; -procedure TDbgControllerHiddenBreakStepBaseCmd.CallProcessContinue( - ASingleStep: boolean); +procedure TDbgControllerHiddenBreakStepBaseCmd.CallProcessContinue(ASingleStep: boolean); begin - if (FStackFrameInfo <> nil) and ASingleStep and (FHiddenBreakpoint = nil) then // TODO: not check FHiddenBreakAddr; - FStackFrameInfo.CheckNextInstruction(NextInstruction); + if (FStackFrameInfo <> nil) then + FStackFrameInfo.CheckNextInstruction(NextInstruction, ASingleStep); FProcess.Continue(FProcess, FThread, ASingleStep); end; diff --git a/components/fpdebug/fpdbgdisasx86.pp b/components/fpdebug/fpdbgdisasx86.pp index b482d471ff..973644c1ce 100644 --- a/components/fpdebug/fpdbgdisasx86.pp +++ b/components/fpdebug/fpdbgdisasx86.pp @@ -224,6 +224,8 @@ type function IsCallInstruction: boolean; override; function IsReturnInstruction: boolean; override; function IsLeaveStackFrame: boolean; override; + //function ModifiesBasePointer: boolean; override; + function ModifiesStackPointer: boolean; override; function IsJumpInstruction(IncludeConditional: Boolean = True; IncludeUncoditional: Boolean = True): boolean; override; function InstructionLength: Integer; override; function X86OpCode: TOpCode; @@ -440,19 +442,112 @@ begin end; function TX86AsmInstruction.IsReturnInstruction: boolean; +var + a: PByte; begin - Disassemble; + ReadCode; if diCodeReadError in FFlags then exit(False); - Result := (FInstruction.OpCode = OPret) or (FInstruction.OpCode = OPretf); + a := @FCodeBin[0]; + + // CF: IRET + Result := (a^ in [$C2, $C3, $CA, $CB, $CF]); end; function TX86AsmInstruction.IsLeaveStackFrame: boolean; +var + a: PByte; begin - Disassemble; + ReadCode; if diCodeReadError in FFlags then exit(False); - Result := (FInstruction.OpCode = OPleave); + a := @FCodeBin[0]; + // C9: leave + Result := (a^ = $C9); + if Result then + exit; + if (FAsmDecoder.FProcess.Mode = dm64) then begin + Result := + // 48 8D 65 00 / 5D: lea rsp,[rbp+$00] / pop ebp + ( (a^ = $48) and (a[1] = $8D) and (a[2] = $65) and (a[3] = $00) + and (a[4] = $5D) + ) or + // 48 89 ec / 5D: mov esp,ebp / pop ebp + ( (a^ = $48) and (a[1] = $89) and (a[2] = $EC) + and (a[3] = $5D) + ); + end + else begin + Result := + // 8D 65 00 / 5D: lea rsp,[rbp+$00] / pop ebp + ( (a[0] = $8D) and (a[1] = $65) and (a[2] = $00) + and (a[3] = $5D) + ) or + // 89 ec / 5D: mov esp,ebp / pop ebp + ( (a[0] = $89) and (a[1] = $EC) + and (a[2] = $5D) + ); + end; +end; + +function TX86AsmInstruction.ModifiesStackPointer: boolean; +var + a: PByte; +begin + (* Enter, Leave + mov sp, ... + lea sp, ... + pop / push + + BUT NOT ret + *) + Result := False; + ReadCode; + if diCodeReadError in FFlags then + exit; + a := @FCodeBin[0]; + + if (FAsmDecoder.FProcess.Mode = dm64) then begin + while (a < @FCodeBin[0] + INSTR_CODEBIN_LEN) and (a^ in [$40..$4F, $64..$67]) do + inc(a); + + // Pop/Push + if (a^ in [$50..$61, $68, $8F, $9C, $9d]) + then + exit(True); + end + else begin + while (a < @FCodeBin[0] + INSTR_CODEBIN_LEN) and (a^ in [$26, $2E, $36, $3E, $64..$67]) do + inc(a); + + // Pop/Push + if (a^ in [$06, $07, $0E, $16, $17, $1E, $1F, $50..$61, $68, $6A, $8F, $9C, $9d]) + then + exit(True); + end; + + // Pop/Push + if (a^ in [$FF]) + then begin + Disassemble; + exit(FInstruction.OpCode = OPpush); + end; + + if (a^ = $0F) and (a[1] in [$A0, $A1, $A8, $A9]) then + exit(True); + + // Enter/Leave + if (a^ in [$C8, $C9]) + then + exit(True); + + // Mov/Lea + if (a^ in [$89, $8B, $8D]) and + ( ((a[1] and $38) = $20) or ((a[1] and $03) = $04) ) // SP is involved + then begin + //Disassemble; + exit(True); // does report some "false positives" + end; end; function TX86AsmInstruction.IsJumpInstruction(IncludeConditional: Boolean;