Page MenuHomeFreeBSD

Cleanups related to debug exceptions on x86.
ClosedPublic

Authored by jhb on Apr 25 2018, 2:39 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, May 6, 4:23 AM
Unknown Object (File)
Sun, May 5, 12:06 PM
Unknown Object (File)
Dec 30 2023, 9:38 AM
Unknown Object (File)
Dec 20 2023, 4:12 AM
Unknown Object (File)
Dec 12 2023, 6:26 PM
Unknown Object (File)
Nov 6 2023, 11:10 AM
Unknown Object (File)
Oct 19 2023, 6:06 PM
Unknown Object (File)
Oct 19 2023, 3:54 AM
Subscribers

Details

Summary
  • Add constants for fields in DR6 and the reserved fields in DR7. Use these constants instead of magic numbers in most places that use DR6 and DR7.
  • Refer to T_TRCTRAP as "debug exception" rather than a "trace trap" as it is not just for trace exceptions.
  • Always read DR6 for debug exceptions and only clear TF in the flags register for user exceptions where DR6.BS is set.
  • Clear DR6 before returning from a debug exception handler as recommended by the SDM dating all the way back to the 386. This allows debuggers to determine the cause of each exception. For kernel traps, clear DR6 in the T_TRCTRAP case and pass DR6 by value to other parts of the handler (namely, user_dbreg_trap()). For user traps, wait until after trapsignal to clear DR6 so that userland debuggers can read DR6 via PT_GETDBREGS while the thread is stopped in trapsignal().
Test Plan
  • have tested this with ptrace_test tests
  • I know that this will probably conflict with the other open review regarding PSL_T, so we'll just have to manage conflicts depending on who commits first.
  • in my TRAP_HWBKPT patch I have done one of the things you have done in your PSL_T change of splitting T_BPTFLT from T_TRCTRAP into separate cases in trap(). Possibly that should be committed as a separate little cleanup before either of those changes as the resulting diffs will be a bit more readable.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

It is a bit confusing to still have it be T_TRCTRAP, but all the comments now say debug exception...

Can I ask you to postpone this till mid-May, please ?

sys/amd64/amd64/machdep.c
2485 ↗(On Diff #41827)

There are at least two erratas for current generations of the Intel CPUs which state that %dr6 is not updated for some situations. I think it is worth at least soften the comment claim.

2487 ↗(On Diff #41827)

return (0); there and in other places. I think you can change all returns.

sys/amd64/amd64/trap.c
523 ↗(On Diff #41827)

This makes it impossible for the usermode handler to see the %dr6 value.

I do not think that Intel specifically mean that _the kernel_ handler must clear the bits, just that some handler should do it.

It is a bit confusing to still have it be T_TRCTRAP, but all the comments now say debug exception...

T_TRCTRAP is part of our API so we are stuck with it (there are things in ports using the value IIRC, though maybe only for the pre-7.0 siginfo case).

I can wait.

sys/amd64/amd64/trap.c
523 ↗(On Diff #41827)

This is the kernel handler, not the usermode handler. The usermode handler is careful to wait until after trapsignal() to clear it.

For the following change to add TRAP_HWBKPT I have to have the kernel clear dr6 rather than depending on the user application so I can reliably differentiate TRAP_TRACE (DR6.BS) from TRAP_HWBKPT (DR6.BS == 0 and at least one watchpoint set).

sys/amd64/amd64/trap.c
523 ↗(On Diff #41827)

Oh, yes, sorry, I misread. This is indeed the kernel handler.

  • Rebase
  • Split T_TRCTRAP from T_BPTFLT.
This revision is now accepted and ready to land.May 18 2018, 3:28 AM
This revision was automatically updated to reflect the committed changes.