Skip to content

Commit c814ac1

Browse files
authored
[lldb] Correct use_editline check in IOHandlerEditline (#171733)
Correct the use_editline check in IOHandlerEditline to prevent a crash when we have an output and/or error file, but no stream. This fixes a regression introduced by 58279d1 that results in a crash when calling el_init with a NULL stream. The original code was checking the stream: GetOutputFILE and GetErrorFILE. ``` use_editline = GetInputFILE() && GetOutputFILE() && GetErrorFILE() && m_input_sp && m_input_sp->GetIsRealTerminal(); ``` The new code is checking the file: `m_output_sp` and `m_error_sp`. ``` use_editline = m_input_sp && m_output_sp && m_error_sp && m_input_sp->GetIsRealTerminal(); ``` The correct check is: ``` use_editline = m_input_sp && m_input_sp->GetIsRealTerminal() && m_output_sp && m_output_sp->GetUnlockedFile().GetStream() && m_error_sp && m_error_sp->GetUnlockedFile().GetStream(); ``` We don't need to update the check for the input, because we're handling the missing stream there correctly in the call to the constructor: ``` m_editline_up = std::make_unique<Editline>( editline_name, m_input_sp ? m_input_sp->GetStream() : nullptr, m_output_sp, m_error_sp, m_color); ``` We can't do the same for the output and error because we need to pass the file, not the stream (to do proper locking). As I was debugging this I added some more assertions. They're generally useful so I'm keeping them. Fixes #170891
1 parent fc78d55 commit c814ac1

File tree

3 files changed

+13
-4
lines changed

3 files changed

+13
-4
lines changed

lldb/include/lldb/Host/StreamFile.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,10 @@ class LockableStreamFile {
9292
/// Unsafe accessors to get the underlying File without a lock. Exists for
9393
/// legacy reasons.
9494
/// @{
95-
File &GetUnlockedFile() { return *m_file_sp; }
95+
File &GetUnlockedFile() {
96+
assert(m_file_sp && "GetUnlockedFile requires a valid FileSP");
97+
return *m_file_sp;
98+
}
9699
std::shared_ptr<File> GetUnlockedFileSP() { return m_file_sp; }
97100
/// @}
98101

lldb/source/Core/IOHandler.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -245,8 +245,13 @@ IOHandlerEditline::IOHandlerEditline(
245245
SetPrompt(prompt);
246246

247247
#if LLDB_ENABLE_LIBEDIT
248-
const bool use_editline = m_input_sp && m_output_sp && m_error_sp &&
249-
m_input_sp->GetIsRealTerminal();
248+
// To use Editline, we need an input, output, and error stream. Not all valid
249+
// files will have a FILE* stream. Don't use Editline if the input is not a
250+
// real terminal.
251+
const bool use_editline =
252+
m_input_sp && m_input_sp->GetIsRealTerminal() && // Input
253+
m_output_sp && m_output_sp->GetUnlockedFile().GetStream() && // Output
254+
m_error_sp && m_error_sp->GetUnlockedFile().GetStream(); // Error
250255
if (use_editline) {
251256
m_editline_up = std::make_unique<Editline>(
252257
editline_name, m_input_sp ? m_input_sp->GetStream() : nullptr,

lldb/source/Host/common/Editline.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1511,7 +1511,8 @@ Editline::Editline(const char *editline_name, FILE *input_file,
15111511
: m_editor_status(EditorStatus::Complete), m_input_file(input_file),
15121512
m_output_stream_sp(output_stream_sp), m_error_stream_sp(error_stream_sp),
15131513
m_input_connection(fileno(input_file), false), m_color(color) {
1514-
assert(output_stream_sp && error_stream_sp);
1514+
assert(output_stream_sp && output_stream_sp->GetUnlockedFile().GetStream());
1515+
assert(error_stream_sp && output_stream_sp->GetUnlockedFile().GetStream());
15151516
// Get a shared history instance
15161517
m_editor_name = (editline_name == nullptr) ? "lldb-tmp" : editline_name;
15171518
m_history_sp = EditlineHistory::GetHistory(m_editor_name);

0 commit comments

Comments
 (0)