[lldb-dev] 32-bit linux IsWatchpointHit assertion failure due to WriteRegister

[Matthew Gardiner]

Todd Fiala wrote:
> So I tried this patch:
>
> tfiala at ubuntu:~/lldb/git/llvm/tools/lldb/source/Plugins$ git diff
> diff --git a/source/Plugins/Process/POSIX/RegisterContextLinux_i386.cpp b/source
> index f2c1bab..72f8838 100644
> --- a/source/Plugins/Process/POSIX/RegisterContextLinux_i386.cpp
> +++ b/source/Plugins/Process/POSIX/RegisterContextLinux_i386.cpp
> @@ -83,8 +83,10 @@ struct UserArea
>  };
>  #define DR_SIZE sizeof(UserArea::u_debugreg[0])
> -#define DR_OFFSET(reg_index) \
> -    (LLVM_EXTENSION offsetof(UserArea, u_debugreg[reg_index]))
> +
> +// FIXME: remove the following code as soon as we get the UserArea structure fi
> +#define DR_OFFSET(reg_index) (0xFC + (reg_index * 4))
> +// #define DR_OFFSET(reg_index)        (LLVM_EXTENSION offsetof(UserArea, u_deb
>  #define FPR_SIZE(reg) sizeof(((FPR_i386*)NULL)->reg)
>  //---------------------------------------------------------------------------
>
> But I'm still getting the assertion.  I did put an #error in the file to make sure I wasn't somehow not building it.
Yes. That's a different (at least that's how I see it) assertion. It occurs because:

bool
RegisterContextPOSIXProcessMonitor_x86_64::IsWatchpointHit(uint32_t hw_index)
{
<snip>
         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))

doesn't write 0. It writes 0xffffffff. Which results in all breakpoint indexes being set but no watchpoints in lldb structures, hence the assert.

That's due to a combination of:

void
WriteRegOperation::Execute(ProcessMonitor *monitor)
{
<snip>
#if __WORDSIZE == 32
     buf = (void*) m_value.GetAsUInt32();
#else
     buf = (void*) m_value.GetAsUInt64();
#endif

and

uint32_t
RegisterValue::GetAsUInt32 (uint32_t fail_value, bool *success_ptr) const
{
<snip>
     switch (m_type)
     {
         default:            break;
         case eTypeUInt8:    return m_data.uint8;
         case eTypeUInt16:   return m_data.uint16;
         case eTypeUInt32:   return m_data.uint32;
         case eTypeFloat:
             if (sizeof(float) == sizeof(uint32_t))
                 return m_data.uint32;
             break;
<snip>
     return fail_value;
}

but I have a sweet fix for this, which doesn't compromise the design of RegisterValue. With this fix and the correct i386 register map, I think we're looking good for 32-bit linux.

>
> I see where my wires crossed when reading the user.h header.
No sweat ;-)
> On your question re: copying the structure:
> this code has to compile in places other than i386 Linux (i.e. we can debug i386 Linux code on other hosts, that don't have the user.h available, or have the wrong one available).
Yeah, I appreciate that. You can't rely on the presence of the "right" user.h. To calculate the offsets you'd need to either i) copy the structures or ii) hard-code the offsets. My contention isn't 
over the *copying*, it's over the usage of structures, when they're not also being used as structures. I think if the actual offsets e.g.

...
const uint32_t kDebugRegister6_Offset = 0x114;
const uint32_t kDebugRegister7_Offset = 0x118;
...

are crafted, then we have more robust code, since breakage would require people deliberately changing a number, instead of a more subtle structure edit. Anyway, that's just my opinion.

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.