Re: [lldb-dev] Support for Error Strings in remote protocol

2017-06-22 Thread Chris Quenelle via lldb-dev
I’m just a new lurker here, so maybe this is obvious…

Is the string part of the programmatic interface?  Or just a comment?
Does the same numeric code always have the same string?
If the same numeric code can have different strings, then the string
represents a specialization of the error code?  If clients depend on
the data that’s in the string, then they may not work correctly in
modes of operation where the string is not available from the server.

If it’s intended to be part of the API then maybe a structured name/value
approach might be better?

Or maybe it’s just supposed to be a form self-documentation so that inspection
of the raw error codes is easier to diagnose?  In that case maybe the string
is always 1-to-1 with the error code?

> On Jun 21, 2017, at 8:31 AM, Ravitheja Addepally via lldb-dev 
>  wrote:
> 
> Hello all,
>Currently the remote protocol in LLDB does not allow sending Error 
> Strings in response to remote packets, it only allows for "ENN" format where 
> N is a hex integer. In our current ongoing work, we would like to have 
> support for Sending Error Strings from lldb-server. I would like to invite 
> any opinions or suggestions in this matter ?
> 
> A very simple proposal would be to just attach an error string maybe as a 
> Name:Value Pair ? like so ->
> 
> EXX;"Error String"
>  or
> EXX;M"Error String"
> 
> I guess removing EXX would make it incompatible with gdb-server. Also adding 
> new packets to query errors might not be desired ?
> 
> 
> Regards,
> A Ravi Theja
> 
> 
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] [llvm-dev] RFC: Cleaning up the Itanium demangler

2017-06-22 Thread Jim Ingham via lldb-dev
Another important criterium for the demangler in the debugger is that it 100% 
cannot crash no matter what it gets fed.  lldb used to have it's own copy of 
the system demangler library because it had bugs, and we needed to be able to 
fix them faster than the system version.  We feed it all the symbols we ingest 
(we actually sniff them a little bit, but we really shouldn't have to do that, 
the demangler should be fast enough rejecting symbols) so if there's one in 
some system library that triggers a demangler crash, you're pretty much dead in 
the water on that system...

Jim


> On Jun 22, 2017, at 11:11 AM, Scott Smith  wrote:
> 
> When I looked at demangler performance, I was able to make significant 
> improvements to the llvm demangler.  At that point removing lldb's fast 
> demangler didn't hurt performance very much, but the fast demangler was still 
> faster.  I forget (and apparently didn't write down) how much it mattered, 
> but post this change I think was single digit %.
> 
> https://reviews.llvm.org/D32500
> 
> 
> On Thu, Jun 22, 2017 at 11:07 AM, Jim Ingham via lldb-dev 
>  wrote:
> This is Greg's area, he'll be able to answer in detail how the name chopper 
> gets used.  IIRC it chops demangled names, so it is indirectly a client of 
> the demangler, but it doesn't use the demangler to do this directly.  Name 
> lookup is done by finding all the base name matches, then comparing the 
> context.  We don't do a very good job of doing fuzzy full name matches - for 
> instance when trying to break on one overload you have to get the arguments 
> exactly as the demangler would produce them.  We could do some more 
> heuristics here (remove all the spaces you can before comparison, etc.) 
> though it would be even easier if we had something that could tokenize names 
> - both mangled & natural.
> 
> The Swift demangler produces a node tree for the demangled elements of a name 
> which is very handy on the Swift side.  A long time ago Greg experimented 
> with such a thing for the C++ demangler, but it ended up being too slow.
> 
> On that note, the demangler is a performance bottleneck for lldb.  Going to 
> the fast demangler over the system one was a big performance win.  Maybe the 
> system demangler is fast enough nowadays, but if it isn't then we can't get 
> rid of the FastDemangler.
> 
> Jim
> 
> > On Jun 22, 2017, at 8:08 AM, Pavel Labath via lldb-dev 
> >  wrote:
> >
> > On 22 June 2017 at 15:21, Erik Pilkington  wrote:
> >>
> >>
> >>
> >> On June 22, 2017 at 5:51:39 AM, Pavel Labath (lab...@google.com) wrote:
> >>
> >> I don't have any concrete feedback, but:
> >>
> >> - +1 for removing the "FastDemagler"
> >>
> >> - If you already construct an AST as a part of your demangling
> >> process, would it be possible to export that AST for external
> >> consumption somehow? Right now in lldb we sometimes need to parse the
> >> demangled name (to get the "basename" of a function for example), and
> >> the code for doing that is quite ugly. It would be much nicer if we
> >> could just query the parsed representation of the name somehow, and
> >> the AST would enable us to do that.
> >>
> >>
> >> I was thinking about this use case a little, actually. I think it makes 
> >> more
> >> sense to provide a function, say getItaniumDemangledBasename(), which could
> >> just parse and query the AST for the base name (the AST already has an way
> >> of doing this). This would allow the demangler to bail out if it knows that
> >> the rest of the input string isn’t relevant, i.e., we could bail out after
> >> parsing the ‘foo’ in _Z3fooiii. That, and not having to print out the
> >> AST should make parsing the base name significantly faster on top of this.
> >>
> >> Do you have any other use case for the AST outside of base names? It still
> >> would be possible to export it from ItaniumDemangle.
> >>
> >
> > Well.. the current parser chops the name into "basename", "context",
> > "arguments", and "qualifiers" part. All of them seem to be used right
> > now, but I don't know e.g. how unavoidable that is. I know about this
> > because I was fixing some bugs there, but I am actually not that
> > familiar with this part of LLDB. I am cc-ing lldb-dev if they have any
> > thoughts on this. We also have the ability to set breakpoints by
> > providing just a part of the context (e.g. "breakpoint set -n
> > foo::bar" even though the full function name is baz::booze::foo::bar),
> > but this seems to be implemented in some different way.
> >
> > I don't think having the ability to short-circuit the demangling would
> > bring as any speed benefit, at least not without a major refactor, as
> > we demangle all the names anyway. Even the AST solution will probably
> > require a fair deal of plumbing on our part to make it useful.
> >
> > Also, any custom-tailored solution will probably make it hard to
> > retrieve any additional info, should we later need it, so I'd be in
> > favor of the AST solution. (I don't 

Re: [lldb-dev] [llvm-dev] RFC: Cleaning up the Itanium demangler

2017-06-22 Thread Scott Smith via lldb-dev
When I looked at demangler performance, I was able to make significant
improvements to the llvm demangler.  At that point removing lldb's fast
demangler didn't hurt performance very much, but the fast demangler was
still faster.  I forget (and apparently didn't write down) how much it
mattered, but post this change I think was single digit %.

https://reviews.llvm.org/D32500


On Thu, Jun 22, 2017 at 11:07 AM, Jim Ingham via lldb-dev <
lldb-dev@lists.llvm.org> wrote:

> This is Greg's area, he'll be able to answer in detail how the name
> chopper gets used.  IIRC it chops demangled names, so it is indirectly a
> client of the demangler, but it doesn't use the demangler to do this
> directly.  Name lookup is done by finding all the base name matches, then
> comparing the context.  We don't do a very good job of doing fuzzy full
> name matches - for instance when trying to break on one overload you have
> to get the arguments exactly as the demangler would produce them.  We could
> do some more heuristics here (remove all the spaces you can before
> comparison, etc.) though it would be even easier if we had something that
> could tokenize names - both mangled & natural.
>
> The Swift demangler produces a node tree for the demangled elements of a
> name which is very handy on the Swift side.  A long time ago Greg
> experimented with such a thing for the C++ demangler, but it ended up being
> too slow.
>
> On that note, the demangler is a performance bottleneck for lldb.  Going
> to the fast demangler over the system one was a big performance win.  Maybe
> the system demangler is fast enough nowadays, but if it isn't then we can't
> get rid of the FastDemangler.
>
> Jim
>
> > On Jun 22, 2017, at 8:08 AM, Pavel Labath via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> >
> > On 22 June 2017 at 15:21, Erik Pilkington 
> wrote:
> >>
> >>
> >>
> >> On June 22, 2017 at 5:51:39 AM, Pavel Labath (lab...@google.com) wrote:
> >>
> >> I don't have any concrete feedback, but:
> >>
> >> - +1 for removing the "FastDemagler"
> >>
> >> - If you already construct an AST as a part of your demangling
> >> process, would it be possible to export that AST for external
> >> consumption somehow? Right now in lldb we sometimes need to parse the
> >> demangled name (to get the "basename" of a function for example), and
> >> the code for doing that is quite ugly. It would be much nicer if we
> >> could just query the parsed representation of the name somehow, and
> >> the AST would enable us to do that.
> >>
> >>
> >> I was thinking about this use case a little, actually. I think it makes
> more
> >> sense to provide a function, say getItaniumDemangledBasename(), which
> could
> >> just parse and query the AST for the base name (the AST already has an
> way
> >> of doing this). This would allow the demangler to bail out if it knows
> that
> >> the rest of the input string isn’t relevant, i.e., we could bail out
> after
> >> parsing the ‘foo’ in _Z3fooiii. That, and not having to print out
> the
> >> AST should make parsing the base name significantly faster on top of
> this.
> >>
> >> Do you have any other use case for the AST outside of base names? It
> still
> >> would be possible to export it from ItaniumDemangle.
> >>
> >
> > Well.. the current parser chops the name into "basename", "context",
> > "arguments", and "qualifiers" part. All of them seem to be used right
> > now, but I don't know e.g. how unavoidable that is. I know about this
> > because I was fixing some bugs there, but I am actually not that
> > familiar with this part of LLDB. I am cc-ing lldb-dev if they have any
> > thoughts on this. We also have the ability to set breakpoints by
> > providing just a part of the context (e.g. "breakpoint set -n
> > foo::bar" even though the full function name is baz::booze::foo::bar),
> > but this seems to be implemented in some different way.
> >
> > I don't think having the ability to short-circuit the demangling would
> > bring as any speed benefit, at least not without a major refactor, as
> > we demangle all the names anyway. Even the AST solution will probably
> > require a fair deal of plumbing on our part to make it useful.
> >
> > Also, any custom-tailored solution will probably make it hard to
> > retrieve any additional info, should we later need it, so I'd be in
> > favor of the AST solution. (I don't know how much it would complicate
> > the implementation though).
> > ___
> > lldb-dev mailing list
> > lldb-dev@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] [llvm-dev] RFC: Cleaning up the Itanium demangler

2017-06-22 Thread Jim Ingham via lldb-dev
This is Greg's area, he'll be able to answer in detail how the name chopper 
gets used.  IIRC it chops demangled names, so it is indirectly a client of the 
demangler, but it doesn't use the demangler to do this directly.  Name lookup 
is done by finding all the base name matches, then comparing the context.  We 
don't do a very good job of doing fuzzy full name matches - for instance when 
trying to break on one overload you have to get the arguments exactly as the 
demangler would produce them.  We could do some more heuristics here (remove 
all the spaces you can before comparison, etc.) though it would be even easier 
if we had something that could tokenize names - both mangled & natural.

The Swift demangler produces a node tree for the demangled elements of a name 
which is very handy on the Swift side.  A long time ago Greg experimented with 
such a thing for the C++ demangler, but it ended up being too slow.

On that note, the demangler is a performance bottleneck for lldb.  Going to the 
fast demangler over the system one was a big performance win.  Maybe the system 
demangler is fast enough nowadays, but if it isn't then we can't get rid of the 
FastDemangler.

Jim

> On Jun 22, 2017, at 8:08 AM, Pavel Labath via lldb-dev 
>  wrote:
> 
> On 22 June 2017 at 15:21, Erik Pilkington  wrote:
>> 
>> 
>> 
>> On June 22, 2017 at 5:51:39 AM, Pavel Labath (lab...@google.com) wrote:
>> 
>> I don't have any concrete feedback, but:
>> 
>> - +1 for removing the "FastDemagler"
>> 
>> - If you already construct an AST as a part of your demangling
>> process, would it be possible to export that AST for external
>> consumption somehow? Right now in lldb we sometimes need to parse the
>> demangled name (to get the "basename" of a function for example), and
>> the code for doing that is quite ugly. It would be much nicer if we
>> could just query the parsed representation of the name somehow, and
>> the AST would enable us to do that.
>> 
>> 
>> I was thinking about this use case a little, actually. I think it makes more
>> sense to provide a function, say getItaniumDemangledBasename(), which could
>> just parse and query the AST for the base name (the AST already has an way
>> of doing this). This would allow the demangler to bail out if it knows that
>> the rest of the input string isn’t relevant, i.e., we could bail out after
>> parsing the ‘foo’ in _Z3fooiii. That, and not having to print out the
>> AST should make parsing the base name significantly faster on top of this.
>> 
>> Do you have any other use case for the AST outside of base names? It still
>> would be possible to export it from ItaniumDemangle.
>> 
> 
> Well.. the current parser chops the name into "basename", "context",
> "arguments", and "qualifiers" part. All of them seem to be used right
> now, but I don't know e.g. how unavoidable that is. I know about this
> because I was fixing some bugs there, but I am actually not that
> familiar with this part of LLDB. I am cc-ing lldb-dev if they have any
> thoughts on this. We also have the ability to set breakpoints by
> providing just a part of the context (e.g. "breakpoint set -n
> foo::bar" even though the full function name is baz::booze::foo::bar),
> but this seems to be implemented in some different way.
> 
> I don't think having the ability to short-circuit the demangling would
> bring as any speed benefit, at least not without a major refactor, as
> we demangle all the names anyway. Even the AST solution will probably
> require a fair deal of plumbing on our part to make it useful.
> 
> Also, any custom-tailored solution will probably make it hard to
> retrieve any additional info, should we later need it, so I'd be in
> favor of the AST solution. (I don't know how much it would complicate
> the implementation though).
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] [llvm-dev] RFC: Cleaning up the Itanium demangler

2017-06-22 Thread Pavel Labath via lldb-dev
On 22 June 2017 at 15:21, Erik Pilkington  wrote:
>
>
>
> On June 22, 2017 at 5:51:39 AM, Pavel Labath (lab...@google.com) wrote:
>
> I don't have any concrete feedback, but:
>
> - +1 for removing the "FastDemagler"
>
> - If you already construct an AST as a part of your demangling
> process, would it be possible to export that AST for external
> consumption somehow? Right now in lldb we sometimes need to parse the
> demangled name (to get the "basename" of a function for example), and
> the code for doing that is quite ugly. It would be much nicer if we
> could just query the parsed representation of the name somehow, and
> the AST would enable us to do that.
>
>
> I was thinking about this use case a little, actually. I think it makes more
> sense to provide a function, say getItaniumDemangledBasename(), which could
> just parse and query the AST for the base name (the AST already has an way
> of doing this). This would allow the demangler to bail out if it knows that
> the rest of the input string isn’t relevant, i.e., we could bail out after
> parsing the ‘foo’ in _Z3fooiii. That, and not having to print out the
> AST should make parsing the base name significantly faster on top of this.
>
> Do you have any other use case for the AST outside of base names? It still
> would be possible to export it from ItaniumDemangle.
>

Well.. the current parser chops the name into "basename", "context",
"arguments", and "qualifiers" part. All of them seem to be used right
now, but I don't know e.g. how unavoidable that is. I know about this
because I was fixing some bugs there, but I am actually not that
familiar with this part of LLDB. I am cc-ing lldb-dev if they have any
thoughts on this. We also have the ability to set breakpoints by
providing just a part of the context (e.g. "breakpoint set -n
foo::bar" even though the full function name is baz::booze::foo::bar),
but this seems to be implemented in some different way.

I don't think having the ability to short-circuit the demangling would
bring as any speed benefit, at least not without a major refactor, as
we demangle all the names anyway. Even the AST solution will probably
require a fair deal of plumbing on our part to make it useful.

Also, any custom-tailored solution will probably make it hard to
retrieve any additional info, should we later need it, so I'd be in
favor of the AST solution. (I don't know how much it would complicate
the implementation though).
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Support for Error Strings in remote protocol

2017-06-22 Thread Pavel Labath via lldb-dev
On 22 June 2017 at 00:18, Stephane Sezer via lldb-dev
 wrote:
> What's the specific use case that you're trying to support with error
> messages in the protocol? My initial thought on this is that it's not really
> the debug server's job to generate human-readable error messages and that
> the debugger is better suited to do the job.
I think this really depends on the use case. Take the A (launch)
packet for example. Launching a process can fail for any number of
reasons. Let's just list the main syscalls involved in doing that:
- open (for redirecting stdio)
- chdir
- fork
- execve
And each of these can fail for several reasons. So if you want to pass
full information, you'd need to pass some sort of operation+errno
combination, and the 8-bit address space is quite crowded for that,
particularly if you want it to be backward compatible. As an example,
take a look at this command:
(lldb) process launch -e /non/existing/file
error: process launch failed: 'A' packet returned an error: 8

Wouldn't it be better if it read:
error: process launch failed: failed to open file
'/non/existing/file': No such file or directory

I think it would be hard to get this level of detail for the client
with just numeric error codes, whereas producing this string is
trivial for the server.


Note that it is still the client who decides what error to print to
the user, and if it thinks it has a better error message, it is free
to ignore the one returned by the server.
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Support for Error Strings in remote protocol

2017-06-22 Thread Ravitheja Addepally via lldb-dev
That's the other option of decoding error codes at the client, there is the
obvious downside of the common error table to become very big ? considering
the number of OS's and Targets ?
Also the lldb-server already knows the target, would be useful if it could
generate an error message as well ?

The use case is as follows ->
we are currently implementing support for Intel Processor Trace for lldb,
the way it is structured is that the lldb-server gathers trace data and we
have a tool running on top of SB API's
which does all the trace specific handling. So basically the client is sort
of transparent. We choose such a design so as to do the least amount of
changes in lldb.

On Thu, Jun 22, 2017 at 1:54 AM, Jim Ingham  wrote:

> Right.  I wasn't actually arguing one method over the other.  Mostly
> pointing out that you can't take error numbers seriously in general, and
> that consequently if we go the error number route, you have to know you are
> talking to lldb-server and particularly one that has rational error
> numbers.  Maybe have a qUsesLLDBSERVERErrorNumbers packet as part of the
> handshake.
>
> Jim
>
>
> > On Jun 21, 2017, at 4:35 PM, Stephane Sezer  wrote:
> >
> > True, but the error strings would be only available with lldb-server as
> well. Keeping a common table of error numbers seems like a good solution.
> >
> > On Wed, Jun 21, 2017 at 4:33 PM Jim Ingham  wrote:
> > Because the gdb remote protocol docs explicitly state:
> >
> > The error response returned for some packets includes a two character
> error number. That number is not well defined.
> >
> > we don't put much stock in the actual error numbers.
> >
> > If you can determine that you are talking to lldb-server, then we could
> actually make these meaningful by keeping a common table.  But that would
> only work for lldbserver.
> >
> > Jim
> >
> >
> > > On Jun 21, 2017, at 4:18 PM, Stephane Sezer via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> > >
> > > What's the specific use case that you're trying to support with error
> messages in the protocol? My initial thought on this is that it's not
> really the debug server's job to generate human-readable error messages and
> that the debugger is better suited to do the job.
> > >
> > > Can this problem be solved by extending the current integer list used
> for errors?
> > >
> > > On Wed, Jun 21, 2017 at 8:31 AM Ravitheja Addepally via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> > > Hello all,
> > >Currently the remote protocol in LLDB does not allow sending
> Error Strings in response to remote packets, it only allows for "ENN"
> format where N is a hex integer. In our current ongoing work, we would like
> to have support for Sending Error Strings from lldb-server. I would like to
> invite any opinions or suggestions in this matter ?
> > >
> > > A very simple proposal would be to just attach an error string maybe
> as a Name:Value Pair ? like so ->
> > >
> > > EXX;"Error String"
> > >  or
> > > EXX;M"Error String"
> > >
> > > I guess removing EXX would make it incompatible with gdb-server. Also
> adding new packets to query errors might not be desired ?
> > >
> > >
> > > Regards,
> > > A Ravi Theja
> > >
> > >
> > > ___
> > > lldb-dev mailing list
> > > lldb-dev@lists.llvm.org
> > > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
> > > --
> > > --
> > > Stephane Sezer
> > > ___
> > > lldb-dev mailing list
> > > lldb-dev@lists.llvm.org
> > > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
> >
> > --
> > --
> > Stephane Sezer
>
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev