From 8d7a05e08e0034e05349979899579ac08a7760b5 Mon Sep 17 00:00:00 2001 From: Archana Ravindar Date: Tue, 1 Aug 2023 16:28:27 +0530 Subject: [PATCH] Address review comments --- pkg/dwarf/regnum/ppc64le.go | 5 +-- pkg/proc/fncall.go | 64 +++++++-------------------- pkg/proc/linutil/regs_ppc64le_arch.go | 17 +------ pkg/proc/ppc64le_arch.go | 2 +- pkg/proc/proc_test.go | 5 +++ pkg/proc/test/support.go | 2 +- 6 files changed, 25 insertions(+), 70 deletions(-) diff --git a/pkg/dwarf/regnum/ppc64le.go b/pkg/dwarf/regnum/ppc64le.go index d583b9bab9..4a0291b513 100644 --- a/pkg/dwarf/regnum/ppc64le.go +++ b/pkg/dwarf/regnum/ppc64le.go @@ -17,16 +17,13 @@ const ( PPC64LE_F0 = PPC64LE_FIRST_FPR PPC64LE_LAST_FPR = 63 // Vector (Altivec/VMX) registers: from V0 to V31 - //PPC64LE_FIRST_VMX = 64 PPC64LE_FIRST_VMX = 77 PPC64LE_V0 = PPC64LE_FIRST_VMX - //PPC64LE_LAST_VMX = 95 PPC64LE_LAST_VMX = 108 // Vector Scalar (VSX) registers: from VS32 to VS63 - //PPC64LE_FIRST_VSX = 96 + // On ppc64le these are mapped to F0 to F31 PPC64LE_FIRST_VSX = 32 PPC64LE_VS0 = PPC64LE_FIRST_VSX - //PPC64LE_LAST_VSX = 160 PPC64LE_LAST_VSX = 63 // Condition Registers: from CR0 to CR7 PPC64LE_CR0 = 0 diff --git a/pkg/proc/fncall.go b/pkg/proc/fncall.go index 35137489fc..f58f22503b 100644 --- a/pkg/proc/fncall.go +++ b/pkg/proc/fncall.go @@ -334,10 +334,16 @@ func evalFunctionCall(scope *EvalScope, node *ast.CallExpr) (*Variable, error) { if err := writePointer(bi, scope.Mem, regs.SP()-3*uint64(bi.Arch.PtrSize()), uint64(fncall.argFrameSize)); err != nil { return nil, err } - case "arm64": + case "arm64","ppc64le": // debugCallV2 on arm64 needs a special call sequence, callOP can not be used sp := regs.SP() - sp -= 2 * uint64(bi.Arch.PtrSize()) + var spOffset uint64 + if bi.Arch.Name == "arm64" { + spOffset = 2 * uint64(bi.Arch.PtrSize()) + } else { + spOffset = 4 * uint64(bi.Arch.PtrSize()) + } + sp -= spOffset if err := setSP(thread, sp); err != nil { return nil, err } @@ -347,7 +353,7 @@ func evalFunctionCall(scope *EvalScope, node *ast.CallExpr) (*Variable, error) { if err := setLR(thread, regs.PC()); err != nil { return nil, err } - if err := writePointer(bi, scope.Mem, sp-uint64(2*bi.Arch.PtrSize()), uint64(fncall.argFrameSize)); err != nil { + if err := writePointer(bi, scope.Mem, sp-spOffset, uint64(fncall.argFrameSize)); err != nil { return nil, err } regs, err = thread.Registers() @@ -363,35 +369,6 @@ func evalFunctionCall(scope *EvalScope, node *ast.CallExpr) (*Variable, error) { if err != nil { return nil, err } - case "ppc64le": - // debugCallV2 on arm64 needs a special call sequence, callOP can not be used - sp := regs.SP() - sp -= 4 * uint64(bi.Arch.PtrSize()) - if err := setSP(thread, sp); err != nil { - return nil, err - } - if err := writePointer(bi, scope.Mem, sp, regs.LR()); err != nil { - return nil, err - } - if err := setLR(thread, regs.PC()); err != nil { - return nil, err - } - if err := writePointer(bi, scope.Mem, sp-uint64(4*bi.Arch.PtrSize()), uint64(fncall.argFrameSize)); err != nil { - return nil, err - } - regs, err = thread.Registers() - if err != nil { - return nil, err - } - regs, err = regs.Copy() - if err != nil { - return nil, err - } - fncall.savedRegs = regs - err = setPC(thread, dbgcallfn.Entry) - if err != nil { - return nil, err - } } @@ -422,12 +399,12 @@ func evalFunctionCall(scope *EvalScope, node *ast.CallExpr) (*Variable, error) { // adjust the value of registers inside scope pcreg, bpreg, spreg := scope.Regs.Reg(scope.Regs.PCRegNum), scope.Regs.Reg(scope.Regs.BPRegNum), scope.Regs.Reg(scope.Regs.SPRegNum) - scope.Regs.AddReg(scope.Regs.BPRegNum, bpreg) - scope.Regs.Reg(scope.Regs.BPRegNum).Uint64Val = uint64(bpoff + int64(scope.g.stack.hi)) scope.Regs.ClearRegisters() scope.Regs.AddReg(scope.Regs.PCRegNum, pcreg) + scope.Regs.AddReg(scope.Regs.BPRegNum, bpreg) scope.Regs.AddReg(scope.Regs.SPRegNum, spreg) scope.Regs.Reg(scope.Regs.SPRegNum).Uint64Val = uint64(spoff + int64(scope.g.stack.hi)) + scope.Regs.Reg(scope.Regs.BPRegNum).Uint64Val = uint64(bpoff + int64(scope.g.stack.hi)) scope.Regs.FrameBase = fboff + int64(scope.g.stack.hi) scope.Regs.CFA = scope.frameOffset + int64(scope.g.stack.hi) @@ -511,17 +488,12 @@ func callOP(bi *BinaryInfo, thread Thread, regs Registers, callAddr uint64) erro return err } return setPC(thread, callAddr) - case "arm64": + case "arm64","ppc64le": if err := setLR(thread, regs.PC()); err != nil { return err } return setPC(thread, callAddr) - case "ppc64le": - if err := setLR(thread, regs.PC()); err != nil { - return err - } - return setPC(thread, callAddr) default: panic("not implemented") } @@ -575,7 +547,6 @@ func funcCallEvalFuncExpr(scope *EvalScope, fncall *functionCallState, allowCall } argnum := len(fncall.expr.Args) - // If the function variable has a child then that child is the method // receiver. However, if the method receiver is not being used (e.g. // func (_ X) Foo()) then it will not actually be listed as a formal @@ -949,10 +920,7 @@ func funcCallStep(callScope *EvalScope, fncall *functionCallState, thread Thread case "amd64": setSP(thread, cfa) setPC(thread, oldpc) - case "arm64": - setLR(thread, oldlr) - setPC(thread, oldpc) - case "ppc64le": + case "arm64","ppc64le": setLR(thread, oldlr) setPC(thread, oldpc) default: @@ -1169,7 +1137,7 @@ func isCallInjectionStop(t *Target, thread Thread, loc *Location) bool { off = -int64(len(thread.BinInfo().Arch.breakpointInstruction)) } text, err := disassembleCurrentInstruction(t, thread, off) - if err != nil || len(text) <= 0 { + if err != nil || len(text) == 0 { return false } return text[0].IsHardBreak() @@ -1280,13 +1248,11 @@ func debugCallProtocolReg(archName string, version int) (uint64, bool) { return 0, false } return protocolReg, true - case "arm64": + case "arm64","ppc64le": if version == 2 { return regnum.ARM64_X0 + 20, true } return 0, false - case "ppc64le": - return regnum.PPC64LE_R0 + 20, true default: return 0, false } diff --git a/pkg/proc/linutil/regs_ppc64le_arch.go b/pkg/proc/linutil/regs_ppc64le_arch.go index 9e65b3f165..19ed93b1bb 100644 --- a/pkg/proc/linutil/regs_ppc64le_arch.go +++ b/pkg/proc/linutil/regs_ppc64le_arch.go @@ -126,7 +126,6 @@ func (r *PPC64LERegisters) Slice(floatingPoint bool) ([]proc.Register, error) { {"Dsisr", r.Regs.Dsisr}, {"Result", r.Regs.Result}, } - //println("Slice in ppc64le!!") out := make([]proc.Register, 0, len(regs)+len(r.Fpregs)) for _, reg := range regs { out = proc.AppendUint64Register(out, reg.k, reg.v) @@ -134,11 +133,9 @@ func (r *PPC64LERegisters) Slice(floatingPoint bool) ([]proc.Register, error) { var floatLoadError error if floatingPoint { if r.loadFpRegs != nil { - //println("loadfpregs is not nil") floatLoadError = r.loadFpRegs(r) r.loadFpRegs = nil } - //println("appending r.Fpregs") out = append(out, r.Fpregs...) } return out, floatLoadError @@ -184,10 +181,7 @@ func (r *PPC64LERegisters) SetReg(regNum uint64, reg *op.DwarfRegister) (fpchang r.Regs.Gpr[regNum-regnum.PPC64LE_R0] = reg.Uint64Val return false, nil - // archana - //case regNum >= regnum.PPC64LE_V0 && regNum <= regnum.PPC64LE_V0+31: case regNum >= regnum.PPC64LE_F0 && regNum <= regnum.PPC64LE_F0+31: - //println("i come here") if r.loadFpRegs != nil { err := r.loadFpRegs(r) r.loadFpRegs = nil @@ -195,14 +189,11 @@ func (r *PPC64LERegisters) SetReg(regNum uint64, reg *op.DwarfRegister) (fpchang return false, err } } - - //println(" called loadFpregs regs load val regnum is ", regNum) -// i := regNum - regnum.PPC64LE_F0 + // On ppc64le, PPC64LE_VS0 .. PPC64LE_VS31 are mapped onto + // PPC64LE_F0 .. PPC64LE_F31 i := regNum - regnum.PPC64LE_VS0 - //println("i before FillBytes",i) reg.FillBytes() copy(r.Fpregset[8*i:], reg.Bytes) - //println(" reg bytes inside float", reg.Bytes) return true, nil default: @@ -216,12 +207,8 @@ type PPC64LEPtraceFpRegs struct { } func (fpregs *PPC64LEPtraceFpRegs) Decode() (regs []proc.Register) { - //println("len in Decode ", len(fpregs.Fp)) for i := 0; i < len(fpregs.Fp); i += 8 { - //regs = proc.AppendBytesRegister(regs, fmt.Sprintf("F%d", i/16), fpregs.Fp[i:i+16]) - // regs = proc.AppendBytesRegister(regs, fmt.Sprintf("V%d", i/16), fpregs.Fp[i:i+16]) regs = proc.AppendBytesRegister(regs, fmt.Sprintf("VS%d", i/8), fpregs.Fp[i:i+8]) - //println("name ",i/8) } return } diff --git a/pkg/proc/ppc64le_arch.go b/pkg/proc/ppc64le_arch.go index 4596b91463..403c812c02 100644 --- a/pkg/proc/ppc64le_arch.go +++ b/pkg/proc/ppc64le_arch.go @@ -41,7 +41,7 @@ func PPC64LEArch(goos string) *Arch { asmRegisters: ppc64leAsmRegisters, RegisterNameToDwarf: nameToDwarfFunc(regnum.PPC64LENameToDwarf), debugCallMinStackSize: 320, - maxRegArgBytes: 8*8 + 13*8, + maxRegArgBytes: 13*8 + 13*8, //maxRegArgBytes: 16*8 + 16*8, } } diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index 4e5d5ca987..e1fbee2829 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -4310,6 +4310,8 @@ func TestReadDeferArgs(t *testing.T) { func TestIssue1374(t *testing.T) { // Continue did not work when stopped at a breakpoint immediately after calling CallFunction. + skipOn(t, "broken - pie mode", "linux", "ppc64le", "native", "pie") + protest.MustSupportFunctionCalls(t, testBackend) withTestProcess("issue1374", t, func(p *proc.Target, grp *proc.TargetGroup, fixture protest.Fixture) { setFileBreakpoint(p, t, fixture.Source, 7) @@ -4540,6 +4542,8 @@ func testCallConcurrentCheckReturns(p *proc.Target, t *testing.T, gid1, gid2 int } func TestCallConcurrent(t *testing.T) { + skipOn(t, "broken - pie mode", "linux", "ppc64le", "native", "pie") + protest.MustSupportFunctionCalls(t, testBackend) withTestProcess("teststepconcurrent", t, func(p *proc.Target, grp *proc.TargetGroup, fixture protest.Fixture) { bp := setFileBreakpoint(p, t, fixture.Source, 24) @@ -4935,6 +4939,7 @@ func TestIssue1925(t *testing.T) { // In particular the stepInstructionOut function called at the end of a // 'call' procedure should clean the G cache like every other function // altering the state of the target process. + skipOn(t, "broken - pie mode", "linux", "ppc64le", "native", "pie") protest.MustSupportFunctionCalls(t, testBackend) withTestProcess("testvariables2", t, func(p *proc.Target, grp *proc.TargetGroup, fixture protest.Fixture) { assertNoError(grp.Continue(), t, "Continue()") diff --git a/pkg/proc/test/support.go b/pkg/proc/test/support.go index 44a0ba5057..bcade70b9f 100644 --- a/pkg/proc/test/support.go +++ b/pkg/proc/test/support.go @@ -15,6 +15,7 @@ import ( "testing" "github.com/go-delve/delve/pkg/goversion" + ) // EnableRace allows to configure whether the race detector is enabled on target process. @@ -321,7 +322,6 @@ func MustSupportFunctionCalls(t *testing.T, testBackend string) { if runtime.GOARCH == "ppc64le" { if !goversion.VersionAfterOrEqual(runtime.Version(), 1, 20) { - //t.Skip("On PPC64LE Building with PIE mode or if version of Go lesser than 1.20 does not support function calls") t.Skip("On PPC64LE Building with Go lesser than 1.20 does not support function calls") } }