[lldb] Refactor host::OpenFileInExternalEditor

This patch refactors the macOS implementation of
OpenFileInExternalEditor. It fixes an AppleEvent memory leak, the
caching of LLDB_EXTERNAL_EDITOR and speculatively fixes a crash when
CFURL is NULL (rdar://108633464). The new code also improves error
handling, readability and documents calls to the CoreFoundation Launch
Services APIs.

A bunch of the Launch Services APIs have been deprecated
(LSFindApplicationForInfo, LSOpenURLsWithRole). The preferred API is
LSOpenCFURLRef but it doesn't specifying the "location" Apple Event
which is used to highlight the current line and switching over would
regress the existing behavior.

rdar://108633464

Differential revision: https://reviews.llvm.org/D149482
This commit is contained in:
Jonas Devlieghere 2023-04-28 13:36:00 -07:00
parent bfb5a4fd53
commit 13dbc16b4d
No known key found for this signature in database
GPG Key ID: 49CC0BD90FDEED4D
5 changed files with 108 additions and 74 deletions

View File

@ -236,8 +236,8 @@ public:
bool run_in_shell = true,
bool hide_stderr = false);
static bool OpenFileInExternalEditor(const FileSpec &file_spec,
uint32_t line_no);
static llvm::Error OpenFileInExternalEditor(const FileSpec &file_spec,
uint32_t line_no);
/// Check if we're running in an interactive graphical session.
///

View File

@ -546,9 +546,10 @@ void Host::Kill(lldb::pid_t pid, int signo) { ::kill(pid, signo); }
#endif
#if !defined(__APPLE__)
bool Host::OpenFileInExternalEditor(const FileSpec &file_spec,
uint32_t line_no) {
return false;
llvm::Error Host::OpenFileInExternalEditor(const FileSpec &file_spec,
uint32_t line_no) {
return llvm::errorCodeToError(
std::error_code(ENOTSUP, std::system_category()));
}
bool Host::IsInteractiveGraphicSession() { return false; }

View File

@ -325,12 +325,36 @@ LaunchInNewTerminalWithAppleScript(const char *exe_path,
#endif // TARGET_OS_OSX
bool Host::OpenFileInExternalEditor(const FileSpec &file_spec,
uint32_t line_no) {
llvm::Error Host::OpenFileInExternalEditor(const FileSpec &file_spec,
uint32_t line_no) {
#if !TARGET_OS_OSX
return false;
return llvm::errorCodeToError(
std::error_code(ENOTSUP, std::system_category()));
#else // !TARGET_OS_OSX
// We attach this to an 'odoc' event to specify a particular selection
Log *log = GetLog(LLDBLog::Host);
const std::string file_path = file_spec.GetPath();
LLDB_LOG(log, "Sending {0}:{1} to external editor",
file_path.empty() ? "<invalid>" : file_path, line_no);
if (file_path.empty())
return llvm::createStringError(llvm::inconvertibleErrorCode(),
"no file specified");
CFCString file_cfstr(file_path.c_str(), kCFStringEncodingUTF8);
CFCReleaser<CFURLRef> file_URL = ::CFURLCreateWithFileSystemPath(
/*allocator=*/NULL,
/*filePath*/ file_cfstr.get(),
/*pathStyle=*/kCFURLPOSIXPathStyle,
/*isDirectory=*/false);
if (!file_URL.get())
return llvm::createStringError(
llvm::inconvertibleErrorCode(),
llvm::formatv("could not create CFURL from path \"{0}\"", file_path));
// Create a new Apple Event descriptor.
typedef struct {
int16_t reserved0; // must be zero
int16_t fLineNumber;
@ -340,18 +364,7 @@ bool Host::OpenFileInExternalEditor(const FileSpec &file_spec,
uint32_t reserved2; // must be zero
} BabelAESelInfo;
Log *log = GetLog(LLDBLog::Host);
char file_path[PATH_MAX];
file_spec.GetPath(file_path, PATH_MAX);
CFCString file_cfstr(file_path, kCFStringEncodingUTF8);
CFCReleaser<CFURLRef> file_URL(::CFURLCreateWithFileSystemPath(
NULL, file_cfstr.get(), kCFURLPOSIXPathStyle, false));
LLDB_LOGF(log,
"Sending source file: \"%s\" and line: %d to external editor.\n",
file_path, line_no);
long error;
// We attach this to an 'odoc' event to specify a particular selection.
BabelAESelInfo file_and_line_info = {
0, // reserved0
(int16_t)(line_no - 1), // fLineNumber (zero based line number)
@ -362,64 +375,74 @@ bool Host::OpenFileInExternalEditor(const FileSpec &file_spec,
};
AEKeyDesc file_and_line_desc;
error = ::AECreateDesc(typeUTF8Text, &file_and_line_info,
sizeof(file_and_line_info),
&(file_and_line_desc.descContent));
if (error != noErr) {
LLDB_LOGF(log, "Error creating AEDesc: %ld.\n", error);
return false;
}
file_and_line_desc.descKey = keyAEPosition;
long error = ::AECreateDesc(/*typeCode=*/typeUTF8Text,
/*dataPtr=*/&file_and_line_info,
/*dataSize=*/sizeof(file_and_line_info),
/*result=*/&(file_and_line_desc.descContent));
static std::string g_app_name;
static FSRef g_app_fsref;
if (error != noErr)
return llvm::createStringError(
llvm::inconvertibleErrorCode(),
llvm::formatv("creating Apple Event descriptor failed: error {0}",
error));
// Deallocate the descriptor on exit.
auto on_exit = llvm::make_scope_exit(
[&]() { AEDisposeDesc(&(file_and_line_desc.descContent)); });
static std::optional<FSRef> g_app_fsref;
static std::string g_app_error;
static std::once_flag g_once_flag;
std::call_once(g_once_flag, [&]() {
if (const char *external_editor = ::getenv("LLDB_EXTERNAL_EDITOR")) {
LLDB_LOG(log, "Looking for external editor: {0}", external_editor);
FSRef app_fsref;
CFCString editor_name(external_editor, kCFStringEncodingUTF8);
long app_error = ::LSFindApplicationForInfo(
/*inCreator=*/kLSUnknownCreator, /*inBundleID=*/NULL,
/*inName=*/editor_name.get(), /*outAppRef=*/&app_fsref,
/*outAppURL=*/NULL);
if (app_error == noErr) {
g_app_fsref = app_fsref;
} else {
g_app_error =
llvm::formatv("could not find external editor \"{0}\": "
"LSFindApplicationForInfo returned error {1}",
external_editor, app_error)
.str();
}
}
});
if (!g_app_error.empty())
return llvm::createStringError(llvm::inconvertibleErrorCode(), g_app_error);
// Build app launch parameters.
LSApplicationParameters app_params;
::memset(&app_params, 0, sizeof(app_params));
app_params.flags =
kLSLaunchDefaults | kLSLaunchDontAddToRecents | kLSLaunchDontSwitch;
char *external_editor = ::getenv("LLDB_EXTERNAL_EDITOR");
if (external_editor) {
LLDB_LOGF(log, "Looking for external editor \"%s\".\n", external_editor);
if (g_app_name.empty() ||
strcmp(g_app_name.c_str(), external_editor) != 0) {
CFCString editor_name(external_editor, kCFStringEncodingUTF8);
error = ::LSFindApplicationForInfo(kLSUnknownCreator, NULL,
editor_name.get(), &g_app_fsref, NULL);
// If we found the app, then store away the name so we don't have to
// re-look it up.
if (error != noErr) {
LLDB_LOGF(log,
"Could not find External Editor application, error: %ld.\n",
error);
return false;
}
}
app_params.application = &g_app_fsref;
}
if (g_app_fsref)
app_params.application = &(g_app_fsref.value());
ProcessSerialNumber psn;
CFCReleaser<CFArrayRef> file_array(
CFArrayCreate(NULL, (const void **)file_URL.ptr_address(false), 1, NULL));
error = ::LSOpenURLsWithRole(file_array.get(), kLSRolesAll,
&file_and_line_desc, &app_params, &psn, 1);
std::array<CFURLRef, 1> file_array = {file_URL.get()};
CFCReleaser<CFArrayRef> cf_array(
CFArrayCreate(/*allocator=*/NULL, /*values=*/(const void **)&file_array,
/*numValues*/ 1, /*callBacks=*/NULL));
error = ::LSOpenURLsWithRole(
/*inURLs=*/cf_array.get(), /*inRole=*/kLSRolesEditor,
/*inAEParam=*/&file_and_line_desc,
/*inAppParams=*/&app_params, /*outPSNs=*/&psn, /*inMaxPSNCount=*/1);
AEDisposeDesc(&(file_and_line_desc.descContent));
if (error != noErr)
return llvm::createStringError(
llvm::inconvertibleErrorCode(),
llvm::formatv("LSOpenURLsWithRole failed: error {0}", error));
if (error != noErr) {
LLDB_LOGF(log, "LSOpenURLsWithRole failed, error: %ld.\n", error);
return false;
}
return true;
return llvm::Error::success();
#endif // TARGET_OS_OSX
}

View File

@ -3271,8 +3271,10 @@ bool CommandInterpreter::SaveTranscript(
if (GetOpenTranscriptInEditor() && Host::IsInteractiveGraphicSession()) {
const FileSpec file_spec;
error = file->GetFileSpec(const_cast<FileSpec &>(file_spec));
if (error.Success())
Host::OpenFileInExternalEditor(file_spec, 1);
if (error.Success()) {
if (llvm::Error e = Host::OpenFileInExternalEditor(file_spec, 1))
result.AppendError(llvm::toString(std::move(e)));
}
}
return true;

View File

@ -305,8 +305,13 @@ bool Thread::SetSelectedFrameByIndexNoisily(uint32_t frame_idx,
frame_sp->GetSymbolContext(eSymbolContextLineEntry));
if (GetProcess()->GetTarget().GetDebugger().GetUseExternalEditor() &&
frame_sc.line_entry.file && frame_sc.line_entry.line != 0) {
already_shown = Host::OpenFileInExternalEditor(
frame_sc.line_entry.file, frame_sc.line_entry.line);
if (llvm::Error e = Host::OpenFileInExternalEditor(
frame_sc.line_entry.file, frame_sc.line_entry.line)) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Host), std::move(e),
"OpenFileInExternalEditor failed: {0}");
} else {
already_shown = true;
}
}
bool show_frame_info = true;
@ -1725,8 +1730,11 @@ size_t Thread::GetStatus(Stream &strm, uint32_t start_frame,
SymbolContext frame_sc(
frame_sp->GetSymbolContext(eSymbolContextLineEntry));
if (frame_sc.line_entry.line != 0 && frame_sc.line_entry.file) {
Host::OpenFileInExternalEditor(frame_sc.line_entry.file,
frame_sc.line_entry.line);
if (llvm::Error e = Host::OpenFileInExternalEditor(
frame_sc.line_entry.file, frame_sc.line_entry.line)) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Host), std::move(e),
"OpenFileInExternalEditor failed: {0}");
}
}
}
}