Report stopped by trace if none of the watchpoint was hit

[tberghammer]

Summary

Report stopped by trace if none of the watchpoint was hit

Some linux kernel reports a watchpoint hit after single stepping even when no watchpoint was hit. This CL looks for a watchpoint which was hit and reports a stop by trace if it haven't found any.

[jingham]

How will you handle the case where the instruction we are stepping over triggers a real watchpoint?

[vharron]

How will you handle the case where the instruction we are stepping over triggers a real watchpoint?

Thanks for the suggestion:

"Create a test that verifies correct behavior when a watchpoint is hit during a step"
http://llvm.org/bugs/show_bug.cgi?id=22814

[jingham]

I didn't understand your reply. Adding a test case won't make this code work correctly.

This change should not go in till you figure out how to identify the watchpoint stop reason which corresponds to the return from the single step and only return true in that case.

[vharron]

Adding a test case won't make this code work correctly.

Sorry, that was confusing. I don't have a great solution at this point. The kernel is what the kernel is and it's on shipped Android systems, so we have to live with it. Hopefully that bug will eventually drive a kernel patch.

This change should not go in till you figure out how to identify the watchpoint stop reason which corresponds to the return from the single step and only return true in that case.

I see your point. Tamas, I've been assuming that this is related to your work on aarch64. Is that correct? If so, do you want to try to patch the kernel, ask Linaro for help or punt this whole problem to them?

[jingham]

Do you get any data back from the watchpoint hit? For instance, if the trace is implemented by a "pc != curr_pc" type watch, then you should be able to get the watched address from the watchpoint hit event. In that case, you can check whether the watched address was the current PC, and only report this as a trace stop when that is true.

Another way to do this, if you are using llgs, is to have THAT guy convert the watchpoint hit into a trace event. llgs is closer to the metal, and might have a better chance to figure out what just went on.

[jasonmolenda]

I'm no expert at this layer but you might want to have your kernel people look at the "ARMv8 Debug Architecture" manual from ARM, specifically section 4.2.1, "Effect of taking debug exception to an EL using AArch64 on system registers" (or the AArch32 section below that). It should be possible for the kernel to distinguish between a single step with the MDSCR_EL1 SS bit and a watchpoint debug event.

[tberghammer]

Thanks for noticing this bug and for all of the suggestions. I created a new fix what address the actual root cause of the problem with reporting the right stop reason from lldb-server.

For ARMv8 the watchpoint handling is not implemented yet, but it should be possible to implement the IsWatchpointHit function.

[jingham]

The approach looks right. I'll let somebody who works on the NativeThread side weigh in on whether the implementation is correct.

[tberghammer]

Friendly ping

[vharron]

source/Plugins/Process/Linux/NativeThreadLinux.cpp

406 ↗

(On Diff #21343)

Which kernels is it known to happen on/not happen on?

[chaoren]

LGTM

Some nits below:

source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h

49 ↗

(On Diff #21343)

Is this necessary?

source/Plugins/Process/Linux/NativeThreadLinux.cpp

405 ↗

(On Diff #21343)

What Vince asked (which kernels). Also nit: grammar.

[tberghammer]

source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h

49 ↗

(On Diff #21343)

Which part?

The override keyword is optional but gives us some more compile time check.

[tberghammer]

source/Plugins/Process/Linux/NativeThreadLinux.cpp

406 ↗

(On Diff #21343)

Added kernel description to comment (android-arm64 platfrom-21) and fixed grammar

[labath]

It could be that the kernel reports a watchpoint hit on single stepping because on arm single stepping is actually implemented using hardware breakpoints. This is just a guess, but in any case it might be interesting to create a test case, which what happens with single stepping after we have set the maximum number of watchpoints possible. Does single stepping still work? Do some of our watchpoints get overwritten, etc.

[jasonmolenda]

On armv7 architecture systems, there are 4 hardware breakpoint registers (DBGBVR) and 4 hardware watchpoint registers (DBGWVR). The breakpoint registers track values in the pc register. You can also say "stop when the PC is not equal to this value" which is how single instruction stepping is done on armv7.

On armv8 architecture systems, AArch32 or AArch64, there is a single instruction step bit (MDSCR_EL1, SS bit) to accomplish this more simply. There are still the dual sets of hardware breakpoint and hardware watchpoint registers -- but the hardware breakpoint registers don't have the "stop when the PC is not equal to this value" capability.

The kernel should have no problem distinguishing between an MDSCR_EL1 SS bit stop event and a watchpoint exception (on armv8 systems). On an armv7 system, it's possible to distinguish between a breakpoint and watchpoint event. And exhausting the watchpoint register file won't prevent stepping from working.

fwiw, lldb today doesn't use the hardware breakpoints, except for single stepping on armv7 systems. breakpoint set does have the --hardware option to request that the remote gdb stub use a hardware breakpoint if it has that capability. But lldb doesn't do that on its own today.

[labath]

Thank you, that was very enlightening. :)

source/Target/ThreadPlanStepOverBreakpoint.cpp

if (stop_info_sp)
         // So the lower levels fake "step onto breakpoint address" and return that as a breakpoint.  So our trace
         // step COULD appear as a breakpoint hit if the next instruction also contained a breakpoint.
         StopReason reason = stop_info_sp->GetStopReason();
 
         switch (reason)
         {
         case eStopReasonTrace:
         case eStopReasonNone:
+        // Some kernel reports the stop reason for a single step request as a hit of a hardware watchpoint
+        case eStopReasonWatchpoint:
             return true;
         case eStopReasonBreakpoint:
             // It's a little surprising that we stop here for a breakpoint hit.  However, when you single step ONTO a
             // breakpoint we still want to call that a breakpoint hit, and trigger the actions, etc.  Otherwise you
             // would see the PC at the breakpoint without having triggered the actions, then you'd continue, the PC
             // wouldn't change, and you'd see the breakpoint hit, which would be odd.
             // So the lower levels fake "step onto breakpoint address" and return that as a breakpoint hit.  So our trace
             // step COULD appear as a breakpoint hit if the next instruction also contained a breakpoint.  We don't want