-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
proc: adds pointer pinning to call injection #3787
base: master
Are you sure you want to change the base?
Conversation
dc74e97
to
93bdc6d
Compare
93bdc6d
to
ebad15c
Compare
pkg/proc/evalop/evalcompile.go
Outdated
|
||
// compileFunctionCallOld compiles a function call when runtime.debugPinner is | ||
// not available in the target. | ||
func (ctx *compileCtx) compileFunctionCallOld(node *ast.CallExpr, id int) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace the Old
suffix with NoPinning
.
pkg/proc/evalop/evalcompile.go
Outdated
@@ -570,6 +634,61 @@ func (ctx *compileCtx) compileFunctionCall(node *ast.CallExpr) error { | |||
return nil | |||
} | |||
|
|||
// compileFunctionCallNew compiles a function call when runtime.debugPinner | |||
// is available in the target. | |||
func (ctx *compileCtx) compileFunctionCallNew(node *ast.CallExpr, id int) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace New
suffix with WithPinning
.
pkg/proc/eval.go
Outdated
} | ||
|
||
func (s *evalStack) push(v *Variable) { | ||
if v == nil { | ||
panic(fmt.Errorf("internal debugger error, nil pushed onto variables stack")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no formatting here, why wrap in fmt.Errorf
?
} | ||
|
||
type evalLookup interface { | ||
FindTypeExpr(ast.Expr) (godwarf.Type, error) | ||
HasBuiltin(string) bool | ||
} | ||
|
||
type Flags uint8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doc comment please.
@@ -30,3 +31,14 @@ func NewCompositeMemory(p *Target, pieces []op.Piece, base uint64) (*compositeMe | |||
func IsJNZ(inst archInst) bool { | |||
return inst.(*x86Inst).Op == x86asm.JNE | |||
} | |||
|
|||
// HasDebugPinner returns true if the target has runtime.debugPinner. | |||
func (bi *BinaryInfo) HasDebugPinner() bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this should be promoted outside of test cases since it's used elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where?
pkg/proc/fncall.go
Outdated
stack.callInjectionContinue = false | ||
for _, v := range fncall.retvars { | ||
saveVariable(v) | ||
//fmt.Printf("addr %#v %s %s %#v\n", v.Addr, v.Name, v.DwarfType.String(), v.mem) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leftover
pkg/proc/fncall.go
Outdated
} | ||
sort.Slice(fncall.addrsToPin, func(i, j int) bool { return fncall.addrsToPin[i] < fncall.addrsToPin[j] }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use slices.Sort
here.
pkg/proc/fncall.go
Outdated
} | ||
sort.Slice(fncall.addrsToPin, func(i, j int) bool { return fncall.addrsToPin[i] < fncall.addrsToPin[j] }) | ||
fncall.addrsToPin = uniqUint64(fncall.addrsToPin) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can use slices.Compact
here.
This commit adds a new mode to call injection. If the runtime.debugPinner function is available in the target executable it obtains a pinner by calling it and then uses it to pin the pointers in the results of call injection. This allows the code for call injection to be refactored to execute the calls in the normal order, since it doesn't need to be concerned with having space on the target's memory to store intermediate values. Updates go-delve#3310
ebad15c
to
5cec6dc
Compare
There's a rare failure that seems to be happening with this changeset and windows, I want to investigate it before merging. |
This commit adds a new mode to call injection. If the runtime.debugPinner
function is available in the target executable it obtains a pinner by
calling it and then uses it to pin the pointers in the results of call
injection.
This allows the code for call injection to be refactored to execute the
calls in the normal order, since it doesn't need to be concerned with having
space on the target's memory to store intermediate values.
Updates #3310