diff --git a/lldb/bindings/interface/SBValueList.i b/lldb/bindings/interface/SBValueList.i --- a/lldb/bindings/interface/SBValueList.i +++ b/lldb/bindings/interface/SBValueList.i @@ -102,6 +102,8 @@ lldb::SBValue GetFirstValueByName (const char* name) const; + lldb::SBError GetError(); + %extend { %nothreadallow; std::string lldb::SBValueList::__str__ (){ diff --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h --- a/lldb/include/lldb/API/SBError.h +++ b/lldb/include/lldb/API/SBError.h @@ -64,6 +64,7 @@ friend class SBCommunication; friend class SBData; friend class SBDebugger; + friend class SBFile; friend class SBHostOS; friend class SBPlatform; friend class SBProcess; @@ -73,8 +74,8 @@ friend class SBThread; friend class SBTrace; friend class SBValue; + friend class SBValueList; friend class SBWatchpoint; - friend class SBFile; friend class lldb_private::ScriptInterpreter; diff --git a/lldb/include/lldb/API/SBValueList.h b/lldb/include/lldb/API/SBValueList.h --- a/lldb/include/lldb/API/SBValueList.h +++ b/lldb/include/lldb/API/SBValueList.h @@ -43,6 +43,33 @@ const lldb::SBValueList &operator=(const lldb::SBValueList &rhs); + // Get an error for why this list is empty. + // + // If this list is empty, check for an underlying error in the debug + // information that prevented this list from being populated. This is not + // meant to return an error if there is no debug information as it is ok for a + // value list to be empty and no error should be returned in that case. If the + // debug info is for an assembly file or language that doesn't have any + // variables, no error should be returned. + // + // This is designed as a way to let users know when they enable certain + // compiler options that enable debug information but provide a degraded + // debug information content, like -gline-tables-only, which is a compiler + // option that allows users to set file and line breakpoints, but users get + // confused when no variables show up during debugging. + // + // It is also designed to inform a user that debug information might be + // available if an external file, like a .dwo file, but that file doesn't + // exist or wasn't able to be loaded due to a mismatched ID. When debugging + // with fission enabled, the line tables are linked into the main executable, + // but if the .dwo or .dwp files are not available or have been modified, + // users can get confused if they can stop at a file and line breakpoint but + // can't see variables in this case. + // + // This error can give vital clues to the user about the cause is and allow + // the user to fix the issue. + lldb::SBError GetError(); + protected: // only useful for visualizing the pointer or comparing two SBValueLists to // see if they are backed by the same underlying Impl. @@ -68,6 +95,8 @@ ValueListImpl &ref(); std::unique_ptr m_opaque_up; + + void SetError(const lldb_private::Status &status); }; } // namespace lldb diff --git a/lldb/include/lldb/Symbol/SymbolFile.h b/lldb/include/lldb/Symbol/SymbolFile.h --- a/lldb/include/lldb/Symbol/SymbolFile.h +++ b/lldb/include/lldb/Symbol/SymbolFile.h @@ -222,6 +222,38 @@ virtual uint32_t ResolveSymbolContext(const Address &so_addr, lldb::SymbolContextItem resolve_scope, SymbolContext &sc) = 0; + + /// Get an error that describes why variables might be missing for a given + /// symbol context. + /// + /// If there is an error in the debug information that prevents variables from + /// being fetched, this error will get filled in. If there is no debug + /// informaiton, no error should be returned. But if there is debug + /// information and something prevents the variables from being available a + /// valid error should be returned. Valid cases include: + /// - compiler option that removes variables (-gline-tables-only) + /// - missing external files + /// - .dwo files in fission are not accessible or missing + /// - .o files on darwin when not using dSYM files that are not accessible + /// or missing + /// - mismatched exteral files + /// - .dwo files in fission where the DWO ID doesn't match + /// - .o files on darwin when modification timestamp doesn't match + /// - corrupted debug info + /// + /// \param[in] frame + /// The stack frame to use as a basis for the context to check. The frame + /// address can be used if there is not debug info due to it not being able + /// to be loaded, or if there is a debug info context, like a compile unit, + /// or function, it can be used to track down more information on why + /// variables are missing. + /// + /// \returns + /// An error specifying why there should have been debug info with variable + /// information but the variables were not able to be resolved. + virtual Status GetFrameVariableError(StackFrame &frame) { + return Status(); + } virtual uint32_t ResolveSymbolContext(const SourceLocationSpec &src_location_spec, lldb::SymbolContextItem resolve_scope, diff --git a/lldb/include/lldb/Target/StackFrame.h b/lldb/include/lldb/Target/StackFrame.h --- a/lldb/include/lldb/Target/StackFrame.h +++ b/lldb/include/lldb/Target/StackFrame.h @@ -254,9 +254,14 @@ /// that are visible to the entire compilation unit (e.g. file /// static in C, globals that are homed in this CU). /// + /// \param [out] error_ptr + /// If there is an error in the debug information that prevents variables + /// from being fetched. \see SymbolFile::GetFrameVariableError() for full + /// details. + /// /// \return /// A pointer to a list of variables. - VariableList *GetVariableList(bool get_file_globals); + VariableList *GetVariableList(bool get_file_globals, Status *error_ptr); /// Retrieve the list of variables that are in scope at this StackFrame's /// pc. diff --git a/lldb/packages/Python/lldbsuite/test/builders/builder.py b/lldb/packages/Python/lldbsuite/test/builders/builder.py --- a/lldb/packages/Python/lldbsuite/test/builders/builder.py +++ b/lldb/packages/Python/lldbsuite/test/builders/builder.py @@ -138,13 +138,14 @@ return None def getBuildCommand(self, debug_info, architecture=None, compiler=None, - dictionary=None, testdir=None, testname=None): + dictionary=None, testdir=None, testname=None, make_targets=None): debug_info_args = self._getDebugInfoArgs(debug_info) if debug_info_args is None: return None - + if make_targets is None: + make_targets = ["all"] command_parts = [ - self.getMake(testdir, testname), debug_info_args, ["all"], + self.getMake(testdir, testname), debug_info_args, make_targets, self.getArchCFlags(architecture), self.getArchSpec(architecture), self.getCCSpec(compiler), self.getExtraMakeArgs(), self.getSDKRootSpec(), self.getModuleCacheSpec(), diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py --- a/lldb/packages/Python/lldbsuite/test/lldbtest.py +++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py @@ -1411,7 +1411,8 @@ debug_info=None, architecture=None, compiler=None, - dictionary=None): + dictionary=None, + make_targets=None): """Platform specific way to build binaries.""" if not architecture and configuration.arch: architecture = configuration.arch @@ -1426,7 +1427,7 @@ module = builder_module() command = builder_module().getBuildCommand(debug_info, architecture, - compiler, dictionary, testdir, testname) + compiler, dictionary, testdir, testname, make_targets) if command is None: raise Exception("Don't know how to build binary") diff --git a/lldb/source/API/SBFrame.cpp b/lldb/source/API/SBFrame.cpp --- a/lldb/source/API/SBFrame.cpp +++ b/lldb/source/API/SBFrame.cpp @@ -602,7 +602,8 @@ &variable_list); if (value_type == eValueTypeVariableGlobal) { const bool get_file_globals = true; - VariableList *frame_vars = frame->GetVariableList(get_file_globals); + VariableList *frame_vars = frame->GetVariableList(get_file_globals, + nullptr); if (frame_vars) frame_vars->AppendVariablesIfUnique(variable_list); } @@ -805,7 +806,10 @@ frame = exe_ctx.GetFramePtr(); if (frame) { VariableList *variable_list = nullptr; - variable_list = frame->GetVariableList(true); + Status var_error; + variable_list = frame->GetVariableList(true, &var_error); + if (var_error.Fail()) + value_list.SetError(var_error); if (variable_list) { const size_t num_variables = variable_list->GetSize(); if (num_variables) { diff --git a/lldb/source/API/SBValueList.cpp b/lldb/source/API/SBValueList.cpp --- a/lldb/source/API/SBValueList.cpp +++ b/lldb/source/API/SBValueList.cpp @@ -7,11 +7,12 @@ //===----------------------------------------------------------------------===// #include "lldb/API/SBValueList.h" +#include "lldb/API/SBError.h" #include "lldb/API/SBStream.h" #include "lldb/API/SBValue.h" #include "lldb/Core/ValueObjectList.h" #include "lldb/Utility/Instrumentation.h" - +#include "lldb/Utility/Status.h" #include using namespace lldb; @@ -27,6 +28,7 @@ if (this == &rhs) return *this; m_values = rhs.m_values; + m_error = rhs.m_error; return *this; } @@ -63,8 +65,13 @@ return lldb::SBValue(); } + const Status &GetError() const { return m_error; } + + void SetError(const Status &error) { m_error = error; } + private: std::vector m_values; + Status m_error; }; SBValueList::SBValueList() { LLDB_INSTRUMENT_VA(this); } @@ -193,3 +200,15 @@ CreateIfNeeded(); return *m_opaque_up; } + +lldb::SBError SBValueList::GetError() { + LLDB_INSTRUMENT_VA(this); + SBError sb_error; + if (m_opaque_up) + sb_error.SetError(m_opaque_up->GetError()); + return sb_error; +} + +void SBValueList::SetError(const lldb_private::Status &status) { + ref().SetError(status); +} diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp --- a/lldb/source/Commands/CommandObjectFrame.cpp +++ b/lldb/source/Commands/CommandObjectFrame.cpp @@ -483,9 +483,14 @@ // might clear the StackFrameList for the thread. So hold onto a shared // pointer to the frame so it stays alive. + Status error; VariableList *variable_list = - frame->GetVariableList(m_option_variable.show_globals); + frame->GetVariableList(m_option_variable.show_globals, &error); + if (error.Fail() && (!variable_list || variable_list->GetSize() == 0)) { + result.AppendError(error.AsCString()); + + } VariableSP var_sp; ValueObjectSP valobj_sp; diff --git a/lldb/source/Core/IOHandlerCursesGUI.cpp b/lldb/source/Core/IOHandlerCursesGUI.cpp --- a/lldb/source/Core/IOHandlerCursesGUI.cpp +++ b/lldb/source/Core/IOHandlerCursesGUI.cpp @@ -5908,7 +5908,7 @@ if (m_frame_block != frame_block) { m_frame_block = frame_block; - VariableList *locals = frame->GetVariableList(true); + VariableList *locals = frame->GetVariableList(true, nullptr); if (locals) { const DynamicValueType use_dynamic = eDynamicDontRunTarget; for (const VariableSP &local_sp : *locals) { diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp @@ -866,7 +866,7 @@ // emit DW_AT_object_pointer // for C++ so it hasn't actually been tested. - VariableList *vars = frame->GetVariableList(false); + VariableList *vars = frame->GetVariableList(false, nullptr); lldb::VariableSP this_var = vars->FindVariable(ConstString("this")); @@ -951,7 +951,7 @@ // In that case, just look up the "self" variable in the current scope // and use its type. - VariableList *vars = frame->GetVariableList(false); + VariableList *vars = frame->GetVariableList(false, nullptr); lldb::VariableSP self_var = vars->FindVariable(ConstString("self")); diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h @@ -257,6 +257,15 @@ lldb_private::DWARFDataExtractor GetLocationData() const; + /// Returns true if any DIEs in the unit match any DW_TAG values in \a tags. + /// + /// \param[in] tags + /// An array of dw_tag_t values to check all abbrevitions for. + /// + /// \returns + /// True if any DIEs match any tag in \a tags, false otherwise. + bool HasAny(llvm::ArrayRef tags); + protected: DWARFUnit(SymbolFileDWARF &dwarf, lldb::user_id_t uid, const DWARFUnitHeader &header, diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp @@ -1060,3 +1060,18 @@ return maybe_offset.takeError(); return FindRnglistFromOffset(*maybe_offset); } + + +bool DWARFUnit::HasAny(llvm::ArrayRef tags) { + ExtractUnitDIEIfNeeded(); + if (m_dwo) + return m_dwo->HasAny(tags); + + for (const auto &die: m_die_array) { + for (const auto tag: tags) { + if (tag == die.Tag()) + return true; + } + } + return false; +} diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -163,6 +163,9 @@ lldb::SymbolContextItem resolve_scope, lldb_private::SymbolContext &sc) override; + lldb_private::Status + GetFrameVariableError(lldb_private::StackFrame &frame) override; + uint32_t ResolveSymbolContext( const lldb_private::SourceLocationSpec &src_location_spec, lldb::SymbolContextItem resolve_scope, diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -4121,3 +4121,27 @@ return m_index->GetIndexTime(); return {}; } + +Status SymbolFileDWARF::GetFrameVariableError(StackFrame &frame) { + std::lock_guard guard(GetModuleMutex()); + CompileUnit *cu = frame.GetSymbolContext(eSymbolContextCompUnit).comp_unit; + if (!cu) + return Status(); + + DWARFCompileUnit *dwarf_cu = GetDWARFCompileUnit(cu); + if (!dwarf_cu) + return Status(); + + // Don't return an error for assembly files as they typically don't have + // varaible information. + if (dwarf_cu->GetDWARFLanguageType() == DW_LANG_Mips_Assembler) + return Status(); + + // Check if this compile unit has any variable DIEs. If it doesn't then there + // is not variable information for the entire compile unit. + if (dwarf_cu->HasAny({DW_TAG_variable, DW_TAG_formal_parameter})) + return Status(); + + return Status("no variable information is available in debug info for this " + "compile unit"); +} diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h @@ -101,6 +101,10 @@ const lldb_private::SourceLocationSpec &src_location_spec, lldb::SymbolContextItem resolve_scope, lldb_private::SymbolContextList &sc_list) override; + + lldb_private::Status + GetFrameVariableError(lldb_private::StackFrame &frame) override; + void FindGlobalVariables(lldb_private::ConstString name, const lldb_private::CompilerDeclContext &parent_decl_ctx, @@ -168,6 +172,7 @@ lldb_private::FileSpec so_file; lldb_private::ConstString oso_path; llvm::sys::TimePoint<> oso_mod_time; + lldb_private::Status oso_load_error; OSOInfoSP oso_sp; lldb::CompUnitSP compile_unit_sp; uint32_t first_symbol_index = UINT32_MAX; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp @@ -31,6 +31,8 @@ #include "lldb/Symbol/VariableList.h" #include "llvm/Support/ScopedPrinter.h" +#include "lldb/Target/StackFrame.h" + #include "LogChannelDWARF.h" #include "SymbolFileDWARF.h" @@ -418,12 +420,14 @@ // modification timestamp, since it will never match. if (comp_unit_info->oso_mod_time != llvm::sys::TimePoint<>() && oso_mod_time != comp_unit_info->oso_mod_time) { - obj_file->GetModule()->ReportError( - "debug map object file '%s' has changed (actual time is " - "%s, debug map time is %s" - ") since this executable was linked, file will be ignored", - oso_file.GetPath().c_str(), llvm::to_string(oso_mod_time).c_str(), - llvm::to_string(comp_unit_info->oso_mod_time).c_str()); + comp_unit_info->oso_load_error.SetErrorStringWithFormat( + "debug map object file \"%s\" changed (actual: 0x%8.8x, debug " + "map: 0x%8.8x) since this executable was linked, debug info " + "will not be loaded", oso_file.GetPath().c_str(), + (uint32_t)llvm::sys::toTimeT(oso_mod_time), + (uint32_t)llvm::sys::toTimeT(comp_unit_info->oso_mod_time)); + obj_file->GetModule()->ReportError("%s", + comp_unit_info->oso_load_error.AsCString()); return nullptr; } @@ -432,6 +436,10 @@ if (!ObjectFile::SplitArchivePathWithObject(oso_path, oso_file, oso_object, must_exist)) { + comp_unit_info->oso_load_error.SetErrorStringWithFormat( + "debug map object file \"%s\" containing debug info does not " + "exist, debug info will not be loaded", + comp_unit_info->oso_path.GetCString()); return nullptr; } } @@ -454,6 +462,20 @@ obj_file->GetModule(), GetCompUnitInfoIndex(comp_unit_info), oso_file, oso_arch, oso_object ? &oso_object : nullptr, 0, oso_object ? comp_unit_info->oso_mod_time : llvm::sys::TimePoint<>()); + + if (!comp_unit_info->oso_sp->module_sp || !comp_unit_info->oso_sp->module_sp->GetObjectFile()) { + if (oso_object && FileSystem::Instance().Exists(oso_file)) { + // If we are loading a .o file from a .a file the "oso_object" will + // have a valid value name and if the .a file exists, either the .o + // file didn't exist in the .a file or the mod time didn't match. + comp_unit_info->oso_load_error.SetErrorStringWithFormat( + "\"%s\" object from the \"%s\" archive: " + "either the .o file doesn't exist in the archive or the " + "modification time (0x%8.8x) of the .o file doesn't match", + oso_object.AsCString(), oso_file.GetPath().c_str(), + (uint32_t)llvm::sys::toTimeT(comp_unit_info->oso_mod_time)); + } + } } } if (comp_unit_info->oso_sp) @@ -1430,3 +1452,48 @@ }); return oso_modules; } + +Status SymbolFileDWARFDebugMap::GetFrameVariableError(StackFrame &frame) { + std::lock_guard guard(GetModuleMutex()); + + // We need to make sure that our PC value from the frame matches the module + // for this object file since we will lookup the PC file address in the debug + // map below. + Address pc_addr = frame.GetFrameCodeAddress(); + if (pc_addr.GetModule() == m_objfile_sp->GetModule()) { + Symtab *symtab = m_objfile_sp->GetSymtab(); + if (symtab) { + const DebugMap::Entry *debug_map_entry = + m_debug_map.FindEntryThatContains(pc_addr.GetFileAddress()); + if (debug_map_entry) { + Symbol *symbol = + symtab->SymbolAtIndex(debug_map_entry->data.GetExeSymbolIndex()); + if (symbol) { + uint32_t oso_idx = 0; + CompileUnitInfo *comp_unit_info = + GetCompileUnitInfoForSymbolWithID(symbol->GetID(), &oso_idx); + if (comp_unit_info) { + Module *oso_module = GetModuleByCompUnitInfo(comp_unit_info); + if (oso_module) { + // Check the .o file's DWARF in case it has an error to display. + SymbolFile *oso_sym_file = oso_module->GetSymbolFile(); + if (oso_sym_file) + return oso_sym_file->GetFrameVariableError(frame); + } + // If we don't have a valid OSO module here, then something went + // wrong as we have a symbol for the address in the debug map, but + // we weren't able to open the .o file. Display an appropriate + // error + if (comp_unit_info->oso_load_error.Fail()) + return comp_unit_info->oso_load_error; + else + return Status("unable to load debug map object file \"%s\" " + "exist, debug info will not be loaded", + comp_unit_info->oso_path.GetCString()); + } + } + } + } + } + return Status(); +} diff --git a/lldb/source/Symbol/Variable.cpp b/lldb/source/Symbol/Variable.cpp --- a/lldb/source/Symbol/Variable.cpp +++ b/lldb/source/Symbol/Variable.cpp @@ -578,7 +578,8 @@ if (frame) { const bool get_file_globals = true; - VariableList *variable_list = frame->GetVariableList(get_file_globals); + VariableList *variable_list = frame->GetVariableList(get_file_globals, + nullptr); if (variable_list) { for (const VariableSP &var_sp : *variable_list) @@ -674,7 +675,7 @@ const bool get_file_globals = true; VariableList *variable_list = - frame->GetVariableList(get_file_globals); + frame->GetVariableList(get_file_globals, nullptr); if (!variable_list) break; diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp --- a/lldb/source/Target/StackFrame.cpp +++ b/lldb/source/Target/StackFrame.cpp @@ -20,6 +20,7 @@ #include "lldb/Symbol/Function.h" #include "lldb/Symbol/Symbol.h" #include "lldb/Symbol/SymbolContextScope.h" +#include "lldb/Symbol/SymbolFile.h" #include "lldb/Symbol/Type.h" #include "lldb/Symbol/VariableList.h" #include "lldb/Target/ABI.h" @@ -420,10 +421,12 @@ return m_sc; } -VariableList *StackFrame::GetVariableList(bool get_file_globals) { +VariableList *StackFrame::GetVariableList(bool get_file_globals, + Status *error_ptr) { std::lock_guard guard(m_mutex); if (m_flags.IsClear(RESOLVED_VARIABLES)) { m_flags.Set(RESOLVED_VARIABLES); + m_variable_list_sp = std::make_shared(); Block *frame_block = GetFrameBlock(); @@ -431,7 +434,6 @@ const bool get_child_variables = true; const bool can_create = true; const bool stop_if_child_block_is_inlined_function = true; - m_variable_list_sp = std::make_shared(); frame_block->AppendBlockVariables(can_create, get_child_variables, stop_if_child_block_is_inlined_function, [](Variable *v) { return true; }, @@ -455,6 +457,17 @@ } } + if (error_ptr && m_variable_list_sp->GetSize() == 0) { + // Check with the symbol file to check if there is an error for why we + // don't have variables that the user might need to know about. + GetSymbolContext(eSymbolContextEverything); + if (m_sc.module_sp) { + SymbolFile *sym_file = m_sc.module_sp->GetSymbolFile(); + if (sym_file) + *error_ptr = sym_file->GetFrameVariableError(*this); + } + } + return m_variable_list_sp.get(); } @@ -1147,16 +1160,16 @@ DynamicValueType use_dynamic) { ValueObjectSP valobj_sp; { // Scope for stack frame mutex. We need to drop this mutex before we figure - // out the dynamic value. That will require converting the StackID in the - // VO back to a StackFrame, which will in turn require locking the - // StackFrameList. If we still hold the StackFrame mutex, we could suffer - // lock inversion against the pattern of getting the StackFrameList and + // out the dynamic value. That will require converting the StackID in the + // VO back to a StackFrame, which will in turn require locking the + // StackFrameList. If we still hold the StackFrame mutex, we could suffer + // lock inversion against the pattern of getting the StackFrameList and // then the stack frame, which is fairly common. std::lock_guard guard(m_mutex); if (IsHistorical()) { return valobj_sp; } - VariableList *var_list = GetVariableList(true); + VariableList *var_list = GetVariableList(true, nullptr); if (var_list) { // Make sure the variable is a frame variable const uint32_t var_idx = var_list->FindIndexForVariable(variable_sp.get()); @@ -1698,7 +1711,7 @@ } const bool get_file_globals = false; - VariableList *variables = GetVariableList(get_file_globals); + VariableList *variables = GetVariableList(get_file_globals, nullptr); if (!variables) { return ValueObjectSP(); diff --git a/lldb/test/API/commands/frame/var/TestFrameVar.py b/lldb/test/API/commands/frame/var/TestFrameVar.py --- a/lldb/test/API/commands/frame/var/TestFrameVar.py +++ b/lldb/test/API/commands/frame/var/TestFrameVar.py @@ -6,8 +6,10 @@ import lldb import lldbsuite.test.lldbutil as lldbutil +from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * - +import os +import time class TestFrameVar(TestBase): @@ -82,4 +84,101 @@ self.assertIn("g_var", output, "Globals didn't find g_var") - + def check_frame_variable_errors(self, thread, error_strings): + command_result = lldb.SBCommandReturnObject() + interp = self.dbg.GetCommandInterpreter() + result = interp.HandleCommand("frame variable", command_result) + self.assertEqual(result, lldb.eReturnStatusFailed, + "frame var succeeded unexpectedly") + command_error = command_result.GetError() + + frame = thread.GetFrameAtIndex(0) + var_list = frame.GetVariables(True, True, False, True) + self.assertEqual(var_list.GetSize(), 0) + api_error = var_list.GetError().GetCString() + + for s in error_strings: + self.assertIn(s, command_error) + for s in error_strings: + self.assertIn(s, api_error) + + + @skipIfRemote + @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 an appropriate error when we + do 'frame variable' that explains why we aren't seeing variables. + ''' + self.build(debug_info="dwarf") + exe = self.getBuildArtifact("a.out") + main_obj = self.getBuildArtifact("main.o") + # 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) + + # We have to set a named breakpoint because we don't have any debug info + # because we deleted the main.o file. + (target, process, thread, bkpt) = lldbutil.run_to_name_breakpoint(self, 'main') + error_strings = [ + 'debug map object file "', + 'main.o" containing debug info does not exist, debug info will not be loaded' + ] + self.check_frame_variable_errors(thread, error_strings) + + + @skipIfRemote + @skipUnlessDarwin + def test_darwin_dwarf_obj_mod_time_mismatch(self): + ''' + Test that if we build a binary with DWARF in .o files and we update + the mod time of the .o file for main.cpp, that we get an appropriate + error when we do 'frame variable' that explains why we aren't seeing + variables. + ''' + self.build(debug_info="dwarf") + exe = self.getBuildArtifact("a.out") + main_obj = self.getBuildArtifact("main.o") + + # Set the modification time for main.o file to the current time after + # sleeping for 2 seconds. This ensures the modification time will have + # changed and will not match the modification time in the debug map and + # force an error when we run to main and try to get variables + time.sleep(2) + os.utime(main_obj, None) + + # We have to set a named breakpoint because we don't have any debug info + # because we deleted the main.o file since the mod times don't match + # and debug info won't be loaded + (target, process, thread, bkpt) = lldbutil.run_to_name_breakpoint(self, 'main') + + error_strings = [ + 'debug map object file "', + 'main.o" changed (actual: 0x', + ', debug map: 0x', + ') since this executable was linked, debug info will not be loaded' + ] + self.check_frame_variable_errors(thread, error_strings) + + + @skipIfRemote + @skipIfWindows # Windows can't set breakpoints by name 'main' in this case. + def test_gline_tables_only(self): + ''' + Test that if we build a binary with "-gline-tables-only" that we can + set a file and line breakpoint successfully, and get an error + letting us know that this build option was enabled when trying to + read variables. + ''' + self.build(dictionary={'CFLAGS_EXTRAS': '-gline-tables-only'}) + exe = self.getBuildArtifact("a.out") + + # We have to set a named breakpoint because we don't have any debug info + # because we deleted the main.o file since the mod times don't match + # and debug info won't be loaded + (target, process, thread, bkpt) = lldbutil.run_to_name_breakpoint(self, 'main') + error_strings = [ + 'no variable information is available in debug info for this compile unit' + ] + self.check_frame_variable_errors(thread, error_strings) diff --git a/lldb/test/API/functionalities/archives/Makefile b/lldb/test/API/functionalities/archives/Makefile --- a/lldb/test/API/functionalities/archives/Makefile +++ b/lldb/test/API/functionalities/archives/Makefile @@ -9,13 +9,11 @@ libfoo.a: a.o b.o $(AR) $(ARFLAGS) $@ $^ - $(RM) $^ # This tests whether lldb can load a thin archive libbar.a: c.o $(eval LLVM_AR := $(LLVM_TOOLS_DIR)/llvm-ar) $(eval LLVM_ARFLAGS := -rcsDT) $(LLVM_AR) $(LLVM_ARFLAGS) $@ $^ - # Note for thin archive case, we cannot remove c.o include Makefile.rules diff --git a/lldb/test/API/functionalities/archives/TestBSDArchives.py b/lldb/test/API/functionalities/archives/TestBSDArchives.py --- a/lldb/test/API/functionalities/archives/TestBSDArchives.py +++ b/lldb/test/API/functionalities/archives/TestBSDArchives.py @@ -6,10 +6,16 @@ from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * from lldbsuite.test import lldbutil - +import os +import time class BSDArchivesTestCase(TestBase): + # If your test case doesn't stress debug info, then + # set this to true. That way it won't be run once for + # each debug info format. + NO_DEBUG_INFO_TESTCASE = True + def setUp(self): # Call super's setUp(). TestBase.setUp(self) @@ -17,8 +23,6 @@ self.line = line_number( 'a.c', '// Set file and line breakpoint inside a().') - # Doesn't depend on any specific debug information. - @no_debug_info_test @expectedFailureAll( oslist=["windows"], bugnumber="llvm.org/pr24527. Makefile.rules doesn't know how to build static libs on Windows") @@ -65,3 +69,80 @@ num_specs = module_specs.GetSize() self.assertEqual(num_specs, 1) self.assertEqual(module_specs.GetSpecAtIndex(0).GetObjectName(), "c.o") + + + def check_frame_variable_errors(self, thread, error_strings): + command_result = lldb.SBCommandReturnObject() + interp = self.dbg.GetCommandInterpreter() + result = interp.HandleCommand("frame variable", command_result) + self.assertEqual(result, lldb.eReturnStatusFailed, + "frame var succeeded unexpectedly") + command_error = command_result.GetError() + + frame = thread.GetFrameAtIndex(0) + var_list = frame.GetVariables(True, True, False, True) + self.assertEqual(var_list.GetSize(), 0) + api_error = var_list.GetError().GetCString() + + for s in error_strings: + self.assertTrue(s in command_error, 'Make sure "%s" exists in the command error "%s"' % (s, command_error)) + for s in error_strings: + self.assertTrue(s in api_error, 'Make sure "%s" exists in the API error "%s"' % (s, api_error)) + + @skipIfRemote + @skipUnlessDarwin + def test_frame_var_errors_when_archive_missing(self): + """ + Break inside a() and remove libfoo.a to make sure we can't load + the debug information and report an appropriate error when doing + 'frame variable'. + """ + self.build() + exe = self.getBuildArtifact("a.out") + libfoo_path = self.getBuildArtifact("libfoo.a") + # 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 for the a() + # function. Since the libfoo.a is missing, the debug info won't be + # loaded and we should see an error when trying to read varibles. + os.unlink(libfoo_path) + + (target, process, thread, bkpt) = lldbutil.run_to_name_breakpoint( + self, 'a', bkpt_module=exe) + + error_strings = [ + 'debug map object file "', + 'libfoo.a(a.o)" containing debug info does not exist, debug info will not be loaded' + ] + self.check_frame_variable_errors(thread, error_strings) + + @skipIfRemote + @skipUnlessDarwin + def test_frame_var_errors_when_mtime_mistmatch_for_object_in_archive(self): + """ + Break inside a() and modify the modification time for "a.o" within + libfoo.a to make sure we can't load the debug information and + report an appropriate error when doing 'frame variable'. + """ + self.build() + exe = self.getBuildArtifact("a.out") + a_path = self.getBuildArtifact("a.o") + + # Change the modification time of the a.o object file after sleeping for + # 2 seconds to ensure the modification time is different. The rebuild + # only the "libfoo.a" target. This means the modification time of the + # a.o within libfoo.a will not match the debug map's modification time + # in a.out and will cause the debug information to not be loaded and we + # should get an appropriate error when reading variables. + time.sleep(2) + os.utime(a_path, None) + self.build(make_targets=["libfoo.a"]) + + (target, process, thread, bkpt) = lldbutil.run_to_name_breakpoint( + self, 'a', bkpt_module=exe) + + error_strings = [ + '"a.o" object from the "', + 'libfoo.a" archive: either the .o file doesn\'t exist in the archive or the modification time (0x', + ') of the .o file doesn\'t match' + ] + self.check_frame_variable_errors(thread, error_strings)