Skip to content

Commit

Permalink
*: switch to int64 for goroutine IDs (go-delve#3110)
Browse files Browse the repository at this point in the history
Go 1.20 switched to uint64 to represent goroutine IDs, we can't
actually follow suit because we have allowed clients to use -1 to refer
to the currently selected goroutine, however we should at least switch
to int64 and also update the rtype check to accept the 1.20 type.
  • Loading branch information
aarzilli authored Aug 16, 2022
1 parent 0c09fc9 commit 5b9f65d
Show file tree
Hide file tree
Showing 19 changed files with 122 additions and 99 deletions.
2 changes: 1 addition & 1 deletion _scripts/rtype-out.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type eface struct {

type g struct {
sched gobuf
goid int64
goid int64|uint64
gopc uintptr
startpc uintptr
waitsince int64
Expand Down
18 changes: 15 additions & 3 deletions _scripts/rtype.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,12 @@
// it must have type T).
//
//
// Anywhere a type is required anytype can be used to specify that we don't
// care about its type.
// Anywhere a type is required the following expressions can be used:
//
// - any builtin type
// - a type defined in the runtime package, without the 'runtime.' prefix
// - anytype to match all possible types
// - an expression of the form T1|T2 where both T1 and T2 can be arbitrary type expressions

package main

Expand Down Expand Up @@ -379,7 +383,7 @@ func processProcVariableUses(decl ast.Node, tinfo *types.Info, procVarIdent *ast
}
var methodName string
if isParseG {
if xident, _ := fncall.Fun.(*ast.Ident); xident != nil && xident.Name == "loadInt64Maybe" {
if xident, _ := fncall.Fun.(*ast.Ident); xident != nil && (xident.Name == "loadInt64Maybe" || xident.Name == "loadUint64Maybe") {
methodName = "loadInt64Maybe"
}
}
Expand Down Expand Up @@ -630,6 +634,14 @@ func matchType(typ types.Type, T string) bool {
if T == "anytype" {
return true
}
if strings.Index(T, "|") > 0 {
for _, t1 := range strings.Split(T, "|") {
if typeStr(typ) == t1 {
return true
}
}
return false
}
return typeStr(typ) == T
}

Expand Down
12 changes: 6 additions & 6 deletions pkg/proc/breakpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ func (bpstate *BreakpointState) checkCond(tgt *Target, breaklet *Breaklet, threa

switch breaklet.Kind {
case UserBreakpoint:
var goroutineID int
var goroutineID int64
lbp := bpstate.Breakpoint.Logical
if lbp != nil {
if g, err := GetG(thread); err == nil {
Expand Down Expand Up @@ -320,7 +320,7 @@ func (bpstate *BreakpointState) checkCond(tgt *Target, breaklet *Breaklet, threa
}

// checkHitCond evaluates bp's hit condition on thread.
func checkHitCond(lbp *LogicalBreakpoint, goroutineID int) bool {
func checkHitCond(lbp *LogicalBreakpoint, goroutineID int64) bool {
if lbp == nil || lbp.HitCond == nil {
return true
}
Expand Down Expand Up @@ -659,7 +659,7 @@ func (t *Target) setBreakpointInternal(logicalID int, addr uint64, kind Breakpoi
lbp := bpmap.Logical[logicalID]
if lbp == nil {
lbp = &LogicalBreakpoint{LogicalID: logicalID}
lbp.HitCount = make(map[int]uint64)
lbp.HitCount = make(map[int64]uint64)
lbp.Enabled = true
bpmap.Logical[logicalID] = lbp
}
Expand Down Expand Up @@ -974,9 +974,9 @@ type LogicalBreakpoint struct {
LoadArgs *LoadConfig
LoadLocals *LoadConfig

HitCount map[int]uint64 // Number of times a breakpoint has been reached in a certain goroutine
TotalHitCount uint64 // Number of times a breakpoint has been reached
HitCondPerG bool // Use per goroutine hitcount as HitCond operand, instead of total hitcount
HitCount map[int64]uint64 // Number of times a breakpoint has been reached in a certain goroutine
TotalHitCount uint64 // Number of times a breakpoint has been reached
HitCondPerG bool // Use per goroutine hitcount as HitCond operand, instead of total hitcount

// HitCond: if not nil the breakpoint will be triggered only if the evaluated HitCond returns
// true with the TotalHitCount.
Expand Down
2 changes: 1 addition & 1 deletion pkg/proc/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ const (
// ConvertEvalScope returns a new EvalScope in the context of the
// specified goroutine ID and stack frame.
// If deferCall is > 0 the eval scope will be relative to the specified deferred call.
func ConvertEvalScope(dbp *Target, gid, frame, deferCall int) (*EvalScope, error) {
func ConvertEvalScope(dbp *Target, gid int64, frame, deferCall int) (*EvalScope, error) {
if _, err := dbp.Valid(); err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/proc/goroutine_cache.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package proc

type goroutineCache struct {
partialGCache map[int]*G
partialGCache map[int64]*G
allGCache []*G

allgentryAddr, allglenAddr uint64
Expand Down Expand Up @@ -41,7 +41,7 @@ func (gcache *goroutineCache) getRuntimeAllg(bi *BinaryInfo, mem MemoryReadWrite

func (gcache *goroutineCache) addGoroutine(g *G) {
if gcache.partialGCache == nil {
gcache.partialGCache = make(map[int]*G)
gcache.partialGCache = make(map[int64]*G)
}
gcache.partialGCache[g.ID] = g
}
Expand Down
14 changes: 7 additions & 7 deletions pkg/proc/proc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1504,7 +1504,7 @@ func TestBreakpointCounts(t *testing.T) {
func TestHardcodedBreakpointCounts(t *testing.T) {
skipOn(t, "broken", "freebsd")
withTestProcess("hcbpcountstest", t, func(p *proc.Target, fixture protest.Fixture) {
counts := map[int]int{}
counts := map[int64]int{}
for {
if err := p.Continue(); err != nil {
if _, exited := err.(proc.ErrProcessExited); exited {
Expand Down Expand Up @@ -2556,7 +2556,7 @@ func TestStepConcurrentPtr(t *testing.T) {
}
}

kvals := map[int]int64{}
kvals := map[int64]int64{}
count := 0
for {
err := p.Continue()
Expand Down Expand Up @@ -2920,7 +2920,7 @@ func TestNextInDeferReturn(t *testing.T) {
})
}

func getg(goid int, gs []*proc.G) *proc.G {
func getg(goid int64, gs []*proc.G) *proc.G {
for _, g := range gs {
if g.ID == goid {
return g
Expand Down Expand Up @@ -3432,7 +3432,7 @@ func TestCgoStacktrace(t *testing.T) {
[]string{"C.helloworld_pt4", "C.helloworld_pt3", "main.helloWorldS", "main.helloWorld", "C.helloworld_pt2", "C.helloworld", "main.main"},
[]string{"main.helloWorld2", "C.helloworld_pt4", "C.helloworld_pt3", "main.helloWorldS", "main.helloWorld", "C.helloworld_pt2", "C.helloworld", "main.main"}}

var gid int
var gid int64

frameOffs := map[string]int64{}
framePointerOffs := map[string]int64{}
Expand Down Expand Up @@ -4449,7 +4449,7 @@ func TestIssue1469(t *testing.T) {
setFileBreakpoint(p, t, fixture.Source, 13)
assertNoError(p.Continue(), t, "Continue()")

gid2thread := make(map[int][]proc.Thread)
gid2thread := make(map[int64][]proc.Thread)
for _, thread := range p.ThreadList() {
g, _ := proc.GetG(thread)
if g == nil {
Expand Down Expand Up @@ -4582,7 +4582,7 @@ func TestAncestors(t *testing.T) {
})
}

func testCallConcurrentCheckReturns(p *proc.Target, t *testing.T, gid1, gid2 int) int {
func testCallConcurrentCheckReturns(p *proc.Target, t *testing.T, gid1, gid2 int64) int {
found := 0
for _, thread := range p.ThreadList() {
g, _ := proc.GetG(thread)
Expand Down Expand Up @@ -5138,7 +5138,7 @@ func TestStepOutPreservesGoroutine(t *testing.T) {

logState := func() {
g := p.SelectedGoroutine()
var goid int = -42
var goid int64 = -42
if g != nil {
goid = g.ID
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/proc/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ type Target struct {
selectedGoroutine *G

// fncallForG stores a mapping of current active function calls.
fncallForG map[int]*callInjection
fncallForG map[int64]*callInjection

asyncPreemptChanged bool // runtime/debug.asyncpreemptoff was changed
asyncPreemptOff int64 // cached value of runtime/debug.asyncpreemptoff
Expand Down Expand Up @@ -194,7 +194,7 @@ func NewTarget(p ProcessInternal, pid int, currentThread Thread, cfg NewTargetCo
t := &Target{
Process: p,
proc: p,
fncallForG: make(map[int]*callInjection),
fncallForG: make(map[int64]*callInjection),
StopReason: cfg.StopReason,
currentThread: currentThread,
CanDump: cfg.CanDump,
Expand Down
20 changes: 15 additions & 5 deletions pkg/proc/variables.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ const (
// G represents a runtime G (goroutine) structure (at least the
// fields that Delve is interested in).
type G struct {
ID int // Goroutine ID
ID int64 // Goroutine ID
PC uint64 // PC of goroutine when it was parked.
SP uint64 // SP of goroutine when it was parked.
BP uint64 // BP of goroutine when it was parked (go >= 1.7).
Expand Down Expand Up @@ -312,7 +312,7 @@ func GoroutinesInfo(dbp *Target, start, count int) ([]*G, int, error) {
}

var (
threadg = map[int]*G{}
threadg = map[int64]*G{}
allg []*G
)

Expand Down Expand Up @@ -367,7 +367,7 @@ func GoroutinesInfo(dbp *Target, start, count int) ([]*G, int, error) {

// FindGoroutine returns a G struct representing the goroutine
// specified by `gid`.
func FindGoroutine(dbp *Target, gid int) (*G, error) {
func FindGoroutine(dbp *Target, gid int64) (*G, error) {
if selg := dbp.SelectedGoroutine(); (gid == -1) || (selg != nil && selg.ID == gid) || (selg == nil && gid == 0) {
// Return the currently selected goroutine in the following circumstances:
//
Expand Down Expand Up @@ -886,7 +886,17 @@ func (v *Variable) parseG() (*G, error) {
return n
}

id := loadInt64Maybe("goid") // +rtype int64
loadUint64Maybe := func(name string) uint64 {
vv := v.loadFieldNamed(name)
if vv == nil {
unreadable = true
return 0
}
n, _ := constant.Uint64Val(vv.Value)
return n
}

id := loadUint64Maybe("goid") // +rtype int64|uint64
gopc := loadInt64Maybe("gopc") // +rtype uintptr
startpc := loadInt64Maybe("startpc") // +rtype uintptr
waitSince := loadInt64Maybe("waitsince") // +rtype int64
Expand Down Expand Up @@ -919,7 +929,7 @@ func (v *Variable) parseG() (*G, error) {
v.Name = "runtime.curg"

g := &G{
ID: int(id),
ID: int64(id),
GoPC: uint64(gopc),
StartPC: uint64(startpc),
PC: uint64(pc),
Expand Down
8 changes: 4 additions & 4 deletions pkg/terminal/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -886,7 +886,7 @@ func (c *Commands) goroutines(t *Term, ctx callContext, argstr string) error {
return nil
}

func selectedGID(state *api.DebuggerState) int {
func selectedGID(state *api.DebuggerState) int64 {
if state.SelectedGoroutine == nil {
return 0
}
Expand All @@ -908,7 +908,7 @@ func (c *Commands) goroutine(t *Term, ctx callContext, argstr string) error {
if args[0] == "" {
return printscope(t)
}
gid, err := strconv.Atoi(argstr)
gid, err := strconv.ParseInt(argstr, 10, 64)
if err != nil {
return err
}
Expand All @@ -927,7 +927,7 @@ func (c *Commands) goroutine(t *Term, ctx callContext, argstr string) error {
}

var err error
ctx.Scope.GoroutineID, err = strconv.Atoi(args[0])
ctx.Scope.GoroutineID, err = strconv.ParseInt(args[0], 10, 64)
if err != nil {
return err
}
Expand Down Expand Up @@ -2586,7 +2586,7 @@ func printcontextThread(t *Term, th *api.Thread) {
return
}

if hitCount, ok := th.Breakpoint.HitCount[strconv.Itoa(th.GoroutineID)]; ok {
if hitCount, ok := th.Breakpoint.HitCount[strconv.FormatInt(th.GoroutineID, 10)]; ok {
fmt.Fprintf(t.stdout, "> %s%s(%s) %s:%d (hits goroutine(%d):%d total:%d) (PC: %#v)\n",
bpname,
fn.Name(),
Expand Down
4 changes: 2 additions & 2 deletions service/api/conversions.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func ConvertLogicalBreakpoint(lbp *proc.LogicalBreakpoint) *Breakpoint {

b.HitCount = map[string]uint64{}
for idx := range lbp.HitCount {
b.HitCount[strconv.Itoa(idx)] = lbp.HitCount[idx]
b.HitCount[strconv.FormatInt(idx, 10)] = lbp.HitCount[idx]
}

if lbp.HitCond != nil {
Expand Down Expand Up @@ -95,7 +95,7 @@ func ConvertThread(th proc.Thread, bp *Breakpoint) *Thread {
file string
line int
pc uint64
gid int
gid int64
)

loc, err := th.Location()
Expand Down
8 changes: 4 additions & 4 deletions service/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ type Thread struct {
Function *Function `json:"function,omitempty"`

// ID of the goroutine running on this thread
GoroutineID int `json:"goroutineID"`
GoroutineID int64 `json:"goroutineID"`

// Breakpoint this thread is stopped at
Breakpoint *Breakpoint `json:"breakPoint,omitempty"`
Expand Down Expand Up @@ -358,7 +358,7 @@ type LoadConfig struct {
// internal G structure.
type Goroutine struct {
// ID is a unique identifier for the goroutine.
ID int `json:"id"`
ID int64 `json:"id"`
// Current location of the goroutine
CurrentLoc Location `json:"currentLoc"`
// Current location of the goroutine, excluding calls inside runtime
Expand Down Expand Up @@ -391,7 +391,7 @@ type DebuggerCommand struct {
ThreadID int `json:"threadID,omitempty"`
// GoroutineID is used to specify which thread to use with the SwitchGoroutine
// and Call commands.
GoroutineID int `json:"goroutineID,omitempty"`
GoroutineID int64 `json:"goroutineID,omitempty"`
// When ReturnInfoLoadConfig is not nil it will be used to load the value
// of any return variables.
ReturnInfoLoadConfig *LoadConfig
Expand Down Expand Up @@ -426,7 +426,7 @@ type BreakpointInfo struct {
// EvalScope is the scope a command should
// be evaluated in. Describes the goroutine and frame number.
type EvalScope struct {
GoroutineID int
GoroutineID int64
Frame int
DeferredCall int // when DeferredCall is n > 0 this eval scope is relative to the n-th deferred call in the current frame
}
Expand Down
8 changes: 4 additions & 4 deletions service/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ type Client interface {
// ReverseStepOut continues backward to the calle rof the current function.
ReverseStepOut() (*api.DebuggerState, error)
// Call resumes process execution while making a function call.
Call(goroutineID int, expr string, unsafe bool) (*api.DebuggerState, error)
Call(goroutineID int64, expr string, unsafe bool) (*api.DebuggerState, error)

// StepInstruction will step a single cpu instruction.
StepInstruction() (*api.DebuggerState, error)
Expand All @@ -59,7 +59,7 @@ type Client interface {
// SwitchThread switches the current thread context.
SwitchThread(threadID int) (*api.DebuggerState, error)
// SwitchGoroutine switches the current goroutine (and the current thread as well)
SwitchGoroutine(goroutineID int) (*api.DebuggerState, error)
SwitchGoroutine(goroutineID int64) (*api.DebuggerState, error)
// Halt suspends the process.
Halt() (*api.DebuggerState, error)

Expand Down Expand Up @@ -121,10 +121,10 @@ type Client interface {
ListGoroutinesWithFilter(start, count int, filters []api.ListGoroutinesFilter, group *api.GoroutineGroupingOptions) ([]*api.Goroutine, []api.GoroutineGroup, int, bool, error)

// Stacktrace returns stacktrace
Stacktrace(goroutineID int, depth int, opts api.StacktraceOptions, cfg *api.LoadConfig) ([]api.Stackframe, error)
Stacktrace(goroutineID int64, depth int, opts api.StacktraceOptions, cfg *api.LoadConfig) ([]api.Stackframe, error)

// Ancestors returns ancestor stacktraces
Ancestors(goroutineID int, numAncestors int, depth int) ([]api.Ancestor, error)
Ancestors(goroutineID int64, numAncestors int, depth int) ([]api.Ancestor, error)

// AttachedToExistingProcess returns whether we attached to a running process or not
AttachedToExistingProcess() bool
Expand Down
Loading

0 comments on commit 5b9f65d

Please sign in to comment.