nelhage / reptyr

Reparent a running program to a new terminal
MIT License
5.71k stars 216 forks source link

freebsd: fill in missing details for reptyr -T to work #108

Closed kevans91 closed 4 years ago

kevans91 commented 4 years ago

Around ~4 commits needed to make it happen; in an ideal world, the difference in remote syscall execution doesn't matter, so even on releases that don't have the proper op for grabbing return value information reptyr -T can still work.

kevans91 commented 4 years ago

I do seem to have discovered a bug external to reptyr (but reptyr tickles it), and I'm not sure yet if it's zsh or ptrace(2) at fault. When zsh is a session leader, we end up hanging when we go to grab_pid() it to ignore SIGHUP.

nelhage commented 4 years ago

I don't have ready access to a FreeBSD setup to test this, unfortunately. Should I go ahead and merge, or do you want to dig down into that bug a bit further, first?

kevans91 commented 4 years ago

I don't have ready access to a FreeBSD setup to test this, unfortunately. Should I go ahead and merge, or do you want to dig down into that bug a bit further, first?

It turns out the bug's kind of a FreeBSD+ptrace design issue and not an application bug -- zsh is in sigsuspend(2) while the child is running, and PT_TO_SCE doesn't trigger the kernel to release it from this state. I think (if I understand the details of our ptrace(2) well enough) we could work around it easily enough by rearchitecting a tad bit so that ptrace_attach_child takes an argument describing the intent of the caller (e.g. trace to syscall entry) so we can ensure we're in that state before we've even finished attached -- I believe SIGSTOP+SIGCONT would be sufficient to kick zsh out of sigsuspend and hitting our PT_TO_SCE as it tries to re-enter sigsuspend.

On the other hand, I don't know if it's worth it -- such a refactor would have to touch the linux side, too, and I don't know if we want to do that. Others seem to agree that a spurious EINTR from sigsuspend() to make life a little easier on tracers isn't a problem.

That being said, I think it's OK to go ahead and merge. If we want to provide a reptyr-level workaround for folks on older branches without a kernel solution, we should do that at the ports level instead of imposing refactoring burden here, I think.

nelhage commented 4 years ago

Sounds good, I'll land this. Thanks again for the great patches, and I'm excited about the prospect of landing CI for FreeBSD.