Breakpoint conditions were making result variables, which they should not do.
authorJim Ingham <jingham@apple.com>
Tue, 12 Apr 2016 17:17:35 +0000 (17:17 +0000)
committerJim Ingham <jingham@apple.com>
Tue, 12 Apr 2016 17:17:35 +0000 (17:17 +0000)
The result variables aren't useful, and if you have a breakpoint on a
common function you can generate a lot of these.  So I changed the
code that checks the condition to set ResultVariableIsInternal in the
EvaluateExpressionOptions that we pass to the execution.
Unfortunately, the check for this variable was done in the wrong place
(the static UserExpression::Evaluate) which is not how breakpoint
conditions execute expressions (UserExpression::Execute).  So I moved
the check to UserExpression::Execute (which Evaluate also calls) and made the
overridden method DoExecute.

llvm-svn: 266093

lldb/include/lldb/Expression/LLVMUserExpression.h
lldb/include/lldb/Expression/UserExpression.h
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
lldb/source/Breakpoint/BreakpointLocation.cpp
lldb/source/Expression/LLVMUserExpression.cpp
lldb/source/Expression/UserExpression.cpp
lldb/source/Plugins/ExpressionParser/Go/GoUserExpression.cpp
lldb/source/Plugins/ExpressionParser/Go/GoUserExpression.h

index 43ac0eb..8775c8b 100644 (file)
@@ -40,11 +40,6 @@ public:
                        const EvaluateExpressionOptions &options);
     ~LLVMUserExpression() override;
 
-    lldb::ExpressionResults
-    Execute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
-            const EvaluateExpressionOptions &options, lldb::UserExpressionSP &shared_ptr_to_me,
-            lldb::ExpressionVariableSP &result) override;
-
     bool
     FinalizeJITExecution(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
                          lldb::ExpressionVariableSP &result,
@@ -70,6 +65,11 @@ public:
     lldb::ModuleSP GetJITModule() override;
 
 protected:
+    lldb::ExpressionResults
+    DoExecute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
+              const EvaluateExpressionOptions &options, lldb::UserExpressionSP &shared_ptr_to_me,
+              lldb::ExpressionVariableSP &result) override;
+
     virtual void
     ScanContext(ExecutionContext &exe_ctx, lldb_private::Error &err) = 0;
 
index b8b5e96..a038af1 100644 (file)
@@ -107,7 +107,8 @@ public:
     MatchesContext (ExecutionContext &exe_ctx);
 
     //------------------------------------------------------------------
-    /// Execute the parsed expression
+    /// Execute the parsed expression by callinng the derived class's
+    /// DoExecute method.
     ///
     /// @param[in] diagnostic_manager
     ///     A diagnostic manager to report errors to.
@@ -133,9 +134,9 @@ public:
     /// @return
     ///     A Process::Execution results value.
     //------------------------------------------------------------------
-    virtual lldb::ExpressionResults
+    lldb::ExpressionResults
     Execute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx, const EvaluateExpressionOptions &options,
-            lldb::UserExpressionSP &shared_ptr_to_me, lldb::ExpressionVariableSP &result) = 0;
+            lldb::UserExpressionSP &shared_ptr_to_me, lldb::ExpressionVariableSP &result);
 
     //------------------------------------------------------------------
     /// Apply the side effects of the function to program state.
@@ -312,6 +313,10 @@ public:
     }
 
 protected:
+    virtual lldb::ExpressionResults
+    DoExecute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx, const EvaluateExpressionOptions &options,
+            lldb::UserExpressionSP &shared_ptr_to_me, lldb::ExpressionVariableSP &result) = 0;
+
     static lldb::addr_t
     GetObjectPointer (lldb::StackFrameSP frame_sp,
                       ConstString &object_name,
index ac2d021..c0e5343 100644 (file)
@@ -179,4 +179,8 @@ class BreakpointConditionsTestCase(TestBase):
         # The hit count for the breakpoint should be 1.
         self.assertTrue(breakpoint.GetHitCount() == 1)
 
+        # Test that the condition expression didn't create a result variable:
+        options = lldb.SBExpressionOptions()
+        value = frame0.EvaluateExpression("$0", options)
+        self.assertTrue(value.GetError().Fail(), "Conditions should not make result variables.")
         process.Continue()
index 1d66d9d..3c32635 100644 (file)
@@ -324,6 +324,7 @@ BreakpointLocation::ConditionSaysStop (ExecutionContext &exe_ctx, Error &error)
     options.SetUnwindOnError(true);
     options.SetIgnoreBreakpoints(true);
     options.SetTryAllThreads(true);
+    options.SetResultIsInternal(true); // Don't generate a user variable for condition expressions.
 
     Error expr_error;
 
index 3c42a54..60f68b1 100644 (file)
@@ -77,9 +77,9 @@ LLVMUserExpression::~LLVMUserExpression()
 }
 
 lldb::ExpressionResults
-LLVMUserExpression::Execute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
-                            const EvaluateExpressionOptions &options, lldb::UserExpressionSP &shared_ptr_to_me,
-                            lldb::ExpressionVariableSP &result)
+LLVMUserExpression::DoExecute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
+                              const EvaluateExpressionOptions &options, lldb::UserExpressionSP &shared_ptr_to_me,
+                              lldb::ExpressionVariableSP &result)
 {
     // The expression log is quite verbose, and if you're just tracking the execution of the
     // expression, it's quite convenient to have these logs come out with the STEP log as well.
index 2438708..3e2e07e 100644 (file)
@@ -356,11 +356,6 @@ UserExpression::Evaluate (ExecutionContext &exe_ctx,
             execution_results =
                 user_expression_sp->Execute(diagnostic_manager, exe_ctx, options, user_expression_sp, expr_result);
 
-            if (options.GetResultIsInternal() && expr_result && process)
-            {
-                process->GetTarget().GetPersistentExpressionStateForLanguage(language)->RemovePersistentVariable (expr_result);
-            }
-
             if (execution_results != lldb::eExpressionCompleted)
             {
                 if (log)
@@ -405,3 +400,21 @@ UserExpression::Evaluate (ExecutionContext &exe_ctx,
 
     return execution_results;
 }
+
+lldb::ExpressionResults
+UserExpression::Execute(DiagnosticManager &diagnostic_manager,
+                        ExecutionContext &exe_ctx,
+                        const EvaluateExpressionOptions &options,
+                        lldb::UserExpressionSP &shared_ptr_to_me,
+                        lldb::ExpressionVariableSP &result_var)
+{
+    lldb::ExpressionResults expr_result = DoExecute(diagnostic_manager, exe_ctx, options, shared_ptr_to_me, result_var);
+    Target *target = exe_ctx.GetTargetPtr();
+    if (options.GetResultIsInternal() && result_var && target)
+    {
+        target->GetPersistentExpressionStateForLanguage(m_language)->RemovePersistentVariable (result_var);
+    }
+    return expr_result;
+}
+
+
index 0457bfe..f69c3e2 100644 (file)
@@ -265,9 +265,9 @@ GoUserExpression::Parse(DiagnosticManager &diagnostic_manager, ExecutionContext
 }
 
 lldb::ExpressionResults
-GoUserExpression::Execute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
-                          const EvaluateExpressionOptions &options, lldb::UserExpressionSP &shared_ptr_to_me,
-                          lldb::ExpressionVariableSP &result)
+GoUserExpression::DoExecute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
+                            const EvaluateExpressionOptions &options, lldb::UserExpressionSP &shared_ptr_to_me,
+                            lldb::ExpressionVariableSP &result)
 {
     Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_EXPRESSIONS | LIBLLDB_LOG_STEP));
 
index 4d6cdd2..711a4c4 100644 (file)
@@ -70,11 +70,6 @@ class GoUserExpression : public UserExpression
             lldb_private::ExecutionPolicy execution_policy, bool keep_result_in_memory,
             bool generate_debug_info) override;
 
-      lldb::ExpressionResults
-      Execute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
-              const EvaluateExpressionOptions &options, lldb::UserExpressionSP &shared_ptr_to_me,
-              lldb::ExpressionVariableSP &result) override;
-
       bool
       CanInterpret() override
       {
@@ -89,6 +84,12 @@ class GoUserExpression : public UserExpression
           return true;
     }
 
+  protected:
+      lldb::ExpressionResults
+      DoExecute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
+                const EvaluateExpressionOptions &options, lldb::UserExpressionSP &shared_ptr_to_me,
+                lldb::ExpressionVariableSP &result) override;
+
   private:
     class GoInterpreter;
     std::unique_ptr<GoInterpreter> m_interpreter;