[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

[Enrico Granata]

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 <mailto: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 <mailto: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 <mailto: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 <mailto: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 <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 <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 <mailto:lldb-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits <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/20150108/bf9005b4/attachment.html>