clayborg created this revision.
clayborg added reviewers: labath, JDevlieghere, yinghuitan.
Herald added a project: All.
clayborg requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

After recent diffs that enable variable errors that stop variables from being 
correctly displayed when debugging, allow users to see these errors in the 
LOCALS variables in the VS Code UI. We do this by detecting when no variables 
are available and when there is an error to be displayed, and we add a single 
variable named "<error>" whose value is a string error that the user can read. 
This allows the user to be aware of the reason variables are not available and 
fix the issue. Previously if someone enabled "-gline-tables-only" or was 
debugging with DWARF in .o files or with .dwo files and those separate object 
files were missing or they were out of date, the user would see nothing in the 
variables view. Communicating these errors to the user is essential to a good 
debugging experience.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134333

Files:
  lldb/source/API/SBError.cpp
  lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===================================================================
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -2953,6 +2953,27 @@
     }
 
     num_children = top_scope->GetSize();
+    if (num_children == 0 && variablesReference == VARREF_LOCALS) {
+      // Check for an error in the SBValueList that might explain why we don't
+      // have locals. If we have an error display it as the sole value in the
+      // the locals.
+      const char *var_err = top_scope->GetError().GetCString();
+      if (var_err) {
+        // Create a fake variable named "error" to explain why variables were
+        // not available. This new error will help let users know when there was
+        // a problem that kept variables from being available for display and
+        // allow users to fix this issue instead of seeing no variables. The
+        // errors are only set when there is a problem that the user could
+        // fix, so no error will show up when you have no debug info, only when
+        // we do have debug info and something that is fixable can be done.
+        llvm::json::Object object;
+        EmplaceSafeString(object, "name", "<error>");
+        EmplaceSafeString(object, "type", "const char *");
+        EmplaceSafeString(object, "value", var_err);
+        object.try_emplace("variablesReference", (int64_t)0);
+        variables.emplace_back(std::move(object));
+      }
+    }
     const int64_t end_idx = start_idx + ((count == 0) ? num_children : count);
 
     // We first find out which variable names are duplicated
Index: lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
===================================================================
--- lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
+++ lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
@@ -7,7 +7,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 import lldbvscode_testcase
-
+import os
 
 def make_buffer_verify_dict(start_idx, count, offset=0):
     verify_dict = {}
@@ -38,6 +38,19 @@
                                  ' "%s")') % (
                                     key, actual_value,
                                     verify_value))
+        if 'contains' in verify_dict:
+            verify = verify_dict['contains']
+            for key in verify:
+                contains_array = verify[key]
+                actual_value = actual[key]
+                self.assertTrue(isinstance(contains_array, list))
+                for verify_value in contains_array:
+                    contains = verify_value in actual_value
+                    self.assertTrue(contains,
+                                    ('"%s" value "%s" doesn\'t contain'
+                                     ' "%s")') % (
+                                        key, actual_value,
+                                        verify_value))
         if 'missing' in verify_dict:
             missing = verify_dict['missing']
             for key in missing:
@@ -508,3 +521,46 @@
                         self.assertTrue(value.startswith('0x'))
                         self.assertTrue('a.out`main + ' in value)
                         self.assertTrue('at main.cpp:' in value)
+
+    @no_debug_info_test
+    @skipUnlessDarwin
+    def test_darwin_dwarf_missing_obj(self):
+        '''
+            Test that if we build a binary with DWARF in .o files and we remove
+            the .o file for main.cpp, that we get a variable named "<error>"
+            whose value matches the appriopriate error. Errors when getting
+            variables are returned in the LLDB API when the user should be
+            notified of issues that can easily be solved by rebuilding or
+            changing compiler options and are designed to give better feedback
+            to the user.
+        '''
+        self.build(debug_info="dwarf")
+        program = self.getBuildArtifact("a.out")
+        main_obj = self.getBuildArtifact("main.o")
+        self.assertTrue(os.path.exists(main_obj))
+        # Delete the main.o file that contains the debug info so we force an
+        # error when we run to main and try to get variables
+        os.unlink(main_obj)
+
+        self.create_debug_adaptor()
+        self.assertTrue(os.path.exists(program), 'executable must exist')
+        self.launch(program)
+
+        functions = ['main']
+        breakpoint_ids = self.set_function_breakpoints(functions)
+        self.assertEquals(len(breakpoint_ids), len(functions), "expect one breakpoint")
+        self.continue_to_breakpoints(breakpoint_ids)
+
+        locals = self.vscode.get_local_variables()
+
+        verify_locals = {
+            '<error>': {
+                'equals': {'type': 'const char *'},
+                'contains': { 'value': [
+                    'debug map object file ',
+                    'main.o" containing debug info does not exist, debug info will not be loaded']
+                }
+            },
+        }
+        varref_dict = {}
+        self.verify_variables(verify_locals, locals, varref_dict)
Index: lldb/source/API/SBError.cpp
===================================================================
--- lldb/source/API/SBError.cpp
+++ lldb/source/API/SBError.cpp
@@ -38,8 +38,19 @@
 const char *SBError::GetCString() const {
   LLDB_INSTRUMENT_VA(this);
 
+  // We need to put the error into a ConstString and then hand out a uniqued
+  // version of the string to ensure the lifetime of the returns string lives
+  // forever in case people use temporary variables that return SBError
+  // instances. Any code like this could fail:
+  //
+  //    const char *error_str = foo.GetError().GetCString();
+  //
+  // A SBError object owns a Status object and it might destruct this object
+  // during this call, we have a race condition as to what happens to the
+  // string since the Status::GetCString() returns a "const char *" from the
+  // owned std::string object.
   if (m_opaque_up)
-    return m_opaque_up->AsCString();
+    return ConstString(m_opaque_up->AsCString()).AsCString();
   return nullptr;
 }
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to