Page MenuHomePhabricator

[lldb] [Process/NetBSD] Use global enable bits for watchpoints
ClosedPublic

Authored by mgorny on Jun 25 2019, 1:57 PM.

Details

Summary

Set global enable bits (i.e. bits 1, 3, 5, 7) to enable watchpoints
on NetBSD rather than the local enable bits (0, 2, 4, 6). The former
are necessary for watchpoints to be correctly recognized by the NetBSD
kernel. The latter cause them to be reported as trace points.

Diff Detail

Repository
rL LLVM

Event Timeline

mgorny created this revision.Jun 25 2019, 1:57 PM
mgorny updated this revision to Diff 206723.Jun 26 2019, 12:11 PM

Removed XFAIL for tests fixed by this.

labath accepted this revision.Jul 1 2019, 2:53 AM

Sorry about the delay, I was OOO last week. BTW, the usual rate of pinging on a patch is one week https://llvm.org/docs/DeveloperPolicy.html#code-reviews.

If the NetBSD kernel really sets the global-enable flag in the dr7 register, I'd be tempted to try to make it crash/misbehave by setting a hardware breakpoint somewhere in the early entry code (before it gets a chance to clear it, if it even does that), ideally in the breakpoint handler so it triggers an infinite loop :). However, that is not relevant to this patch...

lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
733 ↗(On Diff #206723)

Not a big deal, but I'd find 1 << (2*wp_index + 1) less surprising..

This revision is now accepted and ready to land.Jul 1 2019, 2:53 AM
mgorny marked an inline comment as done.Jul 1 2019, 3:08 AM

That's probably one of the reasons why NetBSD normally prevents unprivileged users from setting DRs.

lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
733 ↗(On Diff #206723)

Will do.

mgorny updated this revision to Diff 207255.Jul 1 2019, 3:15 AM

Updated per review.

mgorny marked an inline comment as done.Jul 1 2019, 3:15 AM

I would follow the same kernel behavior here as Linux, but that can be done independently.

krytarowski accepted this revision.Jul 1 2019, 7:52 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 1 2019, 8:12 AM