[Lldb-commits] [lldb] 8b19d13 - [lldb] Make frame var --regex always search globals

Felipe de Azevedo Piovezan via lldb-commits lldb-commits at lists.llvm.org
Wed Jul 19 14:11:40 PDT 2023


Author: Felipe de Azevedo Piovezan
Date: 2023-07-19T17:11:21-04:00
New Revision: 8b19d13fde6e32c8815f3f4e3f629208b0f1d0e9

URL: https://github.com/llvm/llvm-project/commit/8b19d13fde6e32c8815f3f4e3f629208b0f1d0e9
DIFF: https://github.com/llvm/llvm-project/commit/8b19d13fde6e32c8815f3f4e3f629208b0f1d0e9.diff

LOG: [lldb] Make frame var --regex always search globals

Currently frame var --regex sometimes searches globals, sometimes it doesn't.
This happens because `StackFrame::GetVariableList` always returns the biggest
list it has, regardless of whether only globals were requested or not. In other
words, if a previous call to `GetVariableList` requested globals, all subsequent
calls will see them.

The implication here is that users of `StackFrame::GetVariableList` are expected
to filter the results of this function. This is what we do for a vanilla
`frame var` command. But it is not what we do when `--regex` is used. This
commit solves the issue by:

1. Making `--regex` imply `--globals`. This matches the behavior of `frame var
<some_name>`, which will also search the global scope.
2. Making the `--regex` search respect the command object options.

See the added test for an example of the oddities this patch addresses. Without
the patch, the test fails. However it could be made to pass by calling a plain
`frame var` before calling `frame var --regex A::`.

Differential Revision: https://reviews.llvm.org/D155334

Added: 
    

Modified: 
    lldb/include/lldb/Symbol/VariableList.h
    lldb/source/Commands/CommandObjectFrame.cpp
    lldb/test/API/commands/frame/var/TestFrameVar.py

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Symbol/VariableList.h b/lldb/include/lldb/Symbol/VariableList.h
index 9fea628a81b6bf..3ee69527e23f93 100644
--- a/lldb/include/lldb/Symbol/VariableList.h
+++ b/lldb/include/lldb/Symbol/VariableList.h
@@ -75,6 +75,10 @@ class VariableList {
   const_iterator begin() const { return m_variables.begin(); }
   const_iterator end() const { return m_variables.end(); }
 
+  llvm::ArrayRef<lldb::VariableSP> toArrayRef() {
+    return llvm::makeArrayRef(m_variables);
+  }
+
 protected:
   collection m_variables;
 

diff  --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp
index 7638076a0ebeea..1390fd8748dfaf 100644
--- a/lldb/source/Commands/CommandObjectFrame.cpp
+++ b/lldb/source/Commands/CommandObjectFrame.cpp
@@ -499,6 +499,31 @@ may even involve JITing and running code in the target program.)");
     }
   }
 
+  /// Finds all the variables in `all_variables` whose name matches `regex`,
+  /// inserting them into `matches`. Variables already contained in `matches`
+  /// are not inserted again.
+  /// Nullopt is returned in case of no matches.
+  /// A sub-range of `matches` with all newly inserted variables is returned.
+  /// This may be empty if all matches were already contained in `matches`.
+  std::optional<llvm::ArrayRef<VariableSP>>
+  findUniqueRegexMatches(RegularExpression &regex,
+                         VariableList &matches,
+                         const VariableList &all_variables) {
+    bool any_matches = false;
+    const size_t previous_num_vars = matches.GetSize();
+
+    for (const VariableSP &var : all_variables) {
+      if (!var->NameMatches(regex) || !ScopeRequested(var->GetScope()))
+        continue;
+      any_matches = true;
+      matches.AddVariableIfUnique(var);
+    }
+
+    if (any_matches)
+      return matches.toArrayRef().drop_front(previous_num_vars);
+    return std::nullopt;
+  }
+
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     // No need to check "frame" for validity as eCommandRequiresFrame ensures
     // it is valid
@@ -506,6 +531,10 @@ may even involve JITing and running code in the target program.)");
 
     Stream &s = result.GetOutputStream();
 
+    // Using a regex should behave like looking for an exact name match: it
+    // also finds globals.
+    m_option_variable.show_globals |= m_option_variable.use_regex;
+
     // Be careful about the stack frame, if any summary formatter runs code, it
     // might clear the StackFrameList for the thread.  So hold onto a shared
     // pointer to the frame so it stays alive.
@@ -518,7 +547,6 @@ may even involve JITing and running code in the target program.)");
       result.AppendError(error.AsCString());
 
     }
-    VariableSP var_sp;
     ValueObjectSP valobj_sp;
 
     TypeSummaryImplSP summary_format_sp;
@@ -551,46 +579,38 @@ may even involve JITing and running code in the target program.)");
         // objects from them...
         for (auto &entry : command) {
           if (m_option_variable.use_regex) {
-            const size_t regex_start_index = regex_var_list.GetSize();
             llvm::StringRef name_str = entry.ref();
             RegularExpression regex(name_str);
             if (regex.IsValid()) {
-              size_t num_matches = 0;
-              const size_t num_new_regex_vars =
-                  variable_list->AppendVariablesIfUnique(regex, regex_var_list,
-                                                         num_matches);
-              if (num_new_regex_vars > 0) {
-                for (size_t regex_idx = regex_start_index,
-                            end_index = regex_var_list.GetSize();
-                     regex_idx < end_index; ++regex_idx) {
-                  var_sp = regex_var_list.GetVariableAtIndex(regex_idx);
-                  if (var_sp) {
-                    valobj_sp = frame->GetValueObjectForFrameVariable(
-                        var_sp, m_varobj_options.use_dynamic);
-                    if (valobj_sp) {
-                      std::string scope_string;
-                      if (m_option_variable.show_scope)
-                        scope_string = GetScopeString(var_sp).str();
-
-                      if (!scope_string.empty())
-                        s.PutCString(scope_string);
-
-                      if (m_option_variable.show_decl &&
-                          var_sp->GetDeclaration().GetFile()) {
-                        bool show_fullpaths = false;
-                        bool show_module = true;
-                        if (var_sp->DumpDeclaration(&s, show_fullpaths,
-                                                    show_module))
-                          s.PutCString(": ");
-                      }
-                      valobj_sp->Dump(result.GetOutputStream(), options);
-                    }
-                  }
-                }
-              } else if (num_matches == 0) {
+              std::optional<llvm::ArrayRef<VariableSP>> results =
+                  findUniqueRegexMatches(regex, regex_var_list, *variable_list);
+              if (!results) {
                 result.AppendErrorWithFormat(
                     "no variables matched the regular expression '%s'.",
                     entry.c_str());
+                continue;
+              }
+              for (const VariableSP &var_sp : *results) {
+                valobj_sp = frame->GetValueObjectForFrameVariable(
+                    var_sp, m_varobj_options.use_dynamic);
+                if (valobj_sp) {
+                  std::string scope_string;
+                  if (m_option_variable.show_scope)
+                    scope_string = GetScopeString(var_sp).str();
+
+                  if (!scope_string.empty())
+                    s.PutCString(scope_string);
+
+                  if (m_option_variable.show_decl &&
+                      var_sp->GetDeclaration().GetFile()) {
+                    bool show_fullpaths = false;
+                    bool show_module = true;
+                    if (var_sp->DumpDeclaration(&s, show_fullpaths,
+                                                show_module))
+                      s.PutCString(": ");
+                  }
+                  valobj_sp->Dump(result.GetOutputStream(), options);
+                }
               }
             } else {
               if (llvm::Error err = regex.GetError())
@@ -648,7 +668,7 @@ may even involve JITing and running code in the target program.)");
         const size_t num_variables = variable_list->GetSize();
         if (num_variables > 0) {
           for (size_t i = 0; i < num_variables; i++) {
-            var_sp = variable_list->GetVariableAtIndex(i);
+            VariableSP var_sp = variable_list->GetVariableAtIndex(i);
             if (!ScopeRequested(var_sp->GetScope()))
                 continue;
             std::string scope_string;

diff  --git a/lldb/test/API/commands/frame/var/TestFrameVar.py b/lldb/test/API/commands/frame/var/TestFrameVar.py
index 92bae03777ced4..c43121abfe4325 100644
--- a/lldb/test/API/commands/frame/var/TestFrameVar.py
+++ b/lldb/test/API/commands/frame/var/TestFrameVar.py
@@ -58,6 +58,16 @@ def do_test(self):
         command_result = lldb.SBCommandReturnObject()
         interp = self.dbg.GetCommandInterpreter()
 
+        # Ensure --regex can find globals if it is the very first frame var command.
+        self.expect("frame var --regex g_", substrs=["g_var"])
+
+        # Ensure the requested scope is respected:
+        self.expect(
+            "frame var --regex argc --no-args",
+            error=True,
+            substrs=["no variables matched the regular expression 'argc'"],
+        )
+
         # Just get args:
         result = interp.HandleCommand("frame var -l", command_result)
         self.assertEqual(


        


More information about the lldb-commits mailing list