From b53fcbe43ad13799bbfd2fa5d94d5d5a441f9ca5 Mon Sep 17 00:00:00 2001 From: Alessandro Arzilli Date: Thu, 5 May 2022 17:41:40 +0200 Subject: [PATCH] proc: fix RFLAGS corruption after call injection on amd64 (#3002) debugCallV2 for amd64 has a bug where it corrupts the flags registers every time it is called, this commit works around that problem by restoring flags one extra time to its original value after stepping out of debugCallV2. Fixes #2985 --- _fixtures/badflags.go | 19 ++++++++ pkg/proc/dwarf_export_test.go | 9 +++- pkg/proc/fncall.go | 11 +++++ pkg/proc/linutil/regs_amd64_arch.go | 2 + pkg/proc/native/registers_windows_amd64.go | 2 + pkg/proc/proc_test.go | 56 ++++++++++++++++++++++ 6 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 _fixtures/badflags.go diff --git a/_fixtures/badflags.go b/_fixtures/badflags.go new file mode 100644 index 0000000000..e4931b111d --- /dev/null +++ b/_fixtures/badflags.go @@ -0,0 +1,19 @@ +package main + +import ( + "fmt" + "os" +) + +func g() { +} + +func main() { + g() + a := os.Args[1] == "1" + if a { + fmt.Printf("true branch %v\n", a) + } else { + fmt.Printf("false branch %v\n", a) + } +} diff --git a/pkg/proc/dwarf_export_test.go b/pkg/proc/dwarf_export_test.go index b6d22c0879..191fde56d0 100644 --- a/pkg/proc/dwarf_export_test.go +++ b/pkg/proc/dwarf_export_test.go @@ -1,6 +1,9 @@ package proc -import "github.com/go-delve/delve/pkg/dwarf/op" +import ( + "github.com/go-delve/delve/pkg/dwarf/op" + "golang.org/x/arch/x86/x86asm" +) // PackageVars returns bi.packageVars (for tests) func (bi *BinaryInfo) PackageVars() []packageVar { @@ -23,3 +26,7 @@ func NewCompositeMemory(p *Target, pieces []op.Piece, base uint64) (*compositeMe } return mem, err } + +func IsJNZ(inst archInst) bool { + return inst.(*x86Inst).Op == x86asm.JNE +} diff --git a/pkg/proc/fncall.go b/pkg/proc/fncall.go index d2626f02a2..7fdb65f2c5 100644 --- a/pkg/proc/fncall.go +++ b/pkg/proc/fncall.go @@ -939,6 +939,17 @@ func funcCallStep(callScope *EvalScope, fncall *functionCallState, thread Thread if err := stepInstructionOut(p, thread, debugCallName, debugCallName); err != nil { fncall.err = fmt.Errorf("could not step out of %s: %v", debugCallName, err) } + if bi.Arch.Name == "amd64" { + // The tail of debugCallV2 corrupts the state of RFLAGS, we must restore + // it one extra time after stepping out of it. + // See https://github.com/go-delve/delve/issues/2985 and + // TestCallInjectionFlagCorruption + rflags := bi.Arch.RegistersToDwarfRegisters(0, fncall.savedRegs).Uint64Val(regnum.AMD64_Rflags) + err := thread.SetReg(regnum.AMD64_Rflags, op.DwarfRegisterFromUint64(rflags)) + if err != nil { + fncall.err = fmt.Errorf("could not restore RFLAGS register: %v", err) + } + } return true case debugCallRegReadReturn: // 1 diff --git a/pkg/proc/linutil/regs_amd64_arch.go b/pkg/proc/linutil/regs_amd64_arch.go index 38a8df8ff7..a15e565121 100644 --- a/pkg/proc/linutil/regs_amd64_arch.go +++ b/pkg/proc/linutil/regs_amd64_arch.go @@ -194,6 +194,8 @@ func (r *AMD64Registers) SetReg(regNum uint64, reg *op.DwarfRegister) (bool, err p = &r.Regs.R15 case regnum.AMD64_Rip: p = &r.Regs.Rip + case regnum.AMD64_Rflags: + p = &r.Regs.Eflags } if p != nil { diff --git a/pkg/proc/native/registers_windows_amd64.go b/pkg/proc/native/registers_windows_amd64.go index 2f44bc3318..c5652e08b1 100644 --- a/pkg/proc/native/registers_windows_amd64.go +++ b/pkg/proc/native/registers_windows_amd64.go @@ -79,6 +79,8 @@ func (thread *nativeThread) SetReg(regNum uint64, reg *op.DwarfRegister) error { return fmt.Errorf("wrong number of bytes for register %s (%d)", regnum.AMD64ToName(regNum), len(reg.Bytes)) } *p = reg.Uint64Val + } else if regNum == regnum.AMD64_Rflags { + context.Eflags = uint32(reg.Uint64Val) } else { if regNum < regnum.AMD64_XMM0 || regNum > regnum.AMD64_XMM0+15 { return fmt.Errorf("can not set register %s", regnum.AMD64ToName(regNum)) diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index 9a8753b585..0a74ac2a51 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -5852,3 +5852,59 @@ func TestStepIntoAutogeneratedSkip(t *testing.T) { assertLineNumber(p, t, 12, "After step") }) } + +func TestCallInjectionFlagCorruption(t *testing.T) { + // debugCallV2 has a bug in amd64 where its tail corrupts the FLAGS register by running an ADD instruction. + // Since this problem exists in many versions of Go, instead of fixing + // debugCallV2, we work around this problem by restoring FLAGS, one extra + // time, after stepping out of debugCallV2. + // Fixes issue https://github.com/go-delve/delve/issues/2985 + skipUnlessOn(t, "not relevant", "amd64") + + withTestProcessArgs("badflags", t, ".", []string{"0"}, 0, func(p *proc.Target, fixture protest.Fixture) { + mainfn := p.BinInfo().LookupFunc["main.main"] + + // Find JNZ instruction on line :14 + var addr uint64 + text, err := proc.Disassemble(p.Memory(), nil, p.Breakpoints(), p.BinInfo(), mainfn.Entry, mainfn.End) + assertNoError(err, t, "Disassemble") + for _, instr := range text { + if instr.Loc.Line != 14 { + continue + } + if proc.IsJNZ(instr.Inst) { + addr = instr.Loc.PC + } + } + if addr == 0 { + t.Fatalf("Could not find JNZ instruction at line :14") + } + + // Create breakpoint + _, err = p.SetBreakpoint(0, addr, proc.UserBreakpoint, nil) + assertNoError(err, t, "SetBreakpoint") + + // Continue to breakpoint + assertNoError(p.Continue(), t, "Continue()") + assertLineNumber(p, t, 14, "expected line :14") + + // Save RFLAGS register + rflagsBeforeCall := p.BinInfo().Arch.RegistersToDwarfRegisters(0, getRegisters(p, t)).Uint64Val(regnum.AMD64_Rflags) + t.Logf("rflags before = %#x", rflagsBeforeCall) + + // Inject call to main.g() + assertNoError(proc.EvalExpressionWithCalls(p, p.SelectedGoroutine(), "g()", normalLoadConfig, true), t, "Call") + + // Check RFLAGS register after the call + rflagsAfterCall := p.BinInfo().Arch.RegistersToDwarfRegisters(0, getRegisters(p, t)).Uint64Val(regnum.AMD64_Rflags) + t.Logf("rflags after = %#x", rflagsAfterCall) + + if rflagsBeforeCall != rflagsAfterCall { + t.Errorf("mismatched rflags value") + } + + // Single step and check where we end up + assertNoError(p.Step(), t, "Step()") + assertLineNumber(p, t, 17, "expected line :17") // since we passed "0" as argument we should be going into the false branch at line :17 + }) +}