Skip to content

Commit

Permalink
proc: fix RFLAGS corruption after call injection on amd64 (#3002)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
aarzilli authored May 5, 2022
1 parent 51090f0 commit b53fcbe
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 1 deletion.
19 changes: 19 additions & 0 deletions _fixtures/badflags.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
9 changes: 8 additions & 1 deletion pkg/proc/dwarf_export_test.go
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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
}
11 changes: 11 additions & 0 deletions pkg/proc/fncall.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions pkg/proc/linutil/regs_amd64_arch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions pkg/proc/native/registers_windows_amd64.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
56 changes: 56 additions & 0 deletions pkg/proc/proc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
}

0 comments on commit b53fcbe

Please sign in to comment.