Fix a crash from character type confusion interaction with libedit

[kevinfrei]

If you type settings show <tab> LLDB might crash, depending on the version of libedit you're compiled with, and whether you're compiled with -DLLDB_EDITLINE_USE_WCHAR=0 (and depending on how the optimizer lays out the stack...)

The issue has to do with trying to figure out whether the libedit getchar callback is supposed to read a wide or 8 bit character. In order to maintain backward compatibility, there's really no 'clean' way to do it. We just have to make sure that we're invoking el_[w]getc with a buffer that is as wide as the getchar callback (registered by the SetGetCharacterFunction function further down in Editline.cpp.

So, it's 'fixed' with a comment, and a wider version of the 'reply' variable.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Kevin Frei (kevinfrei)

Changes

If you type settings show <tab> LLDB might crash, depending on the version of libedit you're compiled with, and whether you're compiled with -DLLDB_EDITLINE_USE_WCHAR=0 (and depending on how the optimizer lays out the stack...)

The issue has to do with trying to figure out whether the libedit getchar callback is supposed to read a wide or 8 bit character. In order to maintain backward compatibility, there's really no 'clean' way to do it. We just have to make sure that we're invoking el_[w]getc with a buffer that is as wide as the getchar callback (registered by the SetGetCharacterFunction function further down in Editline.cpp.

So, it's 'fixed' with a comment, and a wider version of the 'reply' variable.

---

Full diff: https://github.com/llvm/llvm-project/pull/75388.diff

2 Files Affected:

• (modified) lldb/include/lldb/Host/Editline.h (+2)
• (modified) lldb/source/Host/common/Editline.cpp (+8-2)

diff --git a/lldb/include/lldb/Host/Editline.h b/lldb/include/lldb/Host/Editline.h
index c598244150788d..9049b106f02a34 100644
--- a/lldb/include/lldb/Host/Editline.h
+++ b/lldb/include/lldb/Host/Editline.h
@@ -75,6 +75,8 @@ using EditLineCharType = char;
 // to wchar_t. It is not possible to detect differentiate between the two
 // versions exactly, but this is a pretty good approximation and allows us to
 // build against almost any editline version out there.
+// It does, however, require extra care when invoking el_getc, as the type
+// of the input is a single char buffer, but the callback will write a wchar_t.
 #if LLDB_EDITLINE_USE_WCHAR || defined(EL_CLIENTDATA) || LLDB_HAVE_EL_RFUNC_T
 using EditLineGetCharType = wchar_t;
 #else
diff --git a/lldb/source/Host/common/Editline.cpp b/lldb/source/Host/common/Editline.cpp
index 82e17ec753ab23..ce707e530d008b 100644
--- a/lldb/source/Host/common/Editline.cpp
+++ b/lldb/source/Host/common/Editline.cpp
@@ -978,8 +978,14 @@ void Editline::DisplayCompletions(
       break;
 
     fprintf(editline.m_output_file, "More (Y/n/a): ");
-    char reply = 'n';
-    int got_char = el_getc(editline.m_editline, &reply);
+    // The type for the output and the type for the parameter are different,
+    // to allow interoperability with older versions of libedit. The container
+    // for the reply must be as wide as what our implementation is using,
+    // but libedit may use a narrower type depending on the build
+    // configuration.
+    EditLineGetCharType reply = L'n';
+    int got_char = el_wgetc(editline.m_editline,
+                            reinterpret_cast<EditLineCharType *>(&reply));
     // Check for a ^C or other interruption.
     if (editline.m_editor_status == EditorStatus::Interrupted) {
       editline.m_editor_status = EditorStatus::Editing;

[bulbazord]

Why does this use el_wgetc instead of us switching based on LLDB_EDITLINE_USE_WCHAR? I'm not too familiar with editline, but it seems strange to me that we would use el_wgetc even if there's no wchar_t support.

[kevinfrei]

el_wgetc is #defined to el_getc if there's no wchar_t support (it's not at all consistent in this file, but I wanted to targeted fix for the crash). The core problem is actually with the #if code below the comment in the header:

#if LLDB_EDITLINE_USE_WCHAR || defined(EL_CLIENTDATA) || LLDB_HAVE_EL_RFUNC_T
using EditLineGetCharType = wchar_t;
#else
using EditLineGetCharType = char;
#endif

We set EditLineGetCharType to wchar_t, even if you are building with LLDB_EDITLINE_USE_WCHAR=0 with newer versions of libedit (the ones that define EL_CLIENTDATA) so the function that reads a character (called from within libedit) is always reading a wchar_t and writing it to the buffer that's backed by &reply.

[kevinfrei]

Just realized I didn't point you at the code to answer your simple question: At the top of EditLine.cpp, all the el_w* functions are #defined to el_* functions if you're build without LLDB_EDITLINE_USE_WCHAR

[clayborg]

Looks good to me