[Lldb-commits] [lldb] r266093 - Breakpoint conditions were making result variables, which they should not do.

Jim Ingham via lldb-commits lldb-commits at lists.llvm.org
Tue Apr 12 10:17:36 PDT 2016


Author: jingham
Date: Tue Apr 12 12:17:35 2016
New Revision: 266093

URL: http://llvm.org/viewvc/llvm-project?rev=266093&view=rev
Log:
Breakpoint conditions were making result variables, which they should not do.  

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.


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

Modified: lldb/trunk/include/lldb/Expression/LLVMUserExpression.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/LLVMUserExpression.h?rev=266093&r1=266092&r2=266093&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Expression/LLVMUserExpression.h (original)
+++ lldb/trunk/include/lldb/Expression/LLVMUserExpression.h Tue Apr 12 12:17:35 2016
@@ -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;
 

Modified: lldb/trunk/include/lldb/Expression/UserExpression.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/UserExpression.h?rev=266093&r1=266092&r2=266093&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Expression/UserExpression.h (original)
+++ lldb/trunk/include/lldb/Expression/UserExpression.h Tue Apr 12 12:17:35 2016
@@ -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,

Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py?rev=266093&r1=266092&r2=266093&view=diff
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py Tue Apr 12 12:17:35 2016
@@ -179,4 +179,8 @@ class BreakpointConditionsTestCase(TestB
         # 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()

Modified: lldb/trunk/source/Breakpoint/BreakpointLocation.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointLocation.cpp?rev=266093&r1=266092&r2=266093&view=diff
==============================================================================
--- lldb/trunk/source/Breakpoint/BreakpointLocation.cpp (original)
+++ lldb/trunk/source/Breakpoint/BreakpointLocation.cpp Tue Apr 12 12:17:35 2016
@@ -324,6 +324,7 @@ BreakpointLocation::ConditionSaysStop (E
     options.SetUnwindOnError(true);
     options.SetIgnoreBreakpoints(true);
     options.SetTryAllThreads(true);
+    options.SetResultIsInternal(true); // Don't generate a user variable for condition expressions.
 
     Error expr_error;
 

Modified: lldb/trunk/source/Expression/LLVMUserExpression.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/LLVMUserExpression.cpp?rev=266093&r1=266092&r2=266093&view=diff
==============================================================================
--- lldb/trunk/source/Expression/LLVMUserExpression.cpp (original)
+++ lldb/trunk/source/Expression/LLVMUserExpression.cpp Tue Apr 12 12:17:35 2016
@@ -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.

Modified: lldb/trunk/source/Expression/UserExpression.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/UserExpression.cpp?rev=266093&r1=266092&r2=266093&view=diff
==============================================================================
--- lldb/trunk/source/Expression/UserExpression.cpp (original)
+++ lldb/trunk/source/Expression/UserExpression.cpp Tue Apr 12 12:17:35 2016
@@ -356,11 +356,6 @@ UserExpression::Evaluate (ExecutionConte
             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 (ExecutionConte
 
     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;
+}
+
+

Modified: lldb/trunk/source/Plugins/ExpressionParser/Go/GoUserExpression.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Go/GoUserExpression.cpp?rev=266093&r1=266092&r2=266093&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/ExpressionParser/Go/GoUserExpression.cpp (original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Go/GoUserExpression.cpp Tue Apr 12 12:17:35 2016
@@ -265,9 +265,9 @@ GoUserExpression::Parse(DiagnosticManage
 }
 
 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));
 

Modified: lldb/trunk/source/Plugins/ExpressionParser/Go/GoUserExpression.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Go/GoUserExpression.h?rev=266093&r1=266092&r2=266093&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/ExpressionParser/Go/GoUserExpression.h (original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Go/GoUserExpression.h Tue Apr 12 12:17:35 2016
@@ -70,11 +70,6 @@ class GoUserExpression : public UserExpr
             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 UserExpr
           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;




More information about the lldb-commits mailing list