[Lldb-commits] [lldb] [lldb-dap] Fix global-scope evaluate always routing to command interpreter (PR #225243)

[via lldb-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Nathan Fusselman (nathanfusselman)

<details>
<summary>Changes</summary>

DetectReplMode returned Command unconditionally when there was no frame, since it couldn't check FindVariable without one. A frameless evaluate (no frameId, e.g. `(int)getpid()`) was always treated as an unknown command instead of an expression.

Fall back to SBTarget::FindFirstGlobalVariable when there's no frame: DAP::target is valid independent of frame, so globals/statics can still be checked without one.

rdar://187884135

---
Full diff: https://github.com/llvm/llvm-project/pull/225243.diff


3 Files Affected:

- (modified) lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py (+25-5) 
- (modified) lldb/test/API/tools/lldb-dap/evaluate/main.cpp (+4) 
- (modified) lldb/tools/lldb-dap/DAP.cpp (+6-5) 


``````````diff
diff --git a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
index 2ace6eeccc228..6aef5303c102a 100644
--- a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
+++ b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
@@ -54,6 +54,11 @@ def assert_eval_fails(expression: str):
                 f"expected {expression!r} to fail using {context=!r} in {frame_id=!r}"
             )
 
+        def assert_eval_fails_frameless(expression: str):
+            session.do_evaluate(expression, frameId=None, context=context).error(
+                f"expected {expression!r} to fail using {context=!r} with no frame"
+            )
+
         source = "main.cpp"
         program = self.getBuildArtifact("a.out")
         breakpoint_lines = [
@@ -225,14 +230,22 @@ def assert_eval_fails(expression: str):
         session.verify_evaluate(eval_body, matches="20")
 
         if context_parses_expressions:
-            # Access global variable without a frame
-            # Run in variable mode to avoid interpreting it as a command.
-            session.evaluate("`lldb-dap repl-mode variable", context="repl")
-
+            # Access a global variable with no frame at all (frameId omitted).
             eval_body = session.evaluate("static_int", context=context)
             session.verify_evaluate(eval_body, matches="42", type="int")
 
-            session.evaluate("`lldb-dap repl-mode auto", context="repl")
+            assert_eval_fails_frameless("var1")  # local, not global.
+            assert_eval_fails_frameless("totally_bogus_expr_xyz")
+
+            if context in ("repl", None):
+                # `help` shares its name with a real lldb command (see main.cpp);
+                # it should resolve to the global, with an ambiguity warning.
+                eval_body = session.evaluate("help", context=context)
+                session.verify_evaluate(eval_body, matches="99", type="int")
+
+                # `version` has no matching global; still runs as a command frameless.
+                eval_body = session.evaluate("version", context=context)
+                session.verify_evaluate(eval_body, matches=r"^lldb[ -]\d", has_mem_ref=False)
 
         # In a_function's own frame these names are out of scope.
         assert_eval_fails("var1")
@@ -346,6 +359,13 @@ def assert_eval_fails(expression: str):
 
         session.continue_to_exit()
 
+        if context in ("repl", None):
+            console_str = session.get_console()
+            self.assertIn(
+                "Expression 'help' is both an LLDB command and variable",
+                console_str,
+            )
+
     @skipIfWindows
     def test_generic_evaluate_expressions(self):
         # Tests context-less expression evaluations.
diff --git a/lldb/test/API/tools/lldb-dap/evaluate/main.cpp b/lldb/test/API/tools/lldb-dap/evaluate/main.cpp
index 112726677637c..8201ca0867993 100644
--- a/lldb/test/API/tools/lldb-dap/evaluate/main.cpp
+++ b/lldb/test/API/tools/lldb-dap/evaluate/main.cpp
@@ -8,6 +8,10 @@ static int static_int = 42;
 
 int non_static_int = 43;
 
+// Deliberately shares its name with the `help` lldb command, to test the
+// command/variable ambiguity warning for a frameless (global-scope) evaluate.
+int help = 99;
+
 int a_function(int list) {
   return list; // breakpoint 3
 }
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 7e59541e6c834..7fef0874ddcef 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -592,10 +592,6 @@ ReplMode DAP::DetectReplMode(lldb::SBFrame &frame, std::string &expression,
   if (repl_mode != ReplMode::Auto)
     return repl_mode;
 
-  // We cannot check if expression is a variable without a frame.
-  if (!frame)
-    return ReplMode::Command;
-
   // To determine if the expression is a command or not, check if the first
   // term is a variable or command. If it's a variable in scope we will prefer
   // that behavior and give a warning to the user if they meant to invoke the
@@ -618,7 +614,12 @@ ReplMode DAP::DetectReplMode(lldb::SBFrame &frame, std::string &expression,
   const bool is_command = interpreter.CommandExists(first_cstr) ||
                           interpreter.UserCommandExists(first_cstr) ||
                           interpreter.AliasExists(first_cstr);
-  const bool is_variable = frame.FindVariable(first_cstr).IsValid();
+  // Without a frame there are no locals to check; fall back to `target`
+  // (this DAP's own member, valid independent of `frame`) to look up a
+  // global/static of the same name instead.
+  const bool is_variable =
+      frame ? frame.FindVariable(first_cstr).IsValid()
+            : target.FindFirstGlobalVariable(first_cstr).IsValid();
 
   // If we have both a variable and command, warn the user about the conflict.
   if (!partial_expression && is_command && is_variable) {

``````````

</details>


https://github.com/llvm/llvm-project/pull/225243