[Lldb-commits] [PATCH] Use 32-bit RegisterValue object on IsWatchpoint functions for 32-bit linux

[Matthew Gardiner]

Folks,

An issue currently exists for 32-bit linux such that when lldb receives the
first SIGTRAP, an assertion failure occurs in POSIXThread::WatchNotify. This
failure is due to 0x118 being read back from dr6, but with no watchpoints set.

In trying to trace this I have discovered that IsWatchpointHit/Vacant both
create a RegiaterValue object thusly:

RegisterValue zero_bits = RegisterValue(uint64_t(0));

and then issue a write request. When this write is handled
(Linux/ProcessMonitor.cpp) we coerce this value into a 32-bit int as follows:

#if __WORDSIZE == 32
     buf = (void*) m_value.GetAsUInt32();

This is problematic since RegisterValue (deliberately?) returns "fail_value"
for this call if m_type==eTypeUInt64.

Without really trying to analyse the design of RegisterValue, I think an
expedient fix is to conditionally compile 32-bit lldb to use a 32-bit type
for it's "RegisterValue zero_bits". Yes, the Watchpoint assert failure still
happens, however, at least now we are actually writing what we expect.

Would somebody please apply the following patch which implements the
proposed fix?

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)
@@ -506,7 +506,12 @@
      if (m_watchpoints_initialized == false)
      {
          // Reset the debug status and debug control registers
+#ifdef __x86_64__
          RegisterValue zero_bits = RegisterValue(uint64_t(0));
+#else
+        RegisterValue zero_bits = RegisterValue(uint32_t(0));
+#endif
+
          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;
@@ -562,7 +567,11 @@
      if (m_watchpoints_initialized == false)
      {
          // Reset the debug status and debug control registers
+#ifdef __x86_64__
          RegisterValue zero_bits = RegisterValue(uint64_t(0));
+#else
+        RegisterValue zero_bits = RegisterValue(uint32_t(0));
+#endif
          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;

(Possibly a better fix, would be determining at run-time the target application
architecture, therefore permitting 64-bit lldb to debug 32-bit apps.)

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