-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[lldb] Don't crash when attaching to pid and no binaries found #100287
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
[lldb] Don't crash when attaching to pid and no binaries found #100287
Conversation
There is a narrow window during process launch on macOS where lldb can attach and no binaries will be seen as loaded in the process (none reported by libdyld SPI). A year ago I made changes to set the new-binary-loaded breakpoint correctly despite this. But we've seen a crash when this combination is seen, where CommandObjectProcessAttach::DoExecute assumed there was at least one binary registered in the Target. Fix that. Also fix two FileSpec API uses from when we didn't have a GetPath() method that returned a std::string, and was copying the filepaths into fixed length buffers. All of this code was from ~14 years ago when we didn't have that API. rdar://131631627
@llvm/pr-subscribers-lldb Author: Jason Molenda (jasonmolenda) ChangesThere is a narrow window during process launch on macOS where lldb can attach and no binaries will be seen as loaded in the process (none reported by libdyld SPI). A year ago I made changes to set the new-binary-loaded breakpoint correctly despite this. But we've seen a crash when this combination is seen, where CommandObjectProcessAttach::DoExecute assumed there was at least one binary registered in the Target. Fix that. Also fix two FileSpec API uses from when we didn't have a GetPath() method that returned a std::string, and was copying the filepaths into fixed length buffers. All of this code was from ~14 years ago when we didn't have that API. rdar://131631627 Full diff: https://github.com/llvm/llvm-project/pull/100287.diff 1 Files Affected:
diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp
index 50695af556939..e605abdb3c771 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -369,25 +369,23 @@ class CommandObjectProcessAttach : public CommandObjectProcessLaunchOrAttach {
// Okay, we're done. Last step is to warn if the executable module has
// changed:
- char new_path[PATH_MAX];
ModuleSP new_exec_module_sp(target->GetExecutableModule());
if (!old_exec_module_sp) {
// We might not have a module if we attached to a raw pid...
if (new_exec_module_sp) {
- new_exec_module_sp->GetFileSpec().GetPath(new_path, PATH_MAX);
- result.AppendMessageWithFormat("Executable module set to \"%s\".\n",
- new_path);
+ result.AppendMessageWithFormat(
+ "Executable binary set to \"%s\".\n",
+ new_exec_module_sp->GetFileSpec().GetPath().c_str());
}
+ } else if (!new_exec_module_sp) {
+ result.AppendWarningWithFormat("No executable binary.");
} else if (old_exec_module_sp->GetFileSpec() !=
new_exec_module_sp->GetFileSpec()) {
- char old_path[PATH_MAX];
-
- old_exec_module_sp->GetFileSpec().GetPath(old_path, PATH_MAX);
- new_exec_module_sp->GetFileSpec().GetPath(new_path, PATH_MAX);
result.AppendWarningWithFormat(
- "Executable module changed from \"%s\" to \"%s\".\n", old_path,
- new_path);
+ "Executable binary changed from \"%s\" to \"%s\".\n",
+ old_exec_module_sp->GetFileSpec().GetPath().c_str(),
+ new_exec_module_sp->GetFileSpec().GetPath().c_str());
}
if (!old_arch_spec.IsValid()) {
|
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.
LGTM
Summary: There is a narrow window during process launch on macOS where lldb can attach and no binaries will be seen as loaded in the process (none reported by libdyld SPI). A year ago I made changes to set the new-binary-loaded breakpoint correctly despite this. But we've seen a crash when this combination is seen, where CommandObjectProcessAttach::DoExecute assumed there was at least one binary registered in the Target. Fix that. Also fix two FileSpec API uses from when we didn't have a GetPath() method that returned a std::string, and was copying the filepaths into fixed length buffers. All of this code was from ~14 years ago when we didn't have that API. rdar://131631627 Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60250573
Should this be backported to 19.x? |
…100287) There is a narrow window during process launch on macOS where lldb can attach and no binaries will be seen as loaded in the process (none reported by libdyld SPI). A year ago I made changes to set the new-binary-loaded breakpoint correctly despite this. But we've seen a crash when this combination is seen, where CommandObjectProcessAttach::DoExecute assumed there was at least one binary registered in the Target. Fix that. Also fix two FileSpec API uses from when we didn't have a GetPath() method that returned a std::string, and was copying the filepaths into fixed length buffers. All of this code was from ~14 years ago when we didn't have that API. rdar://131631627 (cherry picked from commit 56535a0)
…found-dont-crash [lldb] Don't crash when attaching to pid and no binaries found (llvm#100287)
There is a narrow window during process launch on macOS where lldb can attach and no binaries will be seen as loaded in the process (none reported by libdyld SPI). A year ago I made changes to set the new-binary-loaded breakpoint correctly despite this. But we've seen a crash when this combination is seen, where CommandObjectProcessAttach::DoExecute assumed there was at least one binary registered in the Target. Fix that.
Also fix two FileSpec API uses from when we didn't have a GetPath() method that returned a std::string, and was copying the filepaths into fixed length buffers. All of this code was from ~14 years ago when we didn't have that API.
rdar://131631627