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

[John Wolfe]

On 2/24/2014 4:19 AM, Matthew Gardiner wrote:
> John Wolfe wrote:
>> 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.
> I appreciate that if hardware breakpoints are being used in a debugger 
> then dr6/7 must be serviced. However, I have written an intel i386 
> linux debugger previously, which implements execution breakpoints 
> purely by inserting 0xCC (BREAK) instructions into the code. It was 
> possible for the debugger to be ignorant of dr6/7 - and require no 
> initialization of those registers.

While the debug registers can be used for instruction break, lldb is 
primarily using them for data watchpoints.  Because the 
SIGTRAP/TRAP_TRACE handling routine must also check for a possible 
hardware (data) breakpoint and the initial action of tracing 
(controlling) another process will result in a trace signal, the 
initialization of dr6 and dr7
to indicate that there no existing data watchpoints is done on the first 
access.

>>
>> 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.
> Agreed. Given that lldb does use HW breaks, then yes, 
> IsWatchPointHit() should do an initial read of the status register, 
> then clear any flags. Given what I understand of page 707 of 
> http://download.intel.com/design/processor/manuals/253668.pdf, I'm not 
> sure about write of zero_bits... the document details a non-trivial 
> mask for dr6...

While it looks like a non-trivial mask, these are "reserved" bits and 
the processor will force/return these to be 1.

>
>>
>> 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.
> On face of it, I think you're right. It's strange though, that in the 
> 32-bit linux debugger, the read back of 0x118 from dr6 seems to vanish 
> when those zero-bit writes are removed.

The key word here is "seems".   By removing the code  and "zero_bits" 
local variable used to write a zero value to d6/7 in IsWatchpointHit(), 
you have potentially changed the location of the "RegisterValue value" 
on the stack.  Being uninitialized, if the ReadRegister() fails to 
return the register contents, you will be seeing a garbage value.
Different locations, potentially different garbage values.

A test with "value" initialized with a known bad value such as 0x0bad 
could confirm or refute this possibility.

>
>>
>> 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.
> Yes, again I agree. There could be another issue here.
>>
>> 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.
> Thanks for this bit - yes ptrace logging is appreciated. I pursue that 
> route for now. Unfortunately ptrace logging is pretty flakey , 
> currently. This was what I see in simple "lldb hello" debug session:
>
> operation ptrace(PTRACE_TRACEME, 0, (nil), (nil), 0) called from file 
> (null) line -1220763056
> operation ptrace(PTRACE_SETOPTIONS, 7972, (nil), (nil), 88) called 
> from file (null) line -1220763056
> operation ptrace(PTRACE_PEEKDATA, 7972, (nil), 0x8048340, 0) called 
> from file (null) line -1220763056
> operation ptrace() failed; errno=5 (<unknown>)
> operation ptrace(PTRACE_POKEUSER, 7972, (nil), 0x114, 4294967295) 
> called from file (null) line -1220763056
> operation ptrace(PTRACE_POKEUSER, 7972, (nil), 0x118, 4294967295) 
> called from file (null) line -1220763056
> operation ptrace(PTRACE_PEEKUSER, 7972, (nil), 0x114, 0) called from 
> file (null) line -1220763056
> operation ptrace(PTRACE_PEEKUSER, 7972, (nil), 0x114, 0) called from 
> file (null) line -1220763056
> operation ptrace(PTRACE_PEEKUSER, 7972, (nil), 0x114, 0) called from 
> file (null) line -1220763056
> operation ptrace(PTRACE_PEEKUSER, 7972, (nil), 0x114, 0) called from 
> file (null) line -1220763056
> operation ptrace(PTRACE_PEEKUSER, 7972, (nil), 0x114, 0) called from 
> file (null) line -1220763056
> operation ptrace(PTRACE_PEEKUSER, 7972, (nil), 0x114, 0) called from 
> file (null) line -1220763056
> operation ptrace(PTRACE_PEEKUSER, 7972, (nil), 0x114, 0) called from 
> file (null) line -1220763056
> operation ptrace(PTRACE_PEEKUSER, 7972, (nil), 0x114, 0) called from 
> file (null) line -1220763056
> operation ptrace(PTRACE_POKEUSER, 7972, (nil), 0x114, 4294967295) 
> called from file (null) line -1220763056
> operation ptrace(PTRACE_PEEKUSER, 7972, (nil), 0x118, 0) called from 
> file (null) line -1220763056
>
> The file/line data is wrong and the all-above return value for the 
> peeks is absent...
>
> Matt
>
>
>
>
>
> Member of the CSR plc group of companies. CSR plc registered in 
> England and Wales, registered number 4187346, registered office 
> Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 
> 0WZ, United Kingdom
> More information can be found at www.csr.com. Keep up to date with CSR 
> on our technical blog, www.csr.com/blog, CSR people blog, 
> www.csr.com/people, YouTube, www.youtube.com/user/CSRplc, Facebook, 
> www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at 
> www.twitter.com/CSR_plc.
> New for 2014, you can now access the wide range of products powered by 
> aptX at www.aptx.com.