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