Skip to content

[lldb] add plugin names to process save-core error output. #143126

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

Merged
merged 3 commits into from
Jun 23, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lldb/include/lldb/Core/PluginManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,8 @@ class PluginManager {
static Status SaveCore(const lldb::ProcessSP &process_sp,
lldb_private::SaveCoreOptions &core_options);

static std::vector<llvm::StringRef> GetSaveCorePluginNames();

// ObjectContainer
static bool RegisterPlugin(
llvm::StringRef name, llvm::StringRef description,
Expand Down
24 changes: 23 additions & 1 deletion lldb/source/Commands/CommandObjectProcess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1281,7 +1281,27 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed {
~CommandOptions() override = default;

llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
return llvm::ArrayRef(g_process_save_core_options);
if (!m_opt_def.empty())
return llvm::ArrayRef(m_opt_def);

auto orig = llvm::ArrayRef(g_process_save_core_options);
m_opt_def.resize(orig.size());
llvm::copy(g_process_save_core_options, m_opt_def.data());
for (OptionDefinition &value : m_opt_def) {
llvm::StringRef opt_name = value.long_option;
if (opt_name != "plugin-name")
continue;

std::vector<llvm::StringRef> plugin_names =
PluginManager::GetSaveCorePluginNames();
m_plugin_enums.resize(plugin_names.size());
for (auto [num, val] : llvm::zip(plugin_names, m_plugin_enums)) {
val.string_value = num.data();
}
value.enum_values = llvm::ArrayRef(m_plugin_enums);
break;
}
return llvm::ArrayRef(m_opt_def);
}

Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
Expand Down Expand Up @@ -1312,6 +1332,8 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed {

// Instance variables to hold the values for command options.
SaveCoreOptions m_core_dump_options;
llvm::SmallVector<OptionEnumValueElement> m_plugin_enums;
std::vector<OptionDefinition> m_opt_def;
};

protected:
Expand Down
27 changes: 22 additions & 5 deletions lldb/source/Core/PluginManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -843,11 +843,28 @@ Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp,
}

// Check to see if any of the object file plugins tried and failed to save.
// If none ran, set the error message.
if (error.Success())
error = Status::FromErrorString(
"no ObjectFile plugins were able to save a core for this process");
return error;
// if any failure, return the error message.
if (error.Fail())
return error;

// Report only for the plugin that was specified.
if (!plugin_name.empty())
return Status::FromErrorStringWithFormatv(
"The \"{}\" plugin is not able to save a core for this process.",
plugin_name);

return Status::FromErrorString(
"no ObjectFile plugins were able to save a core for this process");
}
Comment on lines +850 to +858
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two errors don't have test coverage.

"The "{}" plugin is not able to save a core for this process."

To hit this one we'd need a plugin that can save a core, but not for this process.

"no ObjectFile plugins were able to save a core for this process"

Same thing but there needs to be > 1 plugin that tried.

And the original error doesn't have coverage either, presumably because no one has the appetite to inject a problem into a process. Which is fair enough.

So yes these don't have coverage but it's not the end of the world if someone gets the wrong one so I'm not going to demand tests for them.


std::vector<llvm::StringRef> PluginManager::GetSaveCorePluginNames() {
std::vector<llvm::StringRef> plugin_names;
auto instances = GetObjectFileInstances().GetSnapshot();
for (auto &instance : instances) {
if (instance.save_core)
plugin_names.emplace_back(instance.name);
}
return plugin_names;
}

#pragma mark ObjectContainer
Expand Down
17 changes: 14 additions & 3 deletions lldb/source/Symbol/SaveCoreOptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,20 @@ Status SaveCoreOptions::SetPluginName(const char *name) {
return error;
}

if (!PluginManager::IsRegisteredObjectFilePluginName(name)) {
return Status::FromErrorStringWithFormat(
"plugin name '%s' is not a valid ObjectFile plugin name", name);
std::vector<llvm::StringRef> plugin_names =
PluginManager::GetSaveCorePluginNames();
if (llvm::find(plugin_names, name) == plugin_names.end()) {
StreamString stream;
stream.Printf("plugin name '%s' is not a valid ObjectFile plugin name.",
name);

if (!plugin_names.empty()) {
stream.PutCString(" Valid names are: ");
std::string plugin_names_str = llvm::join(plugin_names, ", ");
stream.PutCString(plugin_names_str);
stream.PutChar('.');
}
return Status(stream.GetString().str());
}

m_plugin_name = name;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,10 @@ def test_save_core_via_process_plugin(self):
os.unlink(core)
except OSError:
pass

def test_help(self):
"""Test that help shows minidump as an option in plugin-names."""
self.expect(
"help process save-core",
substrs=["process save-core", "<plugin>", "Values:", "minidump"],
)
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ run

process save-core --plugin-name=notaplugin dump
# CHECK-LABEL: process save-core --plugin-name=notaplugin dump
# CHECK: error: plugin name 'notaplugin' is not a valid ObjectFile plugin name
# CHECK: error: plugin name 'notaplugin' is not a valid ObjectFile plugin name. Valid names are:{{.*}}minidump{{.*}}
Loading