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

[Matthew Gardiner]

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?

Index: source/Plugins/Process/POSIX/RegisterContextPOSIXProcessMonitor_x86.cpp
===================================================================
--- source/Plugins/Process/POSIX/RegisterContextPOSIXProcessMonitor_x86.cpp	(revision 201779)
+++ source/Plugins/Process/POSIX/RegisterContextPOSIXProcessMonitor_x86.cpp	(working copy)
@@ -503,15 +503,6 @@
  {
      bool is_hit = false;

-    if (m_watchpoints_initialized == false)
-    {
-        // Reset the debug status and debug control registers
-        RegisterValue zero_bits = RegisterValue(uint64_t(0));
-        if (!WriteRegister(m_reg_info.first_dr + 6, zero_bits) || !WriteRegister(m_reg_info.first_dr + 7, zero_bits))
-            assert(false && "Could not initialize watchpoint registers");
-        m_watchpoints_initialized = true;
-    }
-
      if (hw_index < NumSupportedHardwareWatchpoints())
      {
          RegisterValue value;
@@ -559,15 +550,6 @@

      assert(hw_index < NumSupportedHardwareWatchpoints());

-    if (m_watchpoints_initialized == false)
-    {
-        // Reset the debug status and debug control registers
-        RegisterValue zero_bits = RegisterValue(uint64_t(0));
-        if (!WriteRegister(m_reg_info.first_dr + 6, zero_bits) || !WriteRegister(m_reg_info.first_dr + 7, zero_bits))
-            assert(false && "Could not initialize watchpoint registers");
-        m_watchpoints_initialized = true;
-    }
-
      if (ReadRegister(m_reg_info.first_dr + 7, value))
      {
          uint64_t val = value.GetAsUInt64();


thanks
Matthew Gardiner


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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: RegisterContextPOSIXProcessMonitor_x86.cpp.diff
Type: text/x-patch
Size: 1527 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20140221/bd141683/attachment.bin>