When trying to take snapshots of a watched variable, if the frame is unable to evaluate the variable expression,

do not take the sanpshot and forget about the stop info.  It is possible that the variable expression has gone
out of scope, we'll revise the hit count due to the false alarms.

llvm-svn: 161892

6 files changed, 152 insertions, 8 deletions

diff --git a/lldb/include/lldb/Breakpoint/Watchpoint.h b/lldb/include/lldb/Breakpoint/Watchpoint.h
index 0f11b30811c..2b72a2fd4fe 100644
--- a/lldb/include/lldb/Breakpoint/Watchpoint.h
+++ b/lldb/include/lldb/Breakpoint/Watchpoint.h
@@ -36,6 +36,9 @@ public:
 
     ~Watchpoint ();
 
+    void
+    IncrementFalseAlarmsAndReviseHitCount();
+
     bool
     IsEnabled () const;
 
@@ -162,7 +165,8 @@ private:
                 m_watch_write:1,       // 1 if we stop when the watched data is written to
                 m_watch_was_read:1,    // Set to 1 when watchpoint is hit for a read access
                 m_watch_was_written:1; // Set to 1 when watchpoint is hit for a write access
-    uint32_t    m_ignore_count;        // Number of times to ignore this breakpoint
+    uint32_t    m_ignore_count;        // Number of times to ignore this watchpoint
+    uint32_t    m_false_alarms;        // Number of false alarms.
     std::string m_decl_str;            // Declaration information, if any.
     std::string m_watch_spec_str;      // Spec for the watchpoint.
     std::string m_snapshot_old_str;    // Old snapshot for the watchpoint value as by ValueObject::DumpValueObject().
diff --git a/lldb/source/Breakpoint/Watchpoint.cpp b/lldb/source/Breakpoint/Watchpoint.cpp
index 6ba2f9c89f4..d4c099be0c1 100644
--- a/lldb/source/Breakpoint/Watchpoint.cpp
+++ b/lldb/source/Breakpoint/Watchpoint.cpp
@@ -34,6 +34,7 @@ Watchpoint::Watchpoint (lldb::addr_t addr, size_t size, bool hardware) :
     m_watch_was_read(0),
     m_watch_was_written(0),
     m_ignore_count(0),
+    m_false_alarms(0),
     m_decl_str(),
     m_watch_spec_str(),
     m_snapshot_old_str(),
@@ -183,6 +184,25 @@ Watchpoint::SetWatchVariable(bool val)
     m_is_watch_variable = val;
 }
 
+void
+Watchpoint::IncrementFalseAlarmsAndReviseHitCount()
+{
+    ++m_false_alarms;
+    if (m_false_alarms)
+    {
+        if (m_hit_count >= m_false_alarms)
+        {
+            m_hit_count -= m_false_alarms;
+            m_false_alarms = 0;
+        }
+        else
+        {
+            m_false_alarms -= m_hit_count;
+            m_hit_count = 0;
+        }
+    }
+}
+
 // RETURNS - true if we should stop at this breakpoint, false if we
 // should continue.
 
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 5732ed66cc1..434b0067712 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -498,6 +498,7 @@ public:
             StackFrame *frame = exe_ctx.GetFramePtr();
             if (frame)
             {
+                bool snapshot_taken = true;
                 if (!wp_sp->IsWatchVariable())
                 {
                     // We are not watching a variable, just read from the process memory for the watched location.
@@ -535,18 +536,29 @@ public:
                         wp_sp->SetNewSnapshot(ss.GetString());
                     }
                     else
-                        wp_sp->SetNewSnapshot("snapshot attempt failed.");
+                    {
+                        // The variable expression has become out of scope?
+                        // Let us forget about this stop info.
+                        if (log)
+                            log->Printf("Snapshot attempt failed.  Variable expression has become out of scope?");
+                        snapshot_taken = false;
+                        m_should_stop = false;
+                        wp_sp->IncrementFalseAlarmsAndReviseHitCount();
+                    }
 
-                    if (log)
+                    if (log && snapshot_taken)
                         log->Printf("Watchpoint snapshot taken: '%s'\n", wp_sp->GetNewSnapshot().c_str());
                 }
 
                 // Now dump the snapshots we have taken.
-                Debugger &debugger = exe_ctx.GetTargetRef().GetDebugger();
-                StreamSP output_sp = debugger.GetAsyncOutputStream ();
-                wp_sp->DumpSnapshots(output_sp.get());
-                output_sp->EOL();
-                output_sp->Flush();
+                if (snapshot_taken)
+                {
+                    Debugger &debugger = exe_ctx.GetTargetRef().GetDebugger();
+                    StreamSP output_sp = debugger.GetAsyncOutputStream ();
+                    wp_sp->DumpSnapshots(output_sp.get());
+                    output_sp->EOL();
+                    output_sp->Flush();
+                }
             }
 
             if (m_should_stop && wp_sp->GetConditionText() != NULL)
diff --git a/lldb/test/functionalities/watchpoint/variable_out_of_scope/Makefile b/lldb/test/functionalities/watchpoint/variable_out_of_scope/Makefile
new file mode 100644
index 00000000000..b09a579159d
--- /dev/null
+++ b/lldb/test/functionalities/watchpoint/variable_out_of_scope/Makefile
@@ -0,0 +1,5 @@
+LEVEL = ../../../make
+
+C_SOURCES := main.c
+
+include $(LEVEL)/Makefile.rules
diff --git a/lldb/test/functionalities/watchpoint/variable_out_of_scope/TestWatchedVarHitWhenInScope.py b/lldb/test/functionalities/watchpoint/variable_out_of_scope/TestWatchedVarHitWhenInScope.py
new file mode 100644
index 00000000000..b690432c2cd
--- /dev/null
+++ b/lldb/test/functionalities/watchpoint/variable_out_of_scope/TestWatchedVarHitWhenInScope.py
@@ -0,0 +1,88 @@
+"""
+Test that a variable watchpoint should only hit when in scope.
+"""
+
+import os, time
+import unittest2
+import lldb
+from lldbtest import *
+
+class WatchedVariableHitWhenInScopeTestCase(TestBase):
+
+    mydir = os.path.join("functionalities", "watchpoint", "variable_out_of_scope")
+
+    @dsym_test
+    def test_watched_var_should_only_hit_when_in_scope_with_dsym(self):
+        """Test that a variable watchpoint should only hit when in scope."""
+        self.buildDsym(dictionary=self.d)
+        self.setTearDownCleanup(dictionary=self.d)
+        self.watched_var()
+
+    @dwarf_test
+    def test_watched_var_should_only_hit_when_in_scope_with_dwarf(self):
+        """Test that a variable watchpoint should only hit when in scope."""
+        self.buildDwarf(dictionary=self.d)
+        self.setTearDownCleanup(dictionary=self.d)
+        self.watched_var()
+
+    def setUp(self):
+        # Call super's setUp().
+        TestBase.setUp(self)
+        # Our simple source filename.
+        self.source = 'main.c'
+        self.exe_name = self.testMethodName
+        self.d = {'C_SOURCES': self.source, 'EXE': self.exe_name}
+
+    def watched_var(self):
+        """Test a simple sequence of watchpoint creation and watchpoint hit."""
+        exe = os.path.join(os.getcwd(), self.exe_name)
+        self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+        # Add a breakpoint to set a watchpoint when stopped in main.
+        self.expect("breakpoint set -n main", BREAKPOINT_CREATED,
+            startstr = "Breakpoint created: 1: name = 'main'")
+
+        # Run the program.
+        self.runCmd("run", RUN_SUCCEEDED)
+
+        # We should be stopped again due to the breakpoint.
+        # The stop reason of the thread should be breakpoint.
+        self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+            substrs = ['stopped',
+                       'stop reason = breakpoint'])
+
+        # Now let's set a watchpoint for 'c.a'.
+        # There should be only one watchpoint hit (see main.c).
+        self.expect("watchpoint set variable c.a", WATCHPOINT_CREATED,
+            substrs = ['Watchpoint created', 'size = 4', 'type = w'])
+
+        # Use the '-v' option to do verbose listing of the watchpoint.
+        # The hit count should be 0 initially.
+        self.expect("watchpoint list -v",
+            substrs = ['hit_count = 0'])
+
+        self.runCmd("process continue")
+
+        # We should be stopped again due to the watchpoint (write type), but
+        # only once.  The stop reason of the thread should be watchpoint.
+        self.expect("thread list", STOPPED_DUE_TO_WATCHPOINT,
+            substrs = ['stopped',
+                       'stop reason = watchpoint'])
+
+        self.runCmd("process continue")
+        # Don't expect the read of 'global' to trigger a stop exception.
+        # The process status should be 'exited'.
+        self.expect("process status",
+            substrs = ['exited'])
+
+        # Use the '-v' option to do verbose listing of the watchpoint.
+        # The hit count should now be 1.
+        self.expect("watchpoint list -v",
+            substrs = ['hit_count = 1'])
+
+
+if __name__ == '__main__':
+    import atexit
+    lldb.SBDebugger.Initialize()
+    atexit.register(lambda: lldb.SBDebugger.Terminate())
+    unittest2.main()
diff --git a/lldb/test/functionalities/watchpoint/variable_out_of_scope/main.c b/lldb/test/functionalities/watchpoint/variable_out_of_scope/main.c
new file mode 100644
index 00000000000..1bf7a00ac83
--- /dev/null
+++ b/lldb/test/functionalities/watchpoint/variable_out_of_scope/main.c
@@ -0,0 +1,15 @@
+typedef struct
+{
+    int a;
+    float b;
+} mystruct;
+
+int main()
+{
+    mystruct c;
+    
+    c.a = 5;
+    c.b = 3.6;
+    
+    return 0;
+}
\ No newline at end of file