From 6a9a8c977042219ef5ff97371ee866b9efae45b2 Mon Sep 17 00:00:00 2001 From: hengwu0 Date: Tue, 26 Nov 2019 19:03:24 +0800 Subject: [PATCH] nits fix: Fix code format and english grammar * move firstPCAfterPrologueDisassembly() and checkPrologue() out of arch independent. * s/do not/does not/ (for all tests) --- pkg/proc/disasm.go | 46 ++++++++++++++++++++++++++ pkg/proc/disasm_amd64.go | 46 -------------------------- pkg/proc/disasm_arm64.go | 46 -------------------------- pkg/proc/native/threads_linux_arm64.go | 1 - pkg/proc/proc_test.go | 42 +++++++++++------------ pkg/proc/variable_test.go | 2 +- pkg/terminal/command_test.go | 10 +++--- service/test/integration1_test.go | 4 +-- service/test/integration2_test.go | 12 +++---- service/test/variables_test.go | 2 +- 10 files changed, 82 insertions(+), 129 deletions(-) diff --git a/pkg/proc/disasm.go b/pkg/proc/disasm.go index 181bf4f7..207c7326 100644 --- a/pkg/proc/disasm.go +++ b/pkg/proc/disasm.go @@ -22,6 +22,52 @@ const ( GoFlavour ) +// firstPCAfterPrologueDisassembly returns the address of the first +// instruction after the prologue for function fn by disassembling fn and +// matching the instructions against known split-stack prologue patterns. +// If sameline is set firstPCAfterPrologueDisassembly will always return an +// address associated with the same line as fn.Entry +func firstPCAfterPrologueDisassembly(p Process, fn *Function, sameline bool) (uint64, error) { + var mem MemoryReadWriter = p.CurrentThread() + breakpoints := p.Breakpoints() + bi := p.BinInfo() + text, err := disassemble(mem, nil, breakpoints, bi, fn.Entry, fn.End, false) + if err != nil { + return fn.Entry, err + } + + if len(text) <= 0 { + return fn.Entry, nil + } + + for _, prologue := range prologues { + if len(prologue) >= len(text) { + continue + } + if checkPrologue(text, prologue) { + r := &text[len(prologue)] + if sameline { + if r.Loc.Line != text[0].Loc.Line { + return fn.Entry, nil + } + } + return r.Loc.PC, nil + } + } + + return fn.Entry, nil +} + +func checkPrologue(s []AsmInstruction, prologuePattern instrseq) bool { + line := s[0].Loc.Line + for i, op := range prologuePattern { + if s[i].Inst.Op != op || s[i].Loc.Line != line { + return false + } + } + return true +} + // Disassemble disassembles target memory between startAddr and endAddr, marking // the current instruction being executed in goroutine g. // If currentGoroutine is set and thread is stopped at a CALL instruction Disassemble diff --git a/pkg/proc/disasm_amd64.go b/pkg/proc/disasm_amd64.go index fd382568..fe8af89f 100644 --- a/pkg/proc/disasm_amd64.go +++ b/pkg/proc/disasm_amd64.go @@ -153,49 +153,3 @@ func init() { } } } - -// firstPCAfterPrologueDisassembly returns the address of the first -// instruction after the prologue for function fn by disassembling fn and -// matching the instructions against known split-stack prologue patterns. -// If sameline is set firstPCAfterPrologueDisassembly will always return an -// address associated with the same line as fn.Entry -func firstPCAfterPrologueDisassembly(p Process, fn *Function, sameline bool) (uint64, error) { - var mem MemoryReadWriter = p.CurrentThread() - breakpoints := p.Breakpoints() - bi := p.BinInfo() - text, err := disassemble(mem, nil, breakpoints, bi, fn.Entry, fn.End, false) - if err != nil { - return fn.Entry, err - } - - if len(text) <= 0 { - return fn.Entry, nil - } - - for _, prologue := range prologues { - if len(prologue) >= len(text) { - continue - } - if checkPrologue(text, prologue) { - r := &text[len(prologue)] - if sameline { - if r.Loc.Line != text[0].Loc.Line { - return fn.Entry, nil - } - } - return r.Loc.PC, nil - } - } - - return fn.Entry, nil -} - -func checkPrologue(s []AsmInstruction, prologuePattern instrseq) bool { - line := s[0].Loc.Line - for i, op := range prologuePattern { - if s[i].Inst.Op != op || s[i].Loc.Line != line { - return false - } - } - return true -} diff --git a/pkg/proc/disasm_arm64.go b/pkg/proc/disasm_arm64.go index 5aa033fc..f5da9113 100644 --- a/pkg/proc/disasm_arm64.go +++ b/pkg/proc/disasm_arm64.go @@ -115,49 +115,3 @@ func init() { } } } - -// firstPCAfterPrologueDisassembly returns the address of the first -// instruction after the prologue for function fn by disassembling fn and -// matching the instructions against known split-stack prologue patterns. -// If sameline is set firstPCAfterPrologueDisassembly will always return an -// address associated with the same line as fn.Entry -func firstPCAfterPrologueDisassembly(p Process, fn *Function, sameline bool) (uint64, error) { - var mem MemoryReadWriter = p.CurrentThread() - breakpoints := p.Breakpoints() - bi := p.BinInfo() - text, err := disassemble(mem, nil, breakpoints, bi, fn.Entry, fn.End, false) - if err != nil { - return fn.Entry, err - } - - if len(text) <= 0 { - return fn.Entry, nil - } - - for _, prologue := range prologues { - if len(prologue) >= len(text) { - continue - } - if checkPrologue(text, prologue) { - r := &text[len(prologue)] - if sameline { - if r.Loc.Line != text[0].Loc.Line { - return fn.Entry, nil - } - } - return r.Loc.PC, nil - } - } - - return fn.Entry, nil -} - -func checkPrologue(s []AsmInstruction, prologuePattern instrseq) bool { - line := s[0].Loc.Line - for i, op := range prologuePattern { - if s[i].Inst.Op != op || s[i].Loc.Line != line { - return false - } - } - return true -} diff --git a/pkg/proc/native/threads_linux_arm64.go b/pkg/proc/native/threads_linux_arm64.go index d07bd8c4..294c6ac5 100644 --- a/pkg/proc/native/threads_linux_arm64.go +++ b/pkg/proc/native/threads_linux_arm64.go @@ -34,7 +34,6 @@ func (t *Thread) restoreRegisters(savedRegs proc.Registers) error { iov := sys.Iovec{Base: &sr.Fpregset[0], Len: uint64(len(sr.Fpregset))} _, _, restoreRegistersErr = syscall.Syscall6(syscall.SYS_PTRACE, sys.PTRACE_SETREGSET, uintptr(t.ID), uintptr(elf.NT_FPREGSET), uintptr(unsafe.Pointer(&iov)), 0, 0) } - //return }) if restoreRegistersErr == syscall.Errno(0) { restoreRegistersErr = nil diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index 9dbac6f4..8f20ccb8 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -824,7 +824,7 @@ func (l1 *loc) match(l2 proc.Stackframe) bool { func TestStacktrace(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } stacks := [][]loc{ {{4, "main.stacktraceme"}, {8, "main.func1"}, {16, "main.main"}}, @@ -862,7 +862,7 @@ func TestStacktrace(t *testing.T) { func TestStacktrace2(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } withTestProcess("retstack", t, func(p proc.Process, fixture protest.Fixture) { assertNoError(proc.Continue(p), t, "Continue()") @@ -913,7 +913,7 @@ func stackMatch(stack []loc, locations []proc.Stackframe, skipRuntime bool) bool func TestStacktraceGoroutine(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } mainStack := []loc{{14, "main.stacktraceme"}, {29, "main.main"}} if goversion.VersionAfterOrEqual(runtime.Version(), 1, 11) { @@ -1238,7 +1238,7 @@ func TestVariableEvaluation(t *testing.T) { func TestFrameEvaluation(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } protest.AllowRecording(t) withTestProcess("goroutinestackprog", t, func(p proc.Process, fixture protest.Fixture) { @@ -1721,7 +1721,7 @@ func TestIssue384(t *testing.T) { func TestIssue332_Part1(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } // Next shouldn't step inside a function call protest.AllowRecording(t) @@ -1745,7 +1745,7 @@ func TestIssue332_Part1(t *testing.T) { func TestIssue332_Part2(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } // Step should skip a function's prologue // In some parts of the prologue, for some functions, the FDE data is incorrect @@ -1905,7 +1905,7 @@ func TestCmdLineArgs(t *testing.T) { func TestIssue462(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } // Stacktrace of Goroutine 0 fails with an error if runtime.GOOS == "windows" { @@ -1937,7 +1937,7 @@ func TestNextParked(t *testing.T) { t.Skip("test is not valid on FreeBSD") } if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } protest.AllowRecording(t) withTestProcess("parallel_next", t, func(p proc.Process, fixture protest.Fixture) { @@ -1990,7 +1990,7 @@ func TestNextParked(t *testing.T) { func TestStepParked(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } if runtime.GOOS == "freebsd" { t.Skip("test is not valid on FreeBSD") @@ -2316,7 +2316,7 @@ func TestStepConcurrentDirect(t *testing.T) { t.Skip("test is not valid on FreeBSD") } if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } protest.AllowRecording(t) withTestProcess("teststepconcurrent", t, func(p proc.Process, fixture protest.Fixture) { @@ -2700,7 +2700,7 @@ func getg(goid int, gs []*proc.G) *proc.G { func TestStacktraceWithBarriers(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } // Go's Garbage Collector will insert stack barriers into stacks. // This stack barrier is inserted by overwriting the return address for the @@ -3373,7 +3373,7 @@ func TestCgoStacktrace(t *testing.T) { func TestCgoSources(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Cgo-debug for now") + t.Skip("arm64 does not support Cgo-debug for now") } if runtime.GOOS == "windows" { ver, _ := goversion.Parse(runtime.Version()) @@ -3401,7 +3401,7 @@ func TestCgoSources(t *testing.T) { func TestSystemstackStacktrace(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } // check that we can follow a stack switch initiated by runtime.systemstack() withTestProcess("panic", t, func(p proc.Process, fixture protest.Fixture) { @@ -3422,7 +3422,7 @@ func TestSystemstackStacktrace(t *testing.T) { func TestSystemstackOnRuntimeNewstack(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } // The bug being tested here manifests as follows: // - set a breakpoint somewhere or interrupt the program with Ctrl-C @@ -3458,7 +3458,7 @@ func TestSystemstackOnRuntimeNewstack(t *testing.T) { func TestIssue1034(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace and Cgo-debug for now") + t.Skip("arm64 does not support Stacktrace and Cgo-debug for now") } // The external linker on macOS produces an abbrev for DW_TAG_subprogram // without the "has children" flag, we should support this. @@ -3478,7 +3478,7 @@ func TestIssue1034(t *testing.T) { func TestIssue1008(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace and Cgo-debug for now") + t.Skip("arm64 does not support Stacktrace and Cgo-debug for now") } // The external linker on macOS inserts "end of sequence" extended opcodes // in debug_line. which we should support correctly. @@ -3657,7 +3657,7 @@ func TestAllPCsForFileLines(t *testing.T) { func TestInlinedStacktraceAndVariables(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } if ver, _ := goversion.Parse(runtime.Version()); ver.Major >= 0 && !ver.AfterOrEqual(goversion.GoVersion{1, 10, -1, 0, 0, ""}) { // Versions of go before 1.10 do not have DWARF information for inlined calls @@ -4105,7 +4105,7 @@ func TestNextUnknownInstr(t *testing.T) { func TestReadDeferArgs(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } var tests = []struct { frame, deferCall int @@ -4151,7 +4151,7 @@ func TestReadDeferArgs(t *testing.T) { func TestIssue1374(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support FunctionCall for now") + t.Skip("arm64 does not support FunctionCall for now") } // Continue did not work when stopped at a breakpoint immediately after calling CallFunction. protest.MustSupportFunctionCalls(t, testBackend) @@ -4373,7 +4373,7 @@ func TestCallConcurrent(t *testing.T) { t.Skip("test is not valid on FreeBSD") } if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support FunctionCall for now") + t.Skip("arm64 does not support FunctionCall for now") } protest.MustSupportFunctionCalls(t, testBackend) withTestProcess("teststepconcurrent", t, func(p proc.Process, fixture protest.Fixture) { @@ -4457,7 +4457,7 @@ func TestIssue1615(t *testing.T) { func TestCgoStacktrace2(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace and Cgo-debug for now") + t.Skip("arm64 does not support Stacktrace and Cgo-debug for now") } if runtime.GOOS == "windows" { t.Skip("fixture crashes go runtime on windows") diff --git a/pkg/proc/variable_test.go b/pkg/proc/variable_test.go index bc670e90..f68f4831 100644 --- a/pkg/proc/variable_test.go +++ b/pkg/proc/variable_test.go @@ -11,7 +11,7 @@ import ( func TestGoroutineCreationLocation(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support GetStackInfo for now") + t.Skip("arm64 does not support GetStackInfo for now") } protest.AllowRecording(t) withTestProcess("goroutinestackprog", t, func(p proc.Process, fixture protest.Fixture) { diff --git a/pkg/terminal/command_test.go b/pkg/terminal/command_test.go index 66742611..b5e1b134 100644 --- a/pkg/terminal/command_test.go +++ b/pkg/terminal/command_test.go @@ -283,7 +283,7 @@ func TestIssue411(t *testing.T) { func TestScopePrefix(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } const goroutinesLinePrefix = " Goroutine " const goroutinesCurLinePrefix = "* Goroutine " @@ -847,7 +847,7 @@ func TestIssue1090(t *testing.T) { func TestPrintContextParkedGoroutine(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } withTestTerminal("goroutinestackprog", t, func(term *FakeTerminal) { term.MustExec("break stacktraceme") @@ -923,7 +923,7 @@ func TestOptimizationCheck(t *testing.T) { func TestTruncateStacktrace(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } withTestTerminal("stacktraceprog", t, func(term *FakeTerminal) { term.MustExec("break main.stacktraceme") @@ -943,7 +943,7 @@ func TestTruncateStacktrace(t *testing.T) { func TestIssue1493(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support FpRegs for now") + t.Skip("arm64 does not support FpRegs for now") } // The 'regs' command without the '-a' option should only return // general purpose registers. @@ -966,7 +966,7 @@ func findStarFile(name string) string { func TestIssue1598(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support FunctionCall for now") + t.Skip("arm64 does not support FunctionCall for now") } test.MustSupportFunctionCalls(t, testBackend) withTestTerminal("issue1598", t, func(term *FakeTerminal) { diff --git a/service/test/integration1_test.go b/service/test/integration1_test.go index df6bab63..33e551b2 100644 --- a/service/test/integration1_test.go +++ b/service/test/integration1_test.go @@ -725,7 +725,7 @@ func Test1ClientServer_SetVariable(t *testing.T) { func Test1ClientServer_FullStacktrace(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } withTestClient1("goroutinestackprog", t, func(c *rpc1.RPCClient) { _, err := c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.stacktraceme", Line: -1}) @@ -800,7 +800,7 @@ func Test1ClientServer_FullStacktrace(t *testing.T) { func Test1Issue355(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } // After the target process has terminated should return an error but not crash withTestClient1("continuetestprog", t, func(c *rpc1.RPCClient) { diff --git a/service/test/integration2_test.go b/service/test/integration2_test.go index 7a60a414..9e532761 100644 --- a/service/test/integration2_test.go +++ b/service/test/integration2_test.go @@ -804,7 +804,7 @@ func TestClientServer_SetVariable(t *testing.T) { func TestClientServer_FullStacktrace(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } protest.AllowRecording(t) withTestClient2("goroutinestackprog", t, func(c service.Client) { @@ -880,7 +880,7 @@ func TestClientServer_FullStacktrace(t *testing.T) { func TestIssue355(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support Stacktrace for now") + t.Skip("arm64 does not support Stacktrace for now") } // After the target process has terminated should return an error but not crash protest.AllowRecording(t) @@ -1569,7 +1569,7 @@ func mustHaveDebugCalls(t *testing.T, c service.Client) { func TestClientServerFunctionCall(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support FunctionCall for now") + t.Skip("arm64 does not support FunctionCall for now") } protest.MustSupportFunctionCalls(t, testBackend) withTestClient2("fncall", t, func(c service.Client) { @@ -1603,7 +1603,7 @@ func TestClientServerFunctionCall(t *testing.T) { func TestClientServerFunctionCallBadPos(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support FunctionCall for now") + t.Skip("arm64 does not support FunctionCall for now") } protest.MustSupportFunctionCalls(t, testBackend) if goversion.VersionAfterOrEqual(runtime.Version(), 1, 12) { @@ -1633,7 +1633,7 @@ func TestClientServerFunctionCallBadPos(t *testing.T) { func TestClientServerFunctionCallPanic(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support FunctionCall for now") + t.Skip("arm64 does not support FunctionCall for now") } protest.MustSupportFunctionCalls(t, testBackend) withTestClient2("fncall", t, func(c service.Client) { @@ -1662,7 +1662,7 @@ func TestClientServerFunctionCallPanic(t *testing.T) { func TestClientServerFunctionCallStacktrace(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support FunctionCall for now") + t.Skip("arm64 does not support FunctionCall for now") } protest.MustSupportFunctionCalls(t, testBackend) withTestClient2("fncall", t, func(c service.Client) { diff --git a/service/test/variables_test.go b/service/test/variables_test.go index 6473b703..9e91a6fe 100644 --- a/service/test/variables_test.go +++ b/service/test/variables_test.go @@ -1127,7 +1127,7 @@ type testCaseCallFunction struct { func TestCallFunction(t *testing.T) { if runtime.GOARCH == "arm64" { - t.Skip("arm64 do not support CallFunction for now") + t.Skip("arm64 does not support CallFunction for now") } protest.MustSupportFunctionCalls(t, testBackend) -- GitLab