Skip to content
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

Can not inject function calls that use floating point registers on FreeBSD #3001

Closed
aarzilli opened this issue May 4, 2022 · 0 comments · Fixed by #3019
Closed

Can not inject function calls that use floating point registers on FreeBSD #3001

aarzilli opened this issue May 4, 2022 · 0 comments · Fixed by #3019

Comments

@aarzilli
Copy link
Member

aarzilli commented May 4, 2022

It looks like there is some problem with the xsave area not being retrieved on FreeBSD, see the floatsum injection test in TestCallFunction.
Maybe we should disable function call injection on FreeBSD entirely.

aarzilli added a commit to aarzilli/delve that referenced this issue May 4, 2022
On FreeBSD it seems we have problems restoring and setting floating
point registers, since at least restoring is necessary for call
injection to function properly fully disable call injection on FreeBSD.

On rr the same problem exists, however due to the fact that we are
acting on a recording and ending a diversion will restore register
values anyway simply disable the floatsum test.

See also: rr-debugger/rr#3208

Updates go-delve#3001
derekparker pushed a commit that referenced this issue May 5, 2022
* Upgrade FreeBSD version

* proc: fixes concerning call injection on freebsd and rr

On FreeBSD it seems we have problems restoring and setting floating
point registers, since at least restoring is necessary for call
injection to function properly fully disable call injection on FreeBSD.

On rr the same problem exists, however due to the fact that we are
acting on a recording and ending a diversion will restore register
values anyway simply disable the floatsum test.

See also: rr-debugger/rr#3208

Updates #3001
4a6f656c added a commit to 4a6f656c/delve that referenced this issue May 29, 2022
The original code could never work - PT_SETREGS on freebsd does not
take an iovec, nor does it set FP registers. Furthermore, the xsave
bytes were not stored in the amd64util.AMD64Xstate struct.

Updates go-delve#3001
4a6f656c added a commit to 4a6f656c/delve that referenced this issue May 29, 2022
Floating point registers can now be set and restored correctly.

This is a partial revert of 51090f0.

Fixes go-delve#3001
derekparker pushed a commit that referenced this issue May 31, 2022
* pkg/proc: convert freebsd ptrace code to cgo

There is little point in having cgo call a custom C function, when the same
can be done directly from cgo (with less code and effort). Split the amd64
specific code into ptrace_freebsd_amd64.go. Also avoid mixing C.ptrace()
with syscall.SYS_PTRACE.

This will make further changes easier - no functional change intended.

* pkg/proc: check return values of ptrace calls on freebsd

The return values of the PT_GETNUMLWPS and PT_GETLWPLIST ptrace calls were
previously unchecked. While these should not fail, panic instead of using
-1 with slice allocation/handling.

* pkg/proc: return *amd64util.AMD64Xstate from freebsd ptraceGetRegset

Return a pointer to a struct, rather than a struct - this simplifies the
code in both the caller and the ptraceGetRegset function, while also avoiding
struct copying.

* pkg/proc: fix floating point register setting on freebsd

The original code could never work - PT_SETREGS on freebsd does not
take an iovec, nor does it set FP registers. Furthermore, the xsave
bytes were not stored in the amd64util.AMD64Xstate struct.

Updates #3001

* pkg/proc: re-enable function call injection on freebsd

Floating point registers can now be set and restored correctly.

This is a partial revert of 51090f0.

Fixes #3001

* pkg/proc: deduplicate register setting code on freebsd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant