[lldb-dev] [PATCH] Remove unnecessary writing to dr6/dr7 on linux

[John Wolfe]

Matthew,

These code segments are not "unnecessary" writes of dr6/dr7.   They are 
initialization of the debug status and control registers (dr6/dr7) and 
intended to be executed only once - when hardware breakpoints are first 
set or the status checked.  The Intel manuals states clearly that the 
DSR fields must be cleared as hardware breakpoints are serviced and 
nowhere does it state that dr6 or dr7 are guaranteed to be "initialized" 
at any time.

The check for DSR/DCR initialization is obviously needed when the the 
first attempt to set a watchpoint is attempted.    It is also needed in 
IsWatchPointHit() to handle OS variations in signaling a hardware 
breakpoint.  On Linux, at least more recent versions, a 
SIGTRAP/TRAP_HWBKPT clearly identifies the hardware breakpoint.  On 
others, such as FreeBSD which has no TRAP_HWBKPT, the break is signaled 
with a SIGTRAP/TRAP_TRACE.   That is why the POSIXThread::TraceNotify()  
does a check to determine if this trace trap is actually for a watchpoint.

Because a SIGTRAP/TRAP_TRACE signal is sent when the debugger starts and 
a.out or attaches to a process, this is likely to be the point where DSR 
and DCR are initialized and the first check whether a watchpoint has 
been hit.

So, these initialization of dr6 and dr7 are needed on the x86 
architectures and I do not think they are the source of your problems.   
If you have just written zero to dr6, how can a read of dr6 return the 
value of 0x118?  If this is at the time of the initial 
SIGTRAP/TRAP_TRACE, then the register reads/writes would be to the 
process context area.  If the process execution had been started, then 
the reserved bits (always 1) in dr6 should have resulted in a dr6 value
with the 0xffff0ff0 bits set.

I suspect that there is still something wrong in the the write and or 
the read of the debug registers.   The register read/write routines are 
not good about reporting any errors and many uses simply assume that the 
value returned is valid.    I encountered something similar when 
implementing watchpoints on FreeBSD.   In my case the read of dr7 failed 
and I was looking at bits in an local RegisterValue.

I found it helpful to add debugging/logging code in 
FreeBSD/ProcessMonitor.cpp to log the debug register write/read's under 
control of POSIX_LOG_PTRACE.    If you do something similar in the Linux 
version, then

       log enable -f ptrace.log linux ptrace

might shed some light on what and where things may be going wrong.

Hope that helps.

-- John Wolfe
> ------------------- Original Message --------------------
> Date: Fri, 21 Feb 2014 09:42:26 +0000
> From: Matthew Gardiner <mg11 at csr.com>
> To: "lldb-dev at cs.uiuc.edu" <lldb-dev at cs.uiuc.edu>
> Subject: [lldb-dev] [PATCH] Remove unnecessary writing to dr6/dr7 on
> 	linux
> Message-ID: <53071F82.8070006 at csr.com>
> Content-Type: text/plain; charset="iso-8859-1"; Format="flowed"
>
> Folks,
>
> My first two weeks of playing with lldb on 32-bit linux has been
> blighted by the Watchpoint notify failed assertion bug:
>
>    $ lldb hello
> Current executable set to 'hello' (i386).
> (lldb) run
> Process 421 launching
> lldb: /home/mg11/src/heracles2/llvm/tools/lldb/source/Plugins/Process/POSIX/POSIXThread.cpp:514: void POSIXThread::WatchNotify(const ProcessMessage&): Assertion `wp_sp.get() && "No watchpoint found"'
> failed.
> Aborted (core dumped)
>
> After firstly discovering that the x86_64 register map was being used for
> 32-bit linux, I eventually have discovered that this bug occurs due to
> unnecessary writes to dr6 and dr7, in IsWatchpointHit and
> IsWatchpointVacant from RegisterContextPOSIXProcessMonitor_x86.cpp. (I also
> found that the RegisterValue::GetAsXXX functions, in general, return fail_value
> when queried for a smaller integral type than that used in the constructor. But
> that's another story...). Those writes result in dr6 subsequently reading back
> as 0x118, which results in breakpoint detection but with no data in wp_sp, and
> hence the assertion failure.
>
> So is there a good reason these writes? I've read the relevant section of the
> intel manual and I can't find any justification.
>
> Removing the writes, removes the assertion failure. Please could somebody
> consider this applying patch, which removes them - or justify the existence of
> the writes?
>
>