From 041eedd1268323788606580a3463f6e8911ff30e Mon Sep 17 00:00:00 2001 From: Michael Knyszek Date: Fri, 24 Sep 2021 18:27:44 -0400 Subject: [PATCH] pkg/proc: merge register data before writing to register (#2699) Right now, if (*compositeMemory).WriteMemory needs to write a value to a register that's smaller than the full size of the register (say, a uint32 being passed as an argument), then (*AMD64Registers).SetReg can later fail a sanity check that ensures the passed DwarfRegister is a full size register. Fix this by reading the old value of the register and overwriting just the relevant parts with the new register. For the purposes of an argument, it would probably be fine to just pad with zeroes, but merging with the existing value is what gdb does. Fixes #2698 --- _fixtures/fncall.go | 19 ++++++++++++++++++- pkg/dwarf/op/regs.go | 17 +++++++++++++++++ pkg/proc/dwarf_expr_test.go | 2 +- pkg/proc/mem.go | 4 +++- service/test/variables_test.go | 1 + 5 files changed, 40 insertions(+), 3 deletions(-) diff --git a/_fixtures/fncall.go b/_fixtures/fncall.go index 6967e102..326ebade 100644 --- a/_fixtures/fncall.go +++ b/_fixtures/fncall.go @@ -176,6 +176,17 @@ func regabistacktest2(n1, n2, n3, n4, n5, n6, n7, n8, n9, n10 int) (int, int, in return n1 + n2, n2 + n3, n3 + n4, n4 + n5, n5 + n6, n6 + n7, n7 + n8, n8 + n9, n9 + n10, n10 + n1 } +type Issue2698 struct { + a uint32 + b uint8 + c uint8 + d uintptr +} + +func (i Issue2698) String() string { + return fmt.Sprintf("%d %d %d %d", i.a, i.b, i.c, i.d) +} + func main() { one, two := 1, 2 intslice := []int{1, 2, 3} @@ -192,6 +203,12 @@ func main() { var pable_pa PRcvrable = pa var x X = 2 var x2 X2 = 2 + issue2698 := Issue2698{ + a: 1, + b: 2, + c: 3, + d: 4, + } fn2clos := makeclos(pa) fn2glob := call1 @@ -208,5 +225,5 @@ func main() { d.Method() d.Base.Method() x.CallMe() - fmt.Println(one, two, zero, call, call0, call2, callexit, callpanic, callbreak, callstacktrace, stringsJoin, intslice, stringslice, comma, a.VRcvr, a.PRcvr, pa, vable_a, vable_pa, pable_pa, fn2clos, fn2glob, fn2valmeth, fn2ptrmeth, fn2nil, ga, escapeArg, a2, square, intcallpanic, onetwothree, curriedAdd, getAStruct, getAStructPtr, getVRcvrableFromAStruct, getPRcvrableFromAStructPtr, getVRcvrableFromAStructPtr, pa2, noreturncall, str, d, x, x2.CallMe(5), longstrs, regabistacktest, regabistacktest2) + fmt.Println(one, two, zero, call, call0, call2, callexit, callpanic, callbreak, callstacktrace, stringsJoin, intslice, stringslice, comma, a.VRcvr, a.PRcvr, pa, vable_a, vable_pa, pable_pa, fn2clos, fn2glob, fn2valmeth, fn2ptrmeth, fn2nil, ga, escapeArg, a2, square, intcallpanic, onetwothree, curriedAdd, getAStruct, getAStructPtr, getVRcvrableFromAStruct, getPRcvrableFromAStructPtr, getVRcvrableFromAStructPtr, pa2, noreturncall, str, d, x, x2.CallMe(5), longstrs, regabistacktest, regabistacktest2, issue2698.String()) } diff --git a/pkg/dwarf/op/regs.go b/pkg/dwarf/op/regs.go index 88dcf23f..7696b928 100644 --- a/pkg/dwarf/op/regs.go +++ b/pkg/dwarf/op/regs.go @@ -164,3 +164,20 @@ func (reg *DwarfRegister) FillBytes() { reg.Bytes = make([]byte, 8) binary.LittleEndian.PutUint64(reg.Bytes, reg.Uint64Val) } + +// Overwrite takes the contents of reg1 and overwrites them with the contents +// of reg2 in little-endian order, returning a new register. The new register +// will always contain the complete contents of both registers, so if reg2 is +// larger than reg1, the final register will be reg2's size. +func (reg1 *DwarfRegister) Overwrite(reg2 *DwarfRegister) *DwarfRegister { + reg1.FillBytes() + reg2.FillBytes() + width := len(reg1.Bytes) + if len(reg2.Bytes) > len(reg1.Bytes) { + width = len(reg2.Bytes) + } + b := make([]byte, width) + copy(b, reg1.Bytes) + copy(b, reg2.Bytes) + return DwarfRegisterFromBytes(b) +} diff --git a/pkg/proc/dwarf_expr_test.go b/pkg/proc/dwarf_expr_test.go index 51860728..439bb578 100644 --- a/pkg/proc/dwarf_expr_test.go +++ b/pkg/proc/dwarf_expr_test.go @@ -245,7 +245,7 @@ func TestDwarfExprComposite(t *testing.T) { if changeCalls[0] != "3 - 2f00000000000000" { t.Errorf("wrong call to SetReg (Rbx)") } - if changeCalls[1] != "2 - 0c00" { + if changeCalls[1] != "2 - 0c00000000000000" { t.Errorf("wrong call to SetReg (Rcx)") } if mem.data[0x10] != 13 || mem.data[0x11] != 0x00 { diff --git a/pkg/proc/mem.go b/pkg/proc/mem.go index 281a0158..cb2a35f6 100644 --- a/pkg/proc/mem.go +++ b/pkg/proc/mem.go @@ -179,7 +179,9 @@ func (mem *compositeMemory) WriteMemory(addr uint64, data []byte) (int, error) { switch piece.Kind { case op.RegPiece: - err := mem.regs.ChangeFunc(piece.Val, op.DwarfRegisterFromBytes(pieceMem)) + oldReg := mem.regs.Reg(piece.Val) + newReg := op.DwarfRegisterFromBytes(pieceMem) + err := mem.regs.ChangeFunc(piece.Val, oldReg.Overwrite(newReg)) if err != nil { return donesz, err } diff --git a/service/test/variables_test.go b/service/test/variables_test.go index 95a2c8e1..44474084 100644 --- a/service/test/variables_test.go +++ b/service/test/variables_test.go @@ -1286,6 +1286,7 @@ func TestCallFunction(t *testing.T) { var testcases117 = []testCaseCallFunction{ {`regabistacktest("one", "two", "three", "four", "five", 4)`, []string{`:string:"onetwo"`, `:string:"twothree"`, `:string:"threefour"`, `:string:"fourfive"`, `:string:"fiveone"`, ":uint8:8"}, nil}, {`regabistacktest2(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)`, []string{":int:3", ":int:5", ":int:7", ":int:9", ":int:11", ":int:13", ":int:15", ":int:17", ":int:19", ":int:11"}, nil}, + {`issue2698.String()`, []string{`:string:"1 2 3 4"`}, nil}, } withTestProcessArgs("fncall", t, ".", nil, protest.AllNonOptimized, func(p *proc.Target, fixture protest.Fixture) {