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

[Todd Fiala]

Thanks Matt.

Right now I'm going to try what I think are the last two patches you had
put up since we last chatted (my last night).


On Tue, Mar 4, 2014 at 12:47 AM, Matthew Gardiner <mg11 at csr.com> wrote:

> 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.
>



-- 
Todd Fiala | Software Engineer | tfiala at google.com | 650-943-3180
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20140304/584dc98e/attachment.html>