[RFC] Filter invalid encodings from Linux thread names
Commit Message
On Linux, a thread can only be 16 bytes (including the trailing \0).
A user sent in a test case where this causes a truncated UTF-8
sequence, causing gdbserver to create invalid XML.
I went back and forth about different ways to solve this, and in the
end decided to fix it in gdbserver, with the reason being that it
seems important to generate correct XML for the <thread> response.
I am not totally sure whether the call to setlocale could have
unplanned consequences. This is needed, though, for nl_langinfo to
return the correct result.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30618
---
gdbserver/linux-low.cc | 59 ++++++++++++++++++++++++++++++++++++++++--
gdbserver/server.cc | 1 +
2 files changed, 58 insertions(+), 2 deletions(-)
Comments
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
Tom> On Linux, a thread can only be 16 bytes (including the trailing \0).
Tom> A user sent in a test case where this causes a truncated UTF-8
Tom> sequence, causing gdbserver to create invalid XML.
Tom> I went back and forth about different ways to solve this, and in the
Tom> end decided to fix it in gdbserver, with the reason being that it
Tom> seems important to generate correct XML for the <thread> response.
Tom> I am not totally sure whether the call to setlocale could have
Tom> unplanned consequences. This is needed, though, for nl_langinfo to
Tom> return the correct result.
Tom> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30618
I'd appreciate comments on this one.
Eventually I guess I'll checked it in, though.
Tom
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
Tom> On Linux, a thread can only be 16 bytes (including the trailing \0).
Tom> A user sent in a test case where this causes a truncated UTF-8
Tom> sequence, causing gdbserver to create invalid XML.
Tom> I went back and forth about different ways to solve this, and in the
Tom> end decided to fix it in gdbserver, with the reason being that it
Tom> seems important to generate correct XML for the <thread> response.
Tom> I am not totally sure whether the call to setlocale could have
Tom> unplanned consequences. This is needed, though, for nl_langinfo to
Tom> return the correct result.
Tom> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30618
I'm going to check this in now.
Tom
On 11/14/23 11:01, Tom Tromey wrote:
>>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
>
> Tom> On Linux, a thread can only be 16 bytes (including the trailing \0).
> Tom> A user sent in a test case where this causes a truncated UTF-8
> Tom> sequence, causing gdbserver to create invalid XML.
>
> Tom> I went back and forth about different ways to solve this, and in the
> Tom> end decided to fix it in gdbserver, with the reason being that it
> Tom> seems important to generate correct XML for the <thread> response.
>
> Tom> I am not totally sure whether the call to setlocale could have
> Tom> unplanned consequences. This is needed, though, for nl_langinfo to
> Tom> return the correct result.
>
> Tom> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30618
>
> I'm going to check this in now.
>
> Tom
Hi Tom,
I see:
$ make check TESTS="gdb.threads/names.exp" RUNTESTFLAGS="--target_board=native-extended-gdbserver"
FAIL: gdb.threads/names.exp: list threads
I also see:
FAIL: gdb.ada/tasks.exp: info threads
I didn't bisect that one, but I guess it's related.
Simon
@@ -38,14 +38,16 @@
#include <unistd.h>
#include <sys/syscall.h>
#include <sched.h>
-#include <ctype.h>
#include <pwd.h>
#include <sys/types.h>
#include <dirent.h>
#include <sys/stat.h>
#include <sys/vfs.h>
#include <sys/uio.h>
+#include <langinfo.h>
+#include <iconv.h>
#include "gdbsupport/filestuff.h"
+#include "gdbsupport/gdb-safe-ctype.h"
#include "tracepoint.h"
#include <inttypes.h>
#include "gdbsupport/common-inferior.h"
@@ -6898,10 +6900,63 @@ current_lwp_ptid (void)
return ptid_of (current_thread);
}
+/* A helper function that copies NAME to DEST, replacing non-printable
+ characters with '?'. Returns DEST as a convenience. */
+
+static const char *
+replace_non_ascii (char *dest, const char *name)
+{
+ while (*name != '\0')
+ {
+ if (!ISPRINT (*name))
+ *dest++ = '?';
+ else
+ *dest++ = *name;
+ ++name;
+ }
+ return dest;
+}
+
const char *
linux_process_target::thread_name (ptid_t thread)
{
- return linux_proc_tid_get_name (thread);
+ static char dest[100];
+
+ const char *name = linux_proc_tid_get_name (thread);
+ if (name == nullptr)
+ return nullptr;
+
+ /* Linux limits the comm file to 16 bytes (including the trailing
+ \0. If the program or thread name is set when using a multi-byte
+ encoding, this might cause it to be truncated mid-character. In
+ this situation, sending the truncated form in an XML <thread>
+ response will cause a parse error in gdb. So, instead convert
+ from the locale's encoding (we can't be sure this is the correct
+ encoding, but it's as good a guess as we have) to UTF-8, but in a
+ way that ignores any encoding errors. See PR remote/30618. */
+ const char *cset = nl_langinfo (CODESET);
+ iconv_t handle = iconv_open ("UTF-8//IGNORE", cset);
+ if (handle == (iconv_t) -1)
+ return replace_non_ascii (dest, name);
+
+ size_t inbytes = strlen (name);
+ char *inbuf = const_cast<char *> (name);
+ size_t outbytes = sizeof (dest);
+ char *outbuf = dest;
+ size_t result = iconv (handle, &inbuf, &inbytes, &outbuf, &outbytes);
+
+ if (result == (size_t) -1)
+ {
+ if (errno == E2BIG)
+ outbuf = &dest[sizeof (dest) - 1];
+ else if ((errno == EILSEQ || errno == EINVAL)
+ && outbuf < &dest[sizeof (dest) - 2])
+ *outbuf++ '?';
+ *outbuf = '\0';
+ }
+
+ iconv_close (handle);
+ return *dest == '\0' ? nullptr : dest;
}
#if USE_THREAD_DB
@@ -4056,6 +4056,7 @@ captured_main (int argc, char *argv[])
int
main (int argc, char *argv[])
{
+ setlocale (LC_CTYPE, "");
try
{