Skip to content

proc: avoid panic when call injection finishes before SetTarget - #4398

Draft
derekparker wants to merge 2 commits into
go-delve:masterfrom
derekparker:fix/fncall-settarget-empty-stack
Draft

proc: avoid panic when call injection finishes before SetTarget#4398
derekparker wants to merge 2 commits into
go-delve:masterfrom
derekparker:fix/fncall-settarget-empty-stack

Conversation

@derekparker

Copy link
Copy Markdown
Member

Guard CallInjectionSetTarget against an empty fncalls stack and treat premature RestoreRegisters / unknown protocol register values as errors so evaluation fails cleanly instead of panicking (issues #4085, #4363).

@aarzilli

Copy link
Copy Markdown
Member

I don't really see how this could be the problem. For us to get to funcCallStep some instruction must have set callInjectionContinue which is either CallInjectionComplete or CallInjectionComplete2. Either of those get inserted into the instruction stream after CallInjectionSetTarget by evalcompile.go.

Obviously the bug happens, so there must be a path that leads there (which I am not seeing), but to me it does not look like the path is this one.

@derekparker
derekparker force-pushed the fix/fncall-settarget-empty-stack branch from e14d7c0 to 3bb195f Compare July 22, 2026 21:59
@derekparker

Copy link
Copy Markdown
Member Author

I don't really see how this could be the problem. For us to get to funcCallStep some instruction must have set callInjectionContinue which is either CallInjectionComplete or CallInjectionComplete2. Either of those get inserted into the instruction stream after CallInjectionSetTarget by evalcompile.go.

Obviously the bug happens, so there must be a path that leads there (which I am not seeing), but to me it does not look like the path is this one.

CallInjectionStart also sets callInjectionContinue and enters funcCallStep before SetTarget runs. After Start, the next expected protocol value is CompleteCall (0), then resume continues into SetTarget. If that first stop is RestoreRegisters (16) instead (bad protocol register / mis-attributed stop / etc.), funcCallFinish pops the frame and stack.run() hits SetTarget with an empty fncalls stack and that's the fncallPeek panic from #4085/#4363.

@derekparker
derekparker force-pushed the fix/fncall-settarget-empty-stack branch 3 times, most recently from ddaa59c to 3d88507 Compare July 22, 2026 23:34
Post-Start mock drives premature RestoreRegisters and related cases so
the empty-fncalls SetTarget panic can be reproduced without a live
target. Broader eval-stack fuzzing is deferred to fix/eval-stack-fuzz.
Guard CallInjectionSetTarget against an empty fncalls stack and treat
premature RestoreRegisters / unknown protocol register values as errors
so evaluation fails cleanly instead of panicking (issues go-delve#4085, go-delve#4363).
@derekparker
derekparker force-pushed the fix/fncall-settarget-empty-stack branch from 3d88507 to 2cb971b Compare July 22, 2026 23:40

@aarzilli aarzilli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. But then instead of targetSet it should be callInjectionCompleted that we check for in the case for debugCallRegRestoreRegisters, no?

Comment thread pkg/proc/fncall.go
// Unknown protocol register value: do not continue blindly. Continuing
// without SetTarget can finish the call and then panic in
// evalCallInjectionSetTarget (see issues #4085, #4363).
fncall.err = unknownProtocolRegisterError(regval)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could just be the fmt.Errorf call directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants