Fix variable request from reusing variable_ids

Author: Anthony-Eid

lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp

@@ -29,8 +29,8 @@ namespace lldb_dap {
 ///
 /// \return
 ///     A `protocol::Scope`
-static Scope CreateScope(const llvm::StringRef name, int64_t variablesReference,
-                         int64_t namedVariables, bool expensive) {
+Scope CreateScope(const llvm::StringRef name, int64_t variablesReference,
+                  int64_t namedVariables, bool expensive) {
   Scope scope;
   scope.name = name;
 
@@ -75,22 +75,31 @@ ScopesRequestHandler::Run(const ScopesArguments &args) const {
     frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread());
     frame.GetThread().SetSelectedFrame(frame.GetFrameID());
   }
-  dap.variables.locals = frame.GetVariables(/*arguments=*/true,
-                                            /*locals=*/true,
-                                            /*statics=*/false,
-                                            /*in_scope_only=*/true);
-  dap.variables.globals = frame.GetVariables(/*arguments=*/false,
-                                             /*locals=*/false,
-                                             /*statics=*/true,
-                                             /*in_scope_only=*/true);
-  dap.variables.registers = frame.GetRegisters();
-
-  std::vector scopes = {CreateScope("Locals", VARREF_LOCALS,
-                                    dap.variables.locals.GetSize(), false),
-                        CreateScope("Globals", VARREF_GLOBALS,
-                                    dap.variables.globals.GetSize(), false),
-                        CreateScope("Registers", VARREF_REGS,
-                                    dap.variables.registers.GetSize(), false)};
+
+  uint32_t frame_id = frame.GetFrameID();
+
+  dap.variables.ReadyFrame(frame_id, frame);
+  dap.variables.SwitchFrame(frame_id);
+
+  std::vector<Scope> scopes = {};
+
+  int64_t variable_reference = dap.variables.GetNewVariableReference(false);
+  scopes.push_back(CreateScope("Locals", variable_reference,
+                               dap.variables.locals.GetSize(), false));
+
+  dap.variables.AddScopeKind(variable_reference, ScopeKind::Locals, frame_id);
+
+  variable_reference = dap.variables.GetNewVariableReference(false);
+  scopes.push_back(CreateScope("Globals", variable_reference,
+                               dap.variables.globals.GetSize(), false));
+  dap.variables.AddScopeKind(variable_reference, ScopeKind::Globals, frame_id);
+
+  variable_reference = dap.variables.GetNewVariableReference(false);
+  scopes.push_back(CreateScope("Registers", variable_reference,
+                               dap.variables.registers.GetSize(), false));
+
+  dap.variables.AddScopeKind(variable_reference, ScopeKind::Registers,
+                             frame_id);
 
   return ScopesResponseBody{std::move(scopes)};
 }

lldb/tools/lldb-dap/JSONUtils.h

@@ -9,6 +9,7 @@
 #ifndef LLDB_TOOLS_LLDB_DAP_JSONUTILS_H
 #define LLDB_TOOLS_LLDB_DAP_JSONUTILS_H
 
+#include "DAP.h"
 #include "DAPForward.h"
 #include "Protocol/ProtocolTypes.h"
 #include "lldb/API/SBCompileUnit.h"

lldb/tools/lldb-dap/Variables.cpp

@@ -8,27 +8,42 @@
 
 #include "Variables.h"
 #include "JSONUtils.h"
+#include "lldb/API/SBFrame.h"
 
 using namespace lldb_dap;
 
 lldb::SBValueList *Variables::GetTopLevelScope(int64_t variablesReference) {
-  switch (variablesReference) {
-  case VARREF_LOCALS:
+  auto iter = m_scope_kinds.find(variablesReference);
+  if (iter == m_scope_kinds.end()) {
+    return nullptr;
+  }
+
+  ScopeKind scope_kind = iter->second.first;
+  uint32_t frame_id = iter->second.second;
+
+  if (!SwitchFrame(frame_id)) {
+    return nullptr;
+  }
+
+  switch (scope_kind) {
+  case lldb_dap::ScopeKind::Locals:
     return &locals;
-  case VARREF_GLOBALS:
+  case lldb_dap::ScopeKind::Globals:
     return &globals;
-  case VARREF_REGS:
+  case lldb_dap::ScopeKind::Registers:
     return &registers;
-  default:
-    return nullptr;
   }
+
+  return nullptr;
 }
 
 void Variables::Clear() {
   locals.Clear();
   globals.Clear();
   registers.Clear();
   m_referencedvariables.clear();
+  m_frames.clear();
+  m_next_temporary_var_ref = VARREF_FIRST_VAR_IDX;
 }
 
 int64_t Variables::GetNewVariableReference(bool is_permanent) {
@@ -103,3 +118,56 @@ lldb::SBValue Variables::FindVariable(uint64_t variablesReference,
   }
   return variable;
 }
+
+std::optional<ScopeKind>
+Variables::GetScopeKind(const int64_t variablesReference) {
+  auto iter = m_scope_kinds.find(variablesReference);
+  if (iter != m_scope_kinds.end()) {
+    return iter->second.first;
+  }
+
+  return std::nullopt;
+}
+
+bool Variables::SwitchFrame(const uint32_t frame_id) {
+  auto iter = m_frames.find(frame_id);
+
+  if (iter == m_frames.end()) {
+    return false;
+  }
+
+  auto [frame_locals, frame_globals, frame_registers] = iter->second;
+
+  locals = frame_locals;
+  globals = frame_globals;
+  registers = frame_registers;
+
+  return true;
+}
+
+void Variables::ReadyFrame(uint32_t frame_id, lldb::SBFrame &frame) {
+  if (m_frames.find(frame_id) == m_frames.end()) {
+
+    auto locals = frame.GetVariables(/*arguments=*/true,
+                                     /*locals=*/true,
+                                     /*statics=*/false,
+                                     /*in_scope_only=*/true);
+
+    auto globals = frame.GetVariables(/*arguments=*/false,
+                                      /*locals=*/false,
+                                      /*statics=*/true,
+                                      /*in_scope_only=*/true);
+
+    auto registers = frame.GetRegisters();
+
+    m_frames.insert(
+        std::make_pair(frame_id, std::make_tuple(locals, globals, registers)));
+  }
+}
+
+void Variables::AddScopeKind(int64_t variable_reference, ScopeKind kind,
+                             uint32_t frame_id) {
+
+  m_scope_kinds[variable_reference] =
+      std::make_pair(ScopeKind::Globals, frame_id);
+}

lldb/tools/lldb-dap/Variables.h

@@ -12,6 +12,7 @@
 #include "lldb/API/SBValue.h"
 #include "lldb/API/SBValueList.h"
 #include "llvm/ADT/DenseMap.h"
+#include <map>
 
 #define VARREF_FIRST_VAR_IDX (int64_t)4
 #define VARREF_LOCALS (int64_t)1
@@ -20,6 +21,8 @@
 
 namespace lldb_dap {
 
+enum ScopeKind { Locals, Globals, Registers };
+
 struct Variables {
   lldb::SBValueList locals;
   lldb::SBValueList globals;
@@ -47,12 +50,23 @@ struct Variables {
 
   lldb::SBValue FindVariable(uint64_t variablesReference, llvm::StringRef name);
 
+  bool SwitchFrame(uint32_t frame_id);
+  /// Initialize a frame if it hasn't been already, otherwise do nothing
+  void ReadyFrame(uint32_t frame_id, lldb::SBFrame &frame);
+  std::optional<ScopeKind> GetScopeKind(const int64_t variablesReference);
+
   /// Clear all scope variables and non-permanent expandable variables.
   void Clear();
 
+  void AddScopeKind(int64_t variable_reference, ScopeKind kind,
+                    uint32_t frame_id);
+
 private:
   /// Variable_reference start index of permanent expandable variable.
   static constexpr int64_t PermanentVariableStartIndex = (1ll << 32);
+  int64_t m_next_temporary_var_ref{VARREF_FIRST_VAR_IDX};
+
+  std::map<int, std::pair<ScopeKind, uint32_t>> m_scope_kinds;
 
   /// Variables that are alive in this stop state.
   /// Will be cleared when debuggee resumes.
@@ -62,7 +76,9 @@ struct Variables {
   /// These are the variables evaluated from debug console REPL.
   llvm::DenseMap<int64_t, lldb::SBValue> m_referencedpermanent_variables;
 
-  int64_t m_next_temporary_var_ref{VARREF_FIRST_VAR_IDX};
+  std::map<uint32_t,
+           std::tuple<lldb::SBValueList, lldb::SBValueList, lldb::SBValueList>>
+      m_frames;
   int64_t m_next_permanent_var_ref{PermanentVariableStartIndex};
 };