[Lldb-commits] [lldb] r225418 - Fix a problem where a ValueObject could fail to update itself, but since it was previously valid, we'd have an old checksum to compare aginst no new checksum (because failure to update), and assert() and die. Fix the problem by only caring about this assertion logic if updates succeed

[Zachary Turner]

BTW, I'm not sure if it helps you reproduce this issue at all, but I
debugged into why it was failing to update, and it looks like a pointer
value that doesn't map to any section.  Obviously there's another bug
causing that which I have yet to understand the cause of, but thought you
might be curious.

On Thu Jan 08 2015 at 10:56:12 AM Enrico Granata <egranata at apple.com> wrote:

> On second thought, yes, that patch is a total think-o (right idea, wrong
> fix)..
>
> It should probably just say
>
> if (success)
>  assert(what you said)
>
> Better patch incoming.. and I wonder if we can in any way force ourselves
> into this scenario to test the sanity of any change in this area. Let me
> think about that
>
> On Jan 8, 2015, at 10:45 AM, Zachary Turner <zturner at google.com> wrote:
>
> Ok so it looks like your patch doesn't fix the assert for me, but now
> looking more closely at your patch, I wonder if it's correct.  As per the
> description, the purpose of your patch is to fix the case where success ==
> false (which is what's happening to me), but you are adding a requirement
> to the assert that success be true.  Shouldn't this be something like the
> following:
>
> assert (!need_compare_checksums || (!old_checksum.empty() &&
> !m_value_checksum.empty()));
>
>
>
> On Wed Jan 07 2015 at 5:50:44 PM Zachary Turner <zturner at google.com>
> wrote:
>
>> It just happens every time i run one of the tests on windows. It might be
>> triggered because something earlier is broken, but I'll send you a stack
>> trace or something tomorrow. Won't it be nice if/when we can debug windows
>> core dumps from Mac? :P
>> On Wed, Jan 7, 2015 at 5:21 PM Enrico Granata <egranata at apple.com> wrote:
>>
>>> If you end up with a reproducible case of the assertion firing, totally
>>> let me know - bugzilla or just an email
>>> Hopefully it’s not so Windows-specific that I can’t get it to happen on
>>> Mac
>>>
>>> On Jan 7, 2015, at 4:33 PM, Zachary Turner <zturner at google.com> wrote:
>>>
>>> Cool, i think this assertion was actually firing on windows, making it
>>> very annoying to run the test suite. I hope this fixes that
>>> On Wed, Jan 7, 2015 at 4:30 PM Enrico Granata <egranata at apple.com>
>>> wrote:
>>>
>>>> Author: enrico
>>>> Date: Wed Jan  7 18:29:12 2015
>>>> New Revision: 225418
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=225418&view=rev
>>>> Log:
>>>> Fix a problem where a ValueObject could fail to update itself, but
>>>> since it was previously valid, we'd have an old checksum to compare aginst
>>>> no new checksum (because failure to update), and assert() and die. Fix the
>>>> problem by only caring about this assertion logic if updates succeed
>>>>
>>>> Modified:
>>>>     lldb/trunk/source/Core/ValueObject.cpp
>>>>
>>>> Modified: lldb/trunk/source/Core/ValueObject.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/
>>>> ValueObject.cpp?rev=225418&r1=225417&r2=225418&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- lldb/trunk/source/Core/ValueObject.cpp (original)
>>>> +++ lldb/trunk/source/Core/ValueObject.cpp Wed Jan  7 18:29:12 2015
>>>> @@ -250,7 +250,7 @@ ValueObject::UpdateValueIfNeeded (bool u
>>>>                  m_value_checksum.clear();
>>>>              }
>>>>
>>>> -            assert (old_checksum.empty() == !need_compare_checksums);
>>>> +            assert (success && (old_checksum.empty() ==
>>>> !need_compare_checksums));
>>>>
>>>>              if (first_update)
>>>>                  SetValueDidChange (false);
>>>>
>>>>
>>>> _______________________________________________
>>>> lldb-commits mailing list
>>>> lldb-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>>>>
>>>
>>> Thanks,
>>> *- Enrico*
>>> 📩 egranata@.com ☎️ 27683
>>>
>>>
>>>
>>>
>>>
> Thanks,
> *- Enrico*
> 📩 egranata@.com ☎️ 27683
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20150109/ba246cde/attachment.html>