[Lldb-commits] [lldb] r225463 - This patch fixes my think-o in ValueObject::UpdateValueIfNeeded() about the right thing to assert()

[Enrico Granata]

Author: enrico
Date: Thu Jan  8 13:11:43 2015
New Revision: 225463

URL: http://llvm.org/viewvc/llvm-project?rev=225463&view=rev
Log:
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


Added:
    lldb/trunk/test/functionalities/value_md5_crash/
    lldb/trunk/test/functionalities/value_md5_crash/Makefile
    lldb/trunk/test/functionalities/value_md5_crash/TestValueMD5Crash.py
    lldb/trunk/test/functionalities/value_md5_crash/main.cpp
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=225463&r1=225462&r2=225463&view=diff
==============================================================================
--- lldb/trunk/source/Core/ValueObject.cpp (original)
+++ lldb/trunk/source/Core/ValueObject.cpp Thu Jan  8 13:11:43 2015
@@ -250,8 +250,8 @@ ValueObject::UpdateValueIfNeeded (bool u
                 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)

Added: lldb/trunk/test/functionalities/value_md5_crash/Makefile
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/value_md5_crash/Makefile?rev=225463&view=auto
==============================================================================
--- lldb/trunk/test/functionalities/value_md5_crash/Makefile (added)
+++ lldb/trunk/test/functionalities/value_md5_crash/Makefile Thu Jan  8 13:11:43 2015
@@ -0,0 +1,5 @@
+LEVEL = ../../make
+
+CXX_SOURCES := main.cpp
+
+include $(LEVEL)/Makefile.rules

Added: lldb/trunk/test/functionalities/value_md5_crash/TestValueMD5Crash.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/value_md5_crash/TestValueMD5Crash.py?rev=225463&view=auto
==============================================================================
--- lldb/trunk/test/functionalities/value_md5_crash/TestValueMD5Crash.py (added)
+++ lldb/trunk/test/functionalities/value_md5_crash/TestValueMD5Crash.py Thu Jan  8 13:11:43 2015
@@ -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()

Added: lldb/trunk/test/functionalities/value_md5_crash/main.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/value_md5_crash/main.cpp?rev=225463&view=auto
==============================================================================
--- lldb/trunk/test/functionalities/value_md5_crash/main.cpp (added)
+++ lldb/trunk/test/functionalities/value_md5_crash/main.cpp Thu Jan  8 13:11:43 2015
@@ -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
+}
+