-
Notifications
You must be signed in to change notification settings - Fork 14.3k
Add target modules dump separate-debug-info
#66035
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-lldb ChangesSummary: Add a new command
or
(since This lists the separate debug info files and their current status (loaded or not loaded) for the specified modules. This diff implements this command for mach-O files with OSO and ELF files with dwo. Example dwo:
Example dwo with missing dwo:
Example output with dwp:
Example oso on my Mac (after manipulating the mod times with
Test Plan: Tested on Mac OS and Linux.
Reviewers: Subscribers: Tasks: Tags:Patch is 26.45 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/66035.diff 17 Files Affected:
diff --git a/lldb/include/lldb/Symbol/SymbolFile.h b/lldb/include/lldb/Symbol/SymbolFile.h index 8de752816cf94ee..347bfc445caa8b5 100644 --- a/lldb/include/lldb/Symbol/SymbolFile.h +++ b/lldb/include/lldb/Symbol/SymbolFile.h @@ -22,6 +22,7 @@ #include "lldb/Symbol/TypeList.h" #include "lldb/Symbol/TypeSystem.h" #include "lldb/Target/Statistics.h" +#include "lldb/Utility/StructuredData.h" #include "lldb/Utility/XcodeSDK.h" #include "lldb/lldb-private.h" #include "llvm/ADT/DenseSet.h" @@ -377,6 +378,10 @@ class SymbolFile : public PluginInterface { virtual void Dump(Stream &s) = 0; + /// Return true if separate debug info files are supported and this function + /// succeeded, false otherwise. + bool DumpSeparateDebugInfoFiles(StructuredData::Dictionary &d); + /// Metrics gathering functions /// Return the size in bytes of all debug information in the symbol file. @@ -459,6 +464,12 @@ class SymbolFile : public PluginInterface { virtual void GetCompileOptions( std::unordered_map &args) {} + /// Return true if separate debug info files are supported and this function + /// succeeded, false otherwise. + virtual bool GetSeparateDebugInfoFiles(StructuredData::Array &array) { + return false; + }; + private: SymbolFile(const SymbolFile &) = delete; const SymbolFile &operator=(const SymbolFile &) = delete; diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp index 33330ef0926d61f..276d85977c4ef34 100644 --- a/lldb/source/Commands/CommandObjectTarget.cpp +++ b/lldb/source/Commands/CommandObjectTarget.cpp @@ -52,6 +52,7 @@ #include "lldb/Utility/FileSpec.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/State.h" +#include "lldb/Utility/StructuredData.h" #include "lldb/Utility/Timer.h" #include "lldb/lldb-enumerations.h" #include "lldb/lldb-private-enumerations.h" @@ -1462,6 +1463,21 @@ static bool DumpModuleSymbolFile(Stream &strm, Module *module) { return false; } +static bool DumpModuleSeparateDebugInfoFiles( + StructuredData::Array &separate_debug_info_files, Module *module) { + if (module) { + if (SymbolFile *symbol_file = module->GetSymbolFile(true)) { + StructuredData::Dictionary d; + if (symbol_file->DumpSeparateDebugInfoFiles(d)) { + separate_debug_info_files.AddItem( + std::make_shared(std::move(d))); + return true; + } + } + } + return false; +} + static void DumpAddress(ExecutionContextScope *exe_scope, const Address &so_addr, bool verbose, bool all_ranges, Stream &strm) { @@ -2005,9 +2021,10 @@ class CommandObjectTargetModulesDumpSymtab result.GetOutputStream().EOL(); result.GetOutputStream().EOL(); } - if (INTERRUPT_REQUESTED(GetDebugger(), + if (INTERRUPT_REQUESTED(GetDebugger(), "Interrupted in dump all symtabs with {0} " - "of {1} dumped.", num_dumped, num_modules)) + "of {1} dumped.", + num_dumped, num_modules)) break; num_dumped++; @@ -2035,9 +2052,10 @@ class CommandObjectTargetModulesDumpSymtab result.GetOutputStream().EOL(); result.GetOutputStream().EOL(); } - if (INTERRUPT_REQUESTED(GetDebugger(), - "Interrupted in dump symtab list with {0} of {1} dumped.", - num_dumped, num_matches)) + if (INTERRUPT_REQUESTED( + GetDebugger(), + "Interrupted in dump symtab list with {0} of {1} dumped.", + num_dumped, num_matches)) break; num_dumped++; @@ -2099,9 +2117,10 @@ class CommandObjectTargetModulesDumpSections result.GetOutputStream().Format("Dumping sections for {0} modules.\n", num_modules); for (size_t image_idx = 0; image_idx < num_modules; ++image_idx) { - if (INTERRUPT_REQUESTED(GetDebugger(), - "Interrupted in dump all sections with {0} of {1} dumped", - image_idx, num_modules)) + if (INTERRUPT_REQUESTED( + GetDebugger(), + "Interrupted in dump all sections with {0} of {1} dumped", + image_idx, num_modules)) break; num_dumped++; @@ -2120,9 +2139,10 @@ class CommandObjectTargetModulesDumpSections FindModulesByName(target, arg_cstr, module_list, true); if (num_matches > 0) { for (size_t i = 0; i < num_matches; ++i) { - if (INTERRUPT_REQUESTED(GetDebugger(), - "Interrupted in dump section list with {0} of {1} dumped.", - i, num_matches)) + if (INTERRUPT_REQUESTED( + GetDebugger(), + "Interrupted in dump section list with {0} of {1} dumped.", + i, num_matches)) break; Module *module = module_list.GetModulePointerAtIndex(i); @@ -2265,9 +2285,10 @@ class CommandObjectTargetModulesDumpClangAST } for (size_t i = 0; i < num_matches; ++i) { - if (INTERRUPT_REQUESTED(GetDebugger(), - "Interrupted in dump clang ast list with {0} of {1} dumped.", - i, num_matches)) + if (INTERRUPT_REQUESTED( + GetDebugger(), + "Interrupted in dump clang ast list with {0} of {1} dumped.", i, + num_matches)) break; Module *m = module_list.GetModulePointerAtIndex(i); @@ -2406,10 +2427,10 @@ class CommandObjectTargetModulesDumpLineTable if (num_modules > 0) { uint32_t num_dumped = 0; for (ModuleSP module_sp : target_modules.ModulesNoLocking()) { - if (INTERRUPT_REQUESTED(GetDebugger(), + if (INTERRUPT_REQUESTED(GetDebugger(), "Interrupted in dump all line tables with " - "{0} of {1} dumped", num_dumped, - num_modules)) + "{0} of {1} dumped", + num_dumped, num_modules)) break; if (DumpCompileUnitLineTable( @@ -2462,6 +2483,93 @@ class CommandObjectTargetModulesDumpLineTable CommandOptions m_options; }; +#pragma mark CommandObjectTargetModulesDumpSeparateDebugInfoFiles + +// Image debug dwo dumping command + +class CommandObjectTargetModulesDumpSeparateDebugInfoFiles + : public CommandObjectTargetModulesModuleAutoComplete { +public: + CommandObjectTargetModulesDumpSeparateDebugInfoFiles( + CommandInterpreter &interpreter) + : CommandObjectTargetModulesModuleAutoComplete( + interpreter, "target modules dump separate-debug-info", + "Dump the separate debug info symbol files for one or more target " + "modules.", + //"target modules dump separate-debug-info [ ...]") + nullptr, eCommandRequiresTarget) {} + + ~CommandObjectTargetModulesDumpSeparateDebugInfoFiles() override = default; + +protected: + bool DoExecute(Args &command, CommandReturnObject &result) override { + Target &target = GetSelectedTarget(); + uint32_t num_dumped = 0; + + uint32_t addr_byte_size = target.GetArchitecture().GetAddressByteSize(); + result.GetOutputStream().SetAddressByteSize(addr_byte_size); + result.GetErrorStream().SetAddressByteSize(addr_byte_size); + + StructuredData::Array separate_debug_info_files; + if (command.GetArgumentCount() == 0) { + // Dump all sections for all modules images + const ModuleList &target_modules = target.GetImages(); + std::lock_guard guard(target_modules.GetMutex()); + const size_t num_modules = target_modules.GetSize(); + if (num_modules == 0) { + result.AppendError("the target has no associated executable images"); + return false; + } + for (ModuleSP module_sp : target_modules.ModulesNoLocking()) { + if (INTERRUPT_REQUESTED( + GetDebugger(), + "Interrupted in dumping all " + "separate debug info with {0} of {1} modules dumped", + num_dumped, num_modules)) + break; + + if (DumpModuleSeparateDebugInfoFiles(separate_debug_info_files, + module_sp.get())) + num_dumped++; + } + } else { + // Dump specified images (by basename or fullpath) + const char *arg_cstr; + for (int arg_idx = 0; + (arg_cstr = command.GetArgumentAtIndex(arg_idx)) != nullptr; + ++arg_idx) { + ModuleList module_list; + const size_t num_matches = + FindModulesByName(&target, arg_cstr, module_list, true); + if (num_matches > 0) { + for (size_t i = 0; i < num_matches; ++i) { + if (INTERRUPT_REQUESTED(GetDebugger(), + "Interrupted dumping {0} " + "of {1} requested modules", + i, num_matches)) + break; + Module *module = module_list.GetModulePointerAtIndex(i); + if (DumpModuleSeparateDebugInfoFiles(separate_debug_info_files, + module)) + num_dumped++; + } + } else + result.AppendWarningWithFormat( + "Unable to find an image that matches '%s'.\n", arg_cstr); + } + } + + if (num_dumped > 0) { + separate_debug_info_files.Dump(result.GetOutputStream(), + /* pretty_print */ true); + result.SetStatus(eReturnStatusSuccessFinishResult); + } else { + result.AppendError("no matching executable images found"); + } + return result.Succeeded(); + } +}; + #pragma mark CommandObjectTargetModulesDump // Dump multi-word command for target modules @@ -2499,6 +2607,10 @@ class CommandObjectTargetModulesDump : public CommandObjectMultiword { "pcm-info", CommandObjectSP( new CommandObjectTargetModulesDumpClangPCMInfo(interpreter))); + LoadSubCommand("separate-debug-info", + CommandObjectSP( + new CommandObjectTargetModulesDumpSeparateDebugInfoFiles( + interpreter))); } ~CommandObjectTargetModulesDump() override = default; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 04c729e333a9854..695f6b43f9e4331 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -10,6 +10,7 @@ #include "llvm/DebugInfo/DWARF/DWARFDebugLoc.h" #include "llvm/Support/Casting.h" +#include "llvm/Support/Format.h" #include "llvm/Support/Threading.h" #include "lldb/Core/Module.h" @@ -24,6 +25,7 @@ #include "lldb/Utility/RegularExpression.h" #include "lldb/Utility/Scalar.h" #include "lldb/Utility/StreamString.h" +#include "lldb/Utility/StructuredData.h" #include "lldb/Utility/Timer.h" #include "Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.h" @@ -4214,6 +4216,62 @@ void SymbolFileDWARF::DumpClangAST(Stream &s) { clang->Dump(s.AsRawOstream()); } +bool SymbolFileDWARF::GetSeparateDebugInfoFiles(StructuredData::Array &array) { + DWARFDebugInfo &info = DebugInfo(); + const size_t num_cus = info.GetNumUnits(); + for (size_t cu_idx = 0; cu_idx < num_cus; cu_idx++) { + DWARFUnit *unit = info.GetUnitAtIndex(cu_idx); + DWARFCompileUnit *dwarf_cu = llvm::dyn_cast(unit); + if (dwarf_cu == nullptr) { + continue; + } + + // Check if this is a DWO unit by checking if it has a DWO ID. + // NOTE: it seems that `DWARFUnit::IsDWOUnit` is always false? + if (!dwarf_cu->GetDWOId().has_value()) + continue; + + StructuredData::DictionarySP dwo_data = + std::make_shared(); + const uint64_t dwo_id = dwarf_cu->GetDWOId().value(); + dwo_data->AddIntegerItem("dwo_id", dwo_id); + + if (const DWARFBaseDIE die = dwarf_cu->GetUnitDIEOnly()) { + const char *dwo_name = GetDWOName(*dwarf_cu, *die.GetDIE()); + if (dwo_name) { + dwo_data->AddStringItem("dwo_name", dwo_name); + } else { + dwo_data->AddStringItem("error", "missing dwo name"); + } + + const char *comp_dir = die.GetDIE()->GetAttributeValueAsString( + dwarf_cu, DW_AT_comp_dir, nullptr); + if (comp_dir) { + dwo_data->AddStringItem("comp_dir", comp_dir); + } + } else { + dwo_data->AddStringItem( + "error", + llvm::formatv("unable to get unit DIE for DWARFUnit at {0:x}", + dwarf_cu->GetOffset()) + .str()); + } + + // If we have a DWO symbol file, that means we were able to successfully + // load it. + SymbolFile *dwo_symfile = dwarf_cu->GetDwoSymbolFile(); + if (dwo_symfile) { + dwo_data->AddStringItem( + "resolved_dwo_path", + dwo_symfile->GetObjectFile()->GetFileSpec().GetPath()); + } + dwo_data->AddBooleanItem("loaded", dwo_symfile != nullptr); + array.AddItem(dwo_data); + } + + return true; +} + SymbolFileDWARFDebugMap *SymbolFileDWARF::GetDebugMapSymfile() { if (m_debug_map_symfile == nullptr) { lldb::ModuleSP module_sp(m_debug_map_module_wp.lock()); diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h index 191a5abcf265abd..79bcdbbba5d5963 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -30,6 +30,7 @@ #include "lldb/Utility/ConstString.h" #include "lldb/Utility/Flags.h" #include "lldb/Utility/RangeMap.h" +#include "lldb/Utility/StructuredData.h" #include "lldb/lldb-private.h" #include "DWARFContext.h" @@ -282,6 +283,10 @@ class SymbolFileDWARF : public lldb_private::SymbolFileCommon { void DumpClangAST(lldb_private::Stream &s) override; + /// Retrieve the external dwo files. + bool GetSeparateDebugInfoFiles( + lldb_private::StructuredData::Array &array) override; + lldb_private::DWARFContext &GetDWARFContext() { return m_context; } const std::shared_ptr &GetDwpSymbolFile(); diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp index eadedd32e1a4aaf..dd9add83f0d4267 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp @@ -18,8 +18,9 @@ #include "lldb/Host/FileSystem.h" #include "lldb/Utility/RangeMap.h" #include "lldb/Utility/RegularExpression.h" -#include "lldb/Utility/Timer.h" #include "lldb/Utility/StreamString.h" +#include "lldb/Utility/StructuredData.h" +#include "lldb/Utility/Timer.h" //#define DEBUG_OSO_DMAP // DO NOT CHECKIN WITH THIS NOT COMMENTED OUT @@ -1271,6 +1272,36 @@ void SymbolFileDWARFDebugMap::DumpClangAST(Stream &s) { }); } +bool SymbolFileDWARFDebugMap::GetSeparateDebugInfoFiles( + lldb_private::StructuredData::Array &array) { + const uint32_t cu_count = GetNumCompileUnits(); + for (uint32_t cu_idx = 0; cu_idx < cu_count; ++cu_idx) { + const auto &info = m_compile_unit_infos[cu_idx]; + StructuredData::DictionarySP oso_data = + std::make_shared(); + oso_data->AddStringItem("so_file", info.so_file.GetPath()); + oso_data->AddStringItem("oso_path", info.oso_path); + oso_data->AddIntegerItem("oso_mod_time", + (uint32_t)llvm::sys::toTimeT(info.oso_mod_time)); + + bool loaded_successfully = false; + if (GetModuleByOSOIndex(cu_idx)) { + // If we have a valid pointer to the module, we successfully + // loaded the oso if there are no load errors. + if (!info.oso_load_error.Fail()) { + loaded_successfully = true; + } + } + if (!loaded_successfully) { + oso_data->AddStringItem("error", info.oso_load_error.AsCString()); + } + oso_data->AddBooleanItem("loaded", loaded_successfully); + array.AddItem(oso_data); + } + + return true; +} + lldb::CompUnitSP SymbolFileDWARFDebugMap::GetCompileUnit(SymbolFileDWARF *oso_dwarf, DWARFCompileUnit &dwarf_cu) { if (oso_dwarf) { diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h index 881fd4c45ff05a0..b486e82eab73d20 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h @@ -19,6 +19,7 @@ #include #include "UniqueDWARFASTType.h" +#include "lldb/Utility/StructuredData.h" class SymbolFileDWARF; class DWARFCompileUnit; @@ -148,6 +149,10 @@ class SymbolFileDWARFDebugMap : public lldb_private::SymbolFileCommon { void DumpClangAST(lldb_private::Stream &s) override; + /// Get the external oso files. + bool GetSeparateDebugInfoFiles( + lldb_private::StructuredData::Array &array) override; + // PluginInterface protocol llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); } diff --git a/lldb/source/Symbol/SymbolFile.cpp b/lldb/source/Symbol/SymbolFile.cpp index b271efd07bfe36f..ad02a8b2c861907 100644 --- a/lldb/source/Symbol/SymbolFile.cpp +++ b/lldb/source/Symbol/SymbolFile.cpp @@ -18,6 +18,7 @@ #include "lldb/Symbol/VariableList.h" #include "lldb/Utility/Log.h" #include "lldb/Utility/StreamString.h" +#include "lldb/Utility/StructuredData.h" #include "lldb/lldb-private.h" #include @@ -162,6 +163,19 @@ void SymbolFile::AssertModuleLock() { SymbolFile::RegisterInfoResolver::~RegisterInfoResolver() = default; +bool SymbolFile::DumpSeparateDebugInfoFiles(StructuredData::Dictionary &d) { + StructuredData::Array array; + if (!GetSeparateDebugInfoFiles(array)) { + return false; + } + + d.AddStringItem("symfile", GetMainObjectFile()->GetFileSpec().GetPath()); + d.AddItem("separate-debug-info-files", + std::make_shared(std::move(array))); + + return true; +} + Symtab *SymbolFileCommon::GetSymtab() { std::lock_guard guard(GetModuleMutex()); // Fetch the symtab from the main object file. diff --git a/lldb/test/API/commands/target/dump-separate-debug-info/dwo/Makefile b/lldb/test/API/commands/target/dump-separate-debug-info/dwo/Makefile new file mode 100644 index 000000000000000..3b6d788b2b0130a --- /dev/null +++ b/lldb/test/API/comm... |
3a7096f
to
0f4cf36
Compare
I don't think the lldb command line dumps raw JSON anywhere as a command result. Can we make something a little more human readable? |
BTW, the idea seems useful to me, but particularly if you're doing OSO you can get a whole lot of these so a space efficient presentation where you can scan the names easily, etc, will be important as well. |
It was my idea to make this JSON. The problem is each debug info (.dwo and DWARF in .o files for Mac) has very different fields and pretty printing these items free form looked like it should be JSON. For DWO we have the DW_AT_dwo_name, DW_AT_comp_dir, and DW_AT_dwo_id to display, and for OSO we have .o file path + mod time. I wanted the ability to be able to parse the output and use it in a script so the JSON was nice for that end game. It also allows us to include this information in "statistics dump".
Maybe we can add an extra "flavor" to the JSON output, and then have a format string for how the output can look akin to the "frame-format" that the command would use by default? That would allow us to format the output in a nice human readable way for the command output, and also add a --json option to the command. The format string could use the "flavor" ("oso" or "dwo") to format the output string as needed? |
We also want to show the .dwo error or OSO error strings that explain why the file wasn't loaded as we see when we do "frame variable" and the OSO or DWO are missing. |
@@ -282,6 +283,10 @@ class SymbolFileDWARF : public lldb_private::SymbolFileCommon { | |||
|
|||
void DumpClangAST(lldb_private::Stream &s) override; | |||
|
|||
/// Retrieve the external dwo files. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"external dwo file details." ?
FYI I committed https://reviews.llvm.org/D157609 recently. Looks like your test cases are pretty straightforward but just in case you are confused why lldb manages to find a file when it didn't previously. |
Emitting it as JSON seems a fine option for scripting. If you know that this is an array of dictionaries with all the same keys (that's true for an given output, right?), it shouldn't be hard to write a generic "JSON -> table" routine to convert the output. Having that driven by a format is an interesting idea, but you'd have to standardize the key values at least somewhat for that to be generally useful. |
d0538bc
to
94b834f
Compare
Updated to show a more human-readable format @jimingham. |
94b834f
to
2bb9a2a
Compare
Addressed @clayborg's comments: removed trailing whitespaces, simplify the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just need tests for the non --json output for OSO and DWO. If you can also attach the output of the 4 cases (OSO no errors, OSO with errors, DWO no errors and DWO with errors), that might help people see what this output looks like. If you already added the output into the comments disregard!
5bf6c4a
to
9a52ac5
Compare
Added tests for the human-readable/table format. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be easier to read the output if you made a separate column for "error". If you imagine a listing where there are 100 OSO's of which 10 are missing, the paths are going to jump over because of the "error: ..." at the beginning of those lines making it messy for your eyes to track. Seems like there are only two possible errors "file not found" and "file out of date", so it seems like we should be able to make a fairly short error column after the ID and then the path?
If you're tired of mucking with tables at this point, this isn't required, but I think it would make it easier to scan.
The errors are quite long as they mention the offending path to the OSO and the OSO paths are often very long, so you would need to run through the results to calculate the max width of the OSO path and then you would almost certainly overlflow off the right edge of the terminal if you also show the error string. This patch currently doesn't show the OSO path if there is an error , so we show the error in place of the OSO path as the error string contains the path in the error text. Would your prefer to always show the modtime + oso path and then add the error on the next line? |
I was imaging something like:
Or given that there aren't many errors possible, you could even do a one letter thing If the error was of the form "Not found: ..." or "Out of data:..." then you could pick off the error string. You're in charge of the error string so you can arrange to emit it in a way that allows picking this info out of it easily. And you don't need the path in the error since it's in the next "path" field anyway. |
It might be nice having a separate error column with just a one letter thing for now, maybe with just two options "S" and "E" for now. The errors are just arbitrary strings decided by the DWO/OSO-specific logic at the moment -- if we wanted to categorize all of the errors properly, that would probably be outside the scope of this PR? |
People can either look at the detailed JSON output or just go look at that path to figure out what went wrong, so just S & E would be fine for now.
Jim
… On Sep 15, 2023, at 11:18 AM, Tom Yang ***@***.***> wrote:
I think it would be easier to read the output if you made a separate column for "error". If you imagine a listing where there are 100 OSO's of which 10 are missing, the paths are going to jump over because of the "error: ..." at the beginning of those lines making it messy for your eyes to track. Seems like there are only two possible errors "file not found" and "file out of date", so it seems like we should be able to make a fairly short error column after the ID and then the path?
The errors are quite long as they mention the offending path to the OSO and the OSO paths are often very long, so you would need to run through the results to calculate the max width of the OSO path and then you would almost certainly overlflow off the right edge of the terminal if you also show the error string.
This patch currently doesn't show the OSO path if there is an error , so we show the error in place of the OSO path as the error string contains the path in the error text. Would your prefer to always show the modtime + oso path and then add the error on the next line?
I was imaging something like:
(lldb) image dump separate-debug-info
Symbol file: /home/toyang/workspace/dwo-scratch/a.out
Type: "dwo"
Dwo ID Error Dwo Path
------------------ ----------- -----------------------------------------
0x9a429da5abb6faae Missing "/home/toyang/workspace/dwo-scratch/a-main.dwo" for skeleton DIE 0x0000000000000014
0xbcc129959e76ff33 Missing "/home/toyang/workspace/dwo-scratch/a-foo.dwo" for skeleton DIE 0x000000000000003c
Or given that there aren't many errors possible, you could even do a one letter thing S for success, M for missing and O for out of date.
If the error was of the form "Not found: ..." or "Out of data:..." then you could pick off the error string. You're in charge of the error string so you can arrange to emit it in a way that allows picking this info out of it easily. And you don't need the path in the error since it's in the next "path" field anyway.
It might be nice having a separate error column with just a one letter thing for now, maybe with just two options "S" and "E" for now. The errors are just arbitrary strings decided by the DWO/OSO-specific logic at the moment -- if we wanted to categorize all of the errors properly, that would probably be outside the scope of this PR?
—
Reply to this email directly, view it on GitHub <#66035 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADUPVW5GYSWCVVIM5QRE3PLX2SLYLANCNFSM6AAAAAA4UCOLGY>.
You are receiving this because you were mentioned.
|
(aside: isn't the SBAPI meant to be the thing to use if you want to script stuff, rather than having formatted output/scraping that? - but sounds like the output has converged on a table rather than JSON anyway? So maybe a moot point) Re: errors, OSO (what does that stand for anyway) - yeah, maybe separate tables for different data? (one table for OSO, one for DWO, one for errors? (or two for errors, one for OSO errors and one for DWO errors?)) |
N_SO is the stab moniker for a source file, and N_OSO is the object file associated with that source file (N_SO was in Sun's implementation, we made up N_OSO). Most nm''s leave off the N_ when they print stabs, so then this became just OSO...
We don't do DWO on Darwin, and nobody but Darwin does OSO's, so for now I don't think you need to worry about mixing of the entities in these reports.
Jim
… On Sep 20, 2023, at 3:10 PM, David Blaikie ***@***.***> wrote:
(aside: isn't the SBAPI meant to be the thing to use if you want to script stuff, rather than having formatted output/scraping that? - but sounds like the output has converged on a table rather than JSON anyway? So maybe a moot point)
Re: errors, OSO (what does that stand for anyway) - yeah, maybe separate tables for different data? (one table for OSO, one for DWO, one for errors? (or two for errors, one for OSO errors and one for DWO errors?))
—
Reply to this email directly, view it on GitHub <#66035 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADUPVW7ZIZJ2T6I6RHFFENLX3NSWVANCNFSM6AAAAAA4UCOLGY>.
You are receiving this because you were mentioned.
|
Ah, thanks for the context - makes sense. |
I am not a big fan of having a specialized name How about we rename this as a more generic command name (like
The benefit is that this provides a central command to dump all symbol file status instead of several commands to gather info. I do not think the above should be included in this patch but at least the command name opens the possibility for extension in future. Another option is treating What do you guys think? |
Summary: Add a new command ``` target modules dump separate-debug-info [-j] [<filename> [<filename> [...]]] ``` or ``` image dump separate-debug-info [-j] [<filename> [<filename> [...]]] ``` (since `image` is an alias for `target modules`). This lists the separate debug info files and their current status (loaded or not loaded) for the specified modules. This diff implements this command for mach-O files with OSO and ELF files with dwo. Example dwo: ``` (lldb) image dump separate-debug-info Symbol file: /home/toyang/workspace/dwo-scratch/a.out Type: "dwo" Dwo ID Dwo Path ------------------ ----------------------------------------- 0x9a429da5abb6faae /home/toyang/workspace/dwo-scratch/a-main.dwo 0xbcc129959e76ff33 /home/toyang/workspace/dwo-scratch/a-foo.dwo (lldb) image dump separate-debug-info -j [ { "separate-debug-info-files": [ { "comp_dir": "/home/toyang/workspace/dwo-scratch", "dwo_id": 11115620165179865774, "dwo_name": "a-main.dwo", "loaded": true, "resolved_dwo_path": "/home/toyang/workspace/dwo-scratch/a-main.dwo" }, { "comp_dir": "/home/toyang/workspace/dwo-scratch", "dwo_id": 13601198072221073203, "dwo_name": "a-foo.dwo", "loaded": true, "resolved_dwo_path": "/home/toyang/workspace/dwo-scratch/a-foo.dwo" } ], "symfile": "/home/toyang/workspace/dwo-scratch/a.out", "type": "dwo" } ] ``` Example dwo with missing dwo: ``` (lldb) image dump separate-debug-info Symbol file: /home/toyang/workspace/dwo-scratch/a.out Type: "dwo" Dwo ID Dwo Path ------------------ ----------------------------------------- 0x9a429da5abb6faae error: unable to locate .dwo debug file "/home/toyang/workspace/dwo-scratch/a-main.dwo" for skeleton DIE 0x0000000000000014 0xbcc129959e76ff33 error: unable to locate .dwo debug file "/home/toyang/workspace/dwo-scratch/a-foo.dwo" for skeleton DIE 0x000000000000003c (lldb) image dump separate-debug-info -j [ { "separate-debug-info-files": [ { "comp_dir": "/home/toyang/workspace/dwo-scratch", "dwo_id": 11115620165179865774, "dwo_name": "a-main.dwo", "error": "unable to locate .dwo debug file \"/home/toyang/workspace/dwo-scratch/a-main.dwo\" for skeleton DIE 0x0000000000000014", "loaded": false }, { "comp_dir": "/home/toyang/workspace/dwo-scratch", "dwo_id": 13601198072221073203, "dwo_name": "a-foo.dwo", "error": "unable to locate .dwo debug file \"/home/toyang/workspace/dwo-scratch/a-foo.dwo\" for skeleton DIE 0x000000000000003c", "loaded": false } ], "symfile": "/home/toyang/workspace/dwo-scratch/a.out", "type": "dwo" } ] ``` Example output with dwp: ``` (lldb) image dump separate-debug-info Symbol file: /home/toyang/workspace/dwo-scratch/a.out Type: "dwo" Dwo ID Dwo Path ------------------ ----------------------------------------- 0x9a429da5abb6faae /home/toyang/workspace/dwo-scratch/a.out.dwp(a-main.dwo) 0xbcc129959e76ff33 /home/toyang/workspace/dwo-scratch/a.out.dwp(a-foo.dwo) (lldb) image dump separate-debug-info -j [ { "separate-debug-info-files": [ { "comp_dir": "/home/toyang/workspace/dwo-scratch", "dwo_id": 11115620165179865774, "dwo_name": "a-main.dwo", "loaded": true, "resolved_dwo_path": "/home/toyang/workspace/dwo-scratch/a.out.dwp" }, { "comp_dir": "/home/toyang/workspace/dwo-scratch", "dwo_id": 13601198072221073203, "dwo_name": "a-foo.dwo", "loaded": true, "resolved_dwo_path": "/home/toyang/workspace/dwo-scratch/a.out.dwp" } ], "symfile": "/home/toyang/workspace/dwo-scratch/a.out", "type": "dwo" } ] ``` Example oso on my Mac (after manipulating the mod times with `touch`): ``` (lldb) image dump separate-debug-info Symbol file: /Users/toyang/workspace/scratch/a.out Type: "oso" Mod Time Oso Path ------------------ --------------------- 0x0000000064e64868 /Users/toyang/workspace/scratch/foo.a(foo.o) 0x0000000064e64868 /Users/toyang/workspace/scratch/foo.a(main.o) (lldb) image dump separate-debug-info -j [ { "separate-debug-info-files": [ { "loaded": true, "oso_mod_time": 1692813416, "oso_path": "/Users/toyang/workspace/scratch/foo.a(foo.o)", "so_file": "/Users/toyang/workspace/scratch/foo.cpp" }, { "loaded": true, "oso_mod_time": 1692813416, "oso_path": "/Users/toyang/workspace/scratch/foo.a(main.o)", "so_file": "/Users/toyang/workspace/scratch/main.cpp" } ], "symfile": "/Users/toyang/workspace/scratch/a.out", "type": "oso" } ] ``` Test Plan: Tested on Mac OS and Linux. ``` lldb-dotest -p TestDumpDwo lldb-dotest -p TestDumpOso ``` Reviewers: Subscribers: Tasks: Tags:
I would rather keep this kind of thing separate as the format we want to output here. It would be find to go and modify these other commands to include exceptional issues found from the structured data and fit the data into the output structure of each of those commands if they can use it. |
c30657d
to
d6a3859
Compare
The reason we use StructuredData internally are for a few reasons:
The structured data to text in the command uses different tables now. |
Add a new command
or
(since
image
is an alias fortarget modules
).This lists the separate debug info files and their current status (loaded or not loaded) for the specified modules. This diff implements this command for mach-O files with OSO and ELF files with dwo.
Example dwo:
Example dwo with missing dwo:
Example output with dwp:
Example oso on my Mac:
Test Plan:
Tested on Mac OS and Linux.