dtrace/amd64: Fix probe argument fetching
ClosedPublic
Actions

Authored by markj on Sep 14 2024, 6:49 PM.

Details

Reviewers

gnn
avg

Group Reviewers

DTrace

Commits

rG8b1d908cdeb7: dtrace/amd64: Fix probe argument fetching
rG9aabab09c4e1: dtrace/amd64: Fix probe argument fetching

Summary

dtrace_getarg() previously walked the call stack looking for a frame
matching the dtrace_invop_callsite symbol, in order to look for a
trapframe corresponding to an invop (i.e., FBT or kinst) probe. Commit
3ba8e9dc4a0e broke this in some cases by breaking the expected alignment
of the dtrace_invop_callsite symbol.

Rather than groveling around the stack to find invop probe arguments,
simply use the trapframe reference saved by dtrace_invop(). This is
simpler and less fragile.

Reported by: avg
MFC after: 2 weeks
Fixes: 3ba8e9dc4a0e ("dtrace/amd64: Implement emulation of call instructions")

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

markj created this revision.Sep 14 2024, 6:49 PM

Herald added a reviewer: gnn. · View Herald TranscriptSep 14 2024, 6:49 PM

Herald added a subscriber: imp. · View Herald Transcript

markj requested review of this revision.Sep 14 2024, 6:49 PM

Harbormaster completed remote builds in B59484: Diff 143340.Sep 14 2024, 6:49 PM

markj added a child revision: D46673: dtrace_test: Remove the dependency on dtraceall.Sep 14 2024, 6:49 PM

I like how using t_dtrace_trapframe both eliminates the search for the frame below the trap frame and the assumption on the position of the trap frame.

The change looks good to me.

sys/cddl/dev/dtrace/amd64/dtrace_isa.c
35	Just curious, what requires this header?

This revision is now accepted and ready to land.Sep 15 2024, 8:55 AM

markj marked an inline comment as done.Sep 15 2024, 1:26 PM

markj added inline comments.

sys/cddl/dev/dtrace/amd64/dtrace_isa.c
35	This is for the t_dtrace_trapframe macro and per-thread struct kdtrace_thread. It's a bit silly having these in a separate cddl header. I'd like to get rid of it someday.