This patch fixes my think-o in ValueObject::UpdateValueIfNeeded() about the right thing to assert()

It also comes with a (rudimentary) test case that gets itself in a failed update scenario, and checks that we don't crash
This is the easiest case I could think of that forces the failed update case Zachary was seeing



git-svn-id: https://llvm.org/svn/llvm-project/lldb/trunk@225463 91177308-0d34-0410-b5e6-96231b3b80d8

4 files changed, 101 insertions, 2 deletions

diff --git a/source/Core/ValueObject.cpp b/source/Core/ValueObject.cpp
index 25219e574..fa5fb14db 100644
--- a/source/Core/ValueObject.cpp
+++ b/source/Core/ValueObject.cpp
@@ -250,8 +250,8 @@ ValueObject::UpdateValueIfNeeded (bool update_format)
                 m_value_checksum.clear();
             }
             
-            assert (success && (old_checksum.empty() == !need_compare_checksums));
-            
+            assert (!need_compare_checksums || (!old_checksum.empty() && !m_value_checksum.empty()));
+
             if (first_update)
                 SetValueDidChange (false);
             else if (!m_value_did_change && success == false)
diff --git a/test/functionalities/value_md5_crash/Makefile b/test/functionalities/value_md5_crash/Makefile
new file mode 100644
index 000000000..8a7102e34
--- /dev/null
+++ b/test/functionalities/value_md5_crash/Makefile
@@ -0,0 +1,5 @@
+LEVEL = ../../make
+
+CXX_SOURCES := main.cpp
+
+include $(LEVEL)/Makefile.rules
diff --git a/test/functionalities/value_md5_crash/TestValueMD5Crash.py b/test/functionalities/value_md5_crash/TestValueMD5Crash.py
new file mode 100644
index 000000000..ad0e309dd
--- /dev/null
+++ b/test/functionalities/value_md5_crash/TestValueMD5Crash.py
@@ -0,0 +1,65 @@
+"""
+Verify that the hash computing logic for ValueObject's values can't crash us.
+"""
+
+import os, time
+import unittest2
+import lldb
+from lldbtest import *
+import lldbutil
+
+class ValueMD5CrashTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    @unittest2.skipUnless(sys.platform.startswith("darwin"), "requires Darwin")
+    @dsym_test
+    def test_with_dsym_and_run_command(self):
+        """Verify that the hash computing logic for ValueObject's values can't crash us."""
+        self.buildDsym()
+        self.doThings()
+
+    @dwarf_test
+    def test_with_dwarf_and_run_command(self):
+        """Verify that the hash computing logic for ValueObject's values can't crash us."""
+        self.buildDwarf()
+        self.doThings()
+
+    def setUp(self):
+        # Call super's setUp().
+        TestBase.setUp(self)
+        # Find the line number to break at.
+        self.line = line_number('main.cpp', '// break here')
+
+    def doThings(self):
+        """Verify that the hash computing logic for ValueObject's values can't crash us."""
+        self.runCmd("file a.out", CURRENT_EXECUTABLE_SET)
+
+        lldbutil.run_break_set_by_file_and_line (self, "main.cpp", self.line, num_expected_locations=1, loc_exact=True)
+
+        self.runCmd("run", RUN_SUCCEEDED)
+
+        # The stop reason of the thread should be breakpoint.
+        self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+            substrs = ['stopped',
+                       'stop reason = breakpoint'])
+
+        value = self.frame().FindVariable("a")
+        value.SetPreferDynamicValue(lldb.eDynamicCanRunTarget)
+        
+        v = value.GetValue()
+        self.assertTrue(value.GetTypeName() == "B *", "a is a B*")
+        
+        self.runCmd("next")
+        self.runCmd("process kill")
+        
+        # now the process is dead, and value needs updating
+        v = value.GetValue()
+        
+        # if we are here, instead of crashed, the test succeeded
+
+if __name__ == '__main__':
+    import atexit
+    lldb.SBDebugger.Initialize()
+    atexit.register(lambda: lldb.SBDebugger.Terminate())
+    unittest2.main()
diff --git a/test/functionalities/value_md5_crash/main.cpp b/test/functionalities/value_md5_crash/main.cpp
new file mode 100644
index 000000000..ba596b865
--- /dev/null
+++ b/test/functionalities/value_md5_crash/main.cpp
@@ -0,0 +1,29 @@
+//===-- main.cpp ------------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+class A {
+public:
+    virtual int foo() { return 1; }
+    virtual ~A () = default;
+    A() = default;
+};
+
+class B : public A {
+public:
+    virtual int foo() { return 2; }
+    virtual ~B () = default;
+    B() = default;
+};
+
+int main() {
+    A* a = new B();
+    a->foo();  // break here
+    return 0;  // break here
+}
+