Skip to content

Commit

Permalink
proc: fix escapeCheck infinite recursion if something can not be (#3311)
Browse files Browse the repository at this point in the history
deref'd

Fix infinite recursion if escapeCheck, at some point during its
recursion, creates an unreadable variable.

The deeper reason for this is that we evaluate function calls in a very
weird order so that we can always have stack space to store
intermediate evaluation results.
The variable 'value' happens to be stored in a register when we try to
make the call and because of our weird evaluation strategy registers
are no longer available to us when we evaluate 'value'.

This is not a complete fix for the issue, the real fix would be to
evaluate everything in its natural order, storing intermediate values
in Delve's memory instead of the target's stack. To do this we need a
mechanism to pin heap allocated objects, which at the moment does not
exist.

Updates #3310
  • Loading branch information
aarzilli committed Mar 27, 2023
1 parent 3507ff9 commit be88f98
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 0 deletions.
16 changes: 16 additions & 0 deletions _fixtures/reflecttypefncall.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package main

import (
"fmt"
"reflect"
)

func reflectFunc(value reflect.Value) {
fmt.Printf("%s\n", value.Type().Name())
}

func main() {
i := 2
val := reflect.ValueOf(i)
reflectFunc(val)
}
3 changes: 3 additions & 0 deletions pkg/proc/fncall.go
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,9 @@ func alignAddr(addr, align int64) int64 {
}

func escapeCheck(v *Variable, name string, stack stack) error {
if v.Unreadable != nil {
return fmt.Errorf("escape check for %s failed, variable unreadable: %v", name, v.Unreadable)
}
switch v.Kind {
case reflect.Ptr:
var w *Variable
Expand Down
10 changes: 10 additions & 0 deletions pkg/proc/proc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6043,3 +6043,13 @@ func TestFollowExec(t *testing.T) {
}
})
}

func TestEscapeCheckUnreadable(t *testing.T) {
// A failure in escapeCheck to dereference a field should not cause
// infinite recursion. See issue #3310.
withTestProcessArgs("reflecttypefncall", t, ".", []string{}, protest.AllNonOptimized, func(p *proc.Target, grp *proc.TargetGroup, fixture protest.Fixture) {
setFileBreakpoint(p, t, fixture.Source, 9)
assertNoError(grp.Continue(), t, "Continue")
proc.EvalExpressionWithCalls(grp, p.SelectedGoroutine(), "value.Type()", normalLoadConfig, true)
})
}

0 comments on commit be88f98

Please sign in to comment.