[Lldb-commits] [PATCH] D79614: Fix error reporting for qLaunchSuccess, check for fix/enable it via qSupported

Sat May 9 21:48:17 PDT 2020

jasonmolenda added a comment.

In D79614#2028295 <https://reviews.llvm.org/D79614#2028295>, @clayborg wrote:

> In D79614#2027869 <https://reviews.llvm.org/D79614#2027869>, @jasonmolenda wrote:
>
> > In D79614#2027861 <https://reviews.llvm.org/D79614#2027861>, @clayborg wrote:
> >
> > > Why not just fix qLaunchSuccess by passing the string through a "escape_string(...)" function? Any packet that can return a string with unknown content should be escaped correctly. Adding a new packet doesn't really fix the problem in older debugserver/lldb-server binaries anyway.
> >
> >
> > lldb wouldn't know whether to decode the returned error string or not -- short of using a debugserver version #.  We'll need to interoperate with debugservers that send the unescaped qLaunchSuccess error messages for years to come.
>
>
> It should always decode a raw string so I don't see what you mean here. Any packet that returns a normal string, not hex ascii encoded, must escape the string. The only bug here is that we are not escaping the string that debugserver sends back. No need for the hex ASCII stuff if we escape any special characters right?

Ah, I see what you're getting at --- yes I bet if we switch to using the binary escape protocol for the error message that would work just as well.  It's a one-off way to report an error message which doesn't thrill me, but you're right that this may not be worth the addition to qSupported etc.  I'll confirm that using the standard escaping is sufficient and decide whether to just do that, it's probably the right call.  Thanks for the feedback.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79614/new/

https://reviews.llvm.org/D79614