[lldb/Expression] Improve interpreter error message with a non-running target

[mib]

Summary

[lldb/Expression] Improve interpreter error message with a non-running target

When trying to interpret an expression with a function call, if the
process hasn't been launched, the expression fails to be interpreted
and the user gets the following error message:

error: Can't run the expression locally

This message doesn't explain why the expression failed to be
interpreted, that's why this patch improves the error message that is
displayed when trying to run an expression while no process is running.

rdar://11991708

Signed-off-by: Med Ismail Bennani <[e-mail]>

[vsk]

Nice. So, does !support_function_calls always imply that the target isn’t running?

Also — s/interprete/interpret/, I think.

[mib]

Actually, the target could be running but doesn't support interpreting expression with function calls.

May be I should add another error message for target that doesn't support interpreting function calls.

[teemperor]

To give this more context: The IRInterpreter can interpret function calls but only on Hexagon (see D9404). The reason why we essentially always see this error message when the target isn't running is because most people don't target Hexagon and function calls is the most common reason IRInterpreter::CanInterpret returns an error. However, it *could* also fail for any of the other error messages in this function.

So while the idea of the patch is great (the error message is very confusing), this is not the right place to mention a non-running target. We ask the IRInterpretet if it can interpret the expression and it should answer with why it can't interpret it. The reason is that it can't interpret function calls (in this regard the error message can be improved here). If we start talking about a running target here then we also need to add this same text to all other error messages (in fact, we would need to replace all other error message with this to be consistent). It also makes the error message an oxymoron:

error: Interpreting the expression locally failed:
          Interpreter requires to run the program to interprete the expression

I would go up to the ClangExpressionParser and fix the error message there:

if (!can_interpret && execution_policy == eExecutionPolicyNever) {
  err.SetErrorStringWithFormat("Can't run the expression locally: %s",
                               interpret_error.AsCString());
  return err;
}

This could be something like "Can't interpret the expression without a running target. Interpretation failed due to: %s" or something like that.

[mib]

Moved error to ClangExpressionParser instead of IRInterpreter.

[mib]

Update test.

[teemperor]

LGTM, thanks!

lldb/packages/Python/lldbsuite/test/commands/expression/dont_allow_jit/TestAllowJIT.py

def test_allow_jit_options(self):
         self.main_source_file = lldb.SBFileSpec("main.c")
         self.expr_options_test()
 
     def expr_cmd_test(self):
         (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
                                    "Set a breakpoint here", self.main_source_file)
 
         frame = thread.GetFrameAtIndex(0)
-        
         # First make sure we can call the function with 
         interp = self.dbg.GetCommandInterpreter()
         self.expect("expr --allow-jit 1 -- call_me(10)",
                     substrs = ["(int) $", "= 18"])
         # Now make sure it fails with the "can't IR interpret message" if allow-jit is false:
         self.expect("expr --allow-jit 0 -- call_me(10)",
                     error=True,
-                    substrs = ["Can't run the expression locally"])
-        
+                    substrs = ["Can't evaluate the expression without a running target"])
     def expr_options_test(self):
         (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
                                    "Set a breakpoint here", self.main_source_file)
 
         frame = thread.GetFrameAtIndex(0)
 
         # First make sure we can call the function with the default option set. 
         options = lldb.SBExpressionOptions()
         # Check that the default is to allow JIT:
         self.assertEqual(options.GetAllowJIT(), True, "Default is true")
 
         # Now use the options:
         result = frame.EvaluateExpression("call_me(10)", options)
         self.assertTrue(result.GetError().Success(), "expression succeeded")
         self.assertEqual(result.GetValueAsSigned(), 18, "got the right value.")
 
         # Now disallow JIT and make sure it fails:
         options.SetAllowJIT(False)
         # Check that we got the right value:
         self.assertEqual(options.GetAllowJIT(), False, "Got False after setting to False")
 
         # Again use it and ensure we fail:
         result = frame.EvaluateExpression("call_me(10)", options)
         self.assertTrue(result.GetError().Fail(), "expression failed with no JIT")
-        self.assertTrue("Can't run the expression locally" in result.GetError().GetCString(), "Got right error")
+        self.assertTrue("Can't evaluate the expression without a running target" in result.GetError().GetCString(), "Got right error")
 
         # Finally set the allow JIT value back to true and make sure that works:
         options.SetAllowJIT(True)
         self.assertEqual(options.GetAllowJIT(), True, "Set back to True correctly")
 
         # And again, make sure this works:
         result = frame.EvaluateExpression("call_me(10)", options)
         self.assertTrue(result.GetError().Success(), "expression succeeded")
         self.assertEqual(result.GetValueAsSigned(), 18, "got the right value.")
 

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

if (execution_policy != eExecutionPolicyAlways &&
 
       bool interpret_function_calls =
           !process ? false : process->CanInterpretFunctionCalls();
       can_interpret = IRInterpreter::CanInterpret(
           *execution_unit_sp->GetModule(), *execution_unit_sp->GetFunction(),
           interpret_error, interpret_function_calls);
 
       if (!can_interpret && execution_policy == eExecutionPolicyNever) {
-        err.SetErrorStringWithFormat("Can't run the expression locally: %s",
-                                     interpret_error.AsCString());
+        err.SetErrorStringWithFormat(
+            "Can't evaluate the expression without a running target due to: %s",
         return err;
       }
     }
 
     if (!process && execution_policy == eExecutionPolicyAlways) {
       err.SetErrorString("Expression needed to run in the target, but the "
                          "target can't be run");
       return err;