[LLDB] Fix display of value of a vector variables in watchpoint operations

[mohit.bhakkad]

Summary

Consider a vector variable 'v8i16 s0'

Right now if we print value of s0, it gives us proper value:
(lldb) print s0
(v8i16) $0 = (member 1, member 2, ........,member 8)

But if we try to set a watchpoint on it, it shows null for its value:

(lldb) watchpoint set variable s0
Watchpoint created: Watchpoint 1: addr = <addr> size = 16 state = enabled type = w

declare @ 'file_name:line_no'
watchpoint spec = 's0'
new value: (null)

Approach used in patch is already used in in function ValueObjectPrinter::GetValueSummaryError,
which is called for command 'print s0'.

[zturner]

Can you find a way to add a test for this?

[mohit.bhakkad]

Adding a simple test to check displayed value of vector

[zturner]

Looks good to me. Just add a decorator to both tests to skip unless the compiler is gcc or clang ( __attribute((vector_size))__ doesn't work on MSVC or clang-cl, for example

[zturner]

Actually I'm wrong. Leave it enabled and I'll see what happens. clang-cl (which we require for windows tests) supports that syntax after all.

[clayborg]

Back from vacation, sorry for the delay.

One quick fix as noted in inlined comments.

source/Breakpoint/Watchpoint.cpp

224–235 ↗

(On Diff #35879)

It would be nice to store these in local variables and clean up the code a bit like:

const char *old_value_cstr =  m_old_value_sp->GetValueAsCString();
if (old_value_cstr && old_value_cstr[0])
    s->Printf("\n%sold value: %s", prefix, old_value_cstr);
else
{
    const char *old_summary_cstr =  m_old_value_sp-> GetSummaryAsCString();
    if (old_summary_cstr && old_summary_cstr[0])
        s->Printf("\n%sold value: %s", prefix, old_summary_cstr);
}

This way we don't ever print "old value: " when there is no value or summary...

[mohit.bhakkad]

Changing as suggested

[clayborg]

Looks good.

[loladiro]

Shouldn't that test have been inside the test/ folder?

[zturner]

Strange, it was fine in the review, but wrong in the commit. Yea, that
needs to be fixed.

[mohit.bhakkad]

In D13202#263841, @zturner wrote:

Strange, it was fine in the review, but wrong in the commit. Yea, that
needs to be fixed.

Yes it looks strange, I directly applied the patch from review, and it looks like new files are added directly to the trunk.
Mistake from my side is I didn't checked changes before committing, as patch got applied successfully without any
issues. I will be careful from next time.

[mohit.bhakkad]

@loladiro @zturner Thanks for pointing this out, fixed it in rL249897

lldb/trunk/source/Breakpoint/Watchpoint.cpp

 void
 Watchpoint::DumpSnapshots(Stream *s, const char *prefix) const
 {
     if (!prefix)
     {
         s->Printf("\nWatchpoint %u hit:", GetID());
         prefix = "";
     }
-    
     if (m_old_value_sp)
     {
-        s->Printf("\n%sold value: %s", prefix, m_old_value_sp->GetValueAsCString());
+        const char *old_value_cstr =  m_old_value_sp->GetValueAsCString();
+        if (old_value_cstr && old_value_cstr[0])
+            s->Printf("\n%sold value: %s", prefix, old_value_cstr);
+        else
+        {
+            const char *old_summary_cstr =  m_old_value_sp-> GetSummaryAsCString();
+            if (old_summary_cstr && old_summary_cstr[0])
+                s->Printf("\n%sold value: %s", prefix, old_summary_cstr);
         }
+    }
+
     if (m_new_value_sp)
     {
-        s->Printf("\n%snew value: %s", prefix, m_new_value_sp->GetValueAsCString());
+        const char *new_value_cstr =  m_new_value_sp->GetValueAsCString();
+        if (new_value_cstr && new_value_cstr[0])
+            s->Printf("\n%snew value: %s", prefix, new_value_cstr);
+        else
+        {
+            const char *new_summary_cstr =  m_new_value_sp-> GetSummaryAsCString();
+            if (new_summary_cstr && new_summary_cstr[0])
+                s->Printf("\n%snew value: %s", prefix, new_summary_cstr);
+        }
     }
 }
 
 void
 Watchpoint::DumpWithLevel(Stream *s, lldb::DescriptionLevel description_level) const
 {
     if (s == NULL)
         return;

lldb/trunk/watchpoint/watchpoint_on_vectors/Makefile

+LEVEL = ../../../make
+
+C_SOURCES := main.c
+
+include $(LEVEL)/Makefile.rules

lldb/trunk/watchpoint/watchpoint_on_vectors/TestValueOfVectorVariable.py

+"""
+Test displayed value of a vector variable while doing watchpoint operations
+"""
+
+import os, time
+import unittest2
+import lldb
+from lldbtest import *
+import lldbutil
+
+class TestValueOfVectorVariableTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    @dsym_test
+    def test_value_of_vector_variable_with_dsym_using_watchpoint_set(self):
+        """Test verify displayed value of vector variable."""
+        self.buildDsym(dictionary=self.d)
+        self.setTearDownCleanup(dictionary=self.d)
+        self.value_of_vector_variable_with_watchpoint_set()
+
+    @dwarf_test
+    def test_value_of_vector_variable_with_dwarf_using_watchpoint_set(self):
+        """Test verify displayed value of vector variable."""
+        self.buildDwarf(dictionary=self.d)
+        self.setTearDownCleanup(dictionary=self.d)
+        self.value_of_vector_variable_with_watchpoint_set()
+    
+    def setUp(self):
+        # Call super's setUp().
+        TestBase.setUp(self)
+        # Our simple source filename.
+        self.source = 'main.c'
+        self.exe_name = 'a.out'
+        self.d = {'C_SOURCES': self.source, 'EXE': self.exe_name}
+
+    def value_of_vector_variable_with_watchpoint_set(self):
+        """Test verify displayed value of vector variable"""
+        exe = os.path.join(os.getcwd(), 'a.out')
+        self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+        # Set break to get a frame
+        self.runCmd("b main")
+            
+        # Run the program.
+        self.runCmd("run", RUN_SUCCEEDED)
+
+        # Value of a vector variable should be displayed correctly
+        self.expect("watchpoint set variable global_vector", WATCHPOINT_CREATED,
+            substrs = ['new value: (1, 2, 3, 4, 5, 6, 7, 8)'])
+
+if __name__ == '__main__':
+    import atexit
+    lldb.SBDebugger.Initialize()
+    atexit.register(lambda: lldb.SBDebugger.Terminate())
+    unittest2.main()

lldb/trunk/watchpoint/watchpoint_on_vectors/main.c

+//===-- main.c --------------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+typedef short v8i16 __attribute__ ((vector_size(16)));
+v8i16 global_vector = {1, 2, 3, 4, 5, 6, 7, 8};
+
+int main()
+{
+}