jasonmolenda created this revision.
jasonmolenda added reviewers: labath, clayborg.
jasonmolenda added a project: LLDB.
I've been experimenting with returning richer error messages in debugserver
recently, and I found a bug the other day where qLaunchSuccess returns its
error code with an "E" followed by the text of the error message. If the error
message contains one of the magic gdb RSP characters (#, $, *, }), lldb will
often crash. We've seen packets like vAttach adopt a new method of returning
an error message as Exx;HEX-ENCODED instead of the usual Exx.
We've had instances where packets need to be fixed in the past, e.g. Spencer
Michaels reports a bug in the A packet here
https://bugs.llvm.org/show_bug.cgi?id=42471 and I need to fix the constant
values we use in the vFile packets. Spender and I were discussing a
"qProtocolFixes" packet, but Fred suggested we could piggyback on qSupported.
I liked that idea, so that's what I have implemented here.
lldb will send qLaunchSuccessASCIIHexErrorText in its qSupported request, and
if this is a debugserver that has the fix, it will enable the corrected error
reporting mode and include qLaunchSuccessASCIIHexErrorText in the qSupported
response. I think this is a pretty clean way of fixing things like this,
Greg/Pavel what do you think?
I also found a bug in debugserver's cstring_to_asciihex_string() function with
high-bit set characters in the error string (e.g. a utf-8 character) with
signed chars where we'd end up with a bunch of 0xff's in the ascii hex string.
I'm trying to make an API test for this but haven't found a good way to fake up
enough of the lldb side to add a gdb_remote_client test yet.
rdar://problem/62873581
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D79614
Files:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/tools/debugserver/source/RNBRemote.cpp
lldb/tools/debugserver/source/RNBRemote.h
Index: lldb/tools/debugserver/source/RNBRemote.h
===================================================================
--- lldb/tools/debugserver/source/RNBRemote.h
+++ lldb/tools/debugserver/source/RNBRemote.h
@@ -405,6 +405,7 @@
size_t m_compression_minsize; // only packets larger than this size will be
// compressed
bool m_enable_compression_next_send_packet;
+ bool m_qLaunchSuccess_error_fix;
compression_types m_compression_mode;
};
Index: lldb/tools/debugserver/source/RNBRemote.cpp
===================================================================
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -145,6 +145,18 @@
return addr;
}
+std::string cstring_to_asciihex_string(const char *str) {
+ std::string hex_str;
+ hex_str.reserve(strlen(str) * 2);
+ while (str && *str) {
+ uint8_t c = *str++;
+ char hexbuf[5];
+ snprintf(hexbuf, sizeof(hexbuf), "%02x", c);
+ hex_str += hexbuf;
+ }
+ return hex_str;
+}
+
extern void ASLLogCallback(void *baton, uint32_t flags, const char *format,
va_list args);
@@ -176,6 +188,7 @@
m_extended_mode(false), m_noack_mode(false),
m_thread_suffix_supported(false), m_list_threads_in_stop_reply(false),
m_compression_minsize(384), m_enable_compression_next_send_packet(false),
+ m_qLaunchSuccess_error_fix(false),
m_compression_mode(compression_types::none) {
DNBLogThreadedIf(LOG_RNB_REMOTE, "%s", __PRETTY_FUNCTION__);
CreatePacketTable();
@@ -1643,7 +1656,13 @@
return SendPacket("OK");
std::ostringstream ret_str;
std::string status_str;
- ret_str << "E" << m_ctx.LaunchStatusAsString(status_str);
+ if (m_qLaunchSuccess_error_fix) {
+ std::string hexified =
+ cstring_to_asciihex_string(m_ctx.LaunchStatusAsString(status_str));
+ ret_str << "E97;" << hexified.c_str();
+ } else {
+ ret_str << "E" << m_ctx.LaunchStatusAsString(status_str);
+ }
return SendPacket(ret_str.str());
}
@@ -2673,17 +2692,6 @@
}
}
-std::string cstring_to_asciihex_string(const char *str) {
- std::string hex_str;
- hex_str.reserve (strlen (str) * 2);
- while (str && *str) {
- char hexbuf[5];
- snprintf (hexbuf, sizeof(hexbuf), "%02x", *str++);
- hex_str += hexbuf;
- }
- return hex_str;
-}
-
void append_hexified_string(std::ostream &ostrm, const std::string &string) {
size_t string_size = string.size();
const char *string_buf = string.c_str();
@@ -3647,7 +3655,13 @@
snprintf(numbuf, sizeof(numbuf), "%zu", m_compression_minsize);
numbuf[sizeof(numbuf) - 1] = '\0';
strcat(buf, numbuf);
- }
+ }
+
+ // Enable a fix to the qLaunchSuccess packet.
+ if (strstr(p, "qLaunchSuccessASCIIHexErrorText") != nullptr) {
+ m_qLaunchSuccess_error_fix = true;
+ strcat(buf, ";qLaunchSuccessASCIIHexErrorText");
+ }
return SendPacket(buf);
}
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -546,6 +546,7 @@
LazyBool m_supports_qXfer_libraries_svr4_read;
LazyBool m_supports_qXfer_features_read;
LazyBool m_supports_qXfer_memory_map_read;
+ LazyBool m_supports_qLaunchSuccess_error_asciihex;
LazyBool m_supports_augmented_libraries_svr4_read;
LazyBool m_supports_jThreadExtendedInfo;
LazyBool m_supports_jLoadedDynamicLibrariesInfos;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -83,6 +83,7 @@
m_supports_qXfer_libraries_svr4_read(eLazyBoolCalculate),
m_supports_qXfer_features_read(eLazyBoolCalculate),
m_supports_qXfer_memory_map_read(eLazyBoolCalculate),
+ m_supports_qLaunchSuccess_error_asciihex(eLazyBoolCalculate),
m_supports_augmented_libraries_svr4_read(eLazyBoolCalculate),
m_supports_jThreadExtendedInfo(eLazyBoolCalculate),
m_supports_jLoadedDynamicLibrariesInfos(eLazyBoolCalculate),
@@ -297,6 +298,7 @@
m_supports_qXfer_libraries_svr4_read = eLazyBoolCalculate;
m_supports_qXfer_features_read = eLazyBoolCalculate;
m_supports_qXfer_memory_map_read = eLazyBoolCalculate;
+ m_supports_qLaunchSuccess_error_asciihex = eLazyBoolCalculate;
m_supports_augmented_libraries_svr4_read = eLazyBoolCalculate;
m_supports_qProcessInfoPID = true;
m_supports_qfProcessInfo = true;
@@ -342,11 +344,13 @@
m_supports_augmented_libraries_svr4_read = eLazyBoolNo;
m_supports_qXfer_features_read = eLazyBoolNo;
m_supports_qXfer_memory_map_read = eLazyBoolNo;
+ m_supports_qLaunchSuccess_error_asciihex = eLazyBoolNo;
m_max_packet_size = UINT64_MAX; // It's supposed to always be there, but if
// not, we assume no limit
// build the qSupported packet
- std::vector<std::string> features = {"xmlRegisters=i386,arm,mips,arc"};
+ std::vector<std::string> features = {"xmlRegisters=i386,arm,mips,arc",
+ "qLaunchSuccessASCIIHexErrorText"};
StreamString packet;
packet.PutCString("qSupported");
for (uint32_t i = 0; i < features.size(); ++i) {
@@ -446,6 +450,9 @@
LLDB_LOGF(log, "Garbled PacketSize spec in qSupported response");
}
}
+ if (::strstr(response_cstr, "qLaunchSuccessASCIIHexErrorText")) {
+ m_supports_qLaunchSuccess_error_asciihex = eLazyBoolYes;
+ }
}
}
@@ -764,8 +771,13 @@
if (response.IsOKResponse())
return true;
if (response.GetChar() == 'E') {
- // A string the describes what failed when launching...
- error_str = std::string(response.GetStringRef().substr(1));
+ if (m_supports_qLaunchSuccess_error_asciihex == eLazyBoolYes) {
+ error_str = response.GetStatus().AsCString();
+ } else {
+ // The old format of this packet would respond with a
+ // 'E' followed by the plain error string with no escaping.
+ error_str = std::string(response.GetStringRef().substr(1));
+ }
} else {
error_str.assign("unknown error occurred launching process");
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
