[Lldb-commits] [PATCH] D93895: Implement vAttachWait in lldb-server

Tue Jan 5 14:06:46 PST 2021

clayborg added a comment.

In D93895#2480324 <https://reviews.llvm.org/D93895#2480324>, @augusto2112 wrote:

>> So it might be nice to add support for vAttachOrWait, along with the query packet, in this patch if you have the time?
>
> Hi @clayborg, I saw that implementing vAttachOrWait was already 90% done, so I did just that :) The current patch as-is supports vAttachOrWait. My question was more on the lines of if keeping the query for vAttachOrWait made sense now that it's implemented on Linux. Are there be other targets where it isn't implemented?

Best to not change anything if we don't know. Yes there are other GDB servers out there that may not implement vAttachOrWait, so I would vote to not change anything.

> I also left a comment with a couple of questions earlier, but I think it got hidden by my last comment. I'll repost it here:
>
>> @clayborg I've added support for the 'waitfor-interval' and 'waitfor-duration' flags. Yesterday I thought they existed in macOS, but now I'm not so sure, as I couldn't find them on "Options.td". They were added in 2009, so maybe they did exist but were dropped later. Or I just didn't look at the right place.
>>
>> A couple of questions:
>>
>> - About the default polling interval, I've set it to 1ms for now, but I found that (in my system) this keeps a core at 100%. 10ms keeps still keeps it at around 80%, and 100ms keeps it at around 35%. Maybe 100ms is a good balance?

debugserver in the llvm sources uses 1ms as its default. I would vote to be as quick as possible so we can catch the process as soon as possible. The option --waitfor-interval specifies the time in microseconds, so we will want to mirror that. So even though it does peg one core at 100%, I think it might be worth it to catch the process sooner than later.

>> - On "Options.td", besides the "process attach" command, there's also a "platform process attach". I haven't touched it since I'm not familiar with the platform command. Should I add these flags to the platform counterpart as well?

Just "process attach" is fine to start with. The platform layer doesn't always have to use a GDB server, so adding those options there could end up being hard for some platforms to implement. In fact I would mind if we don't even add these options to "process attach" and just always use the defaults, because as you say, if someone selects or replaces the lldb-server with an older one, then we don't want the launching of the lldb-server to fail because it doesn't recognize an option that is being passed to it. The better way to fix this would be to make a new attach wait packet that takes these as arguments to the packet, but we don't need to do that.

> Could you confirm for me that 'waitfor-interval' and 'waitfor-duration' don't exist on macOS? If I'm sending a different message format than what's done on macOS that'd be an issue.

Here lies the problem that I mentioned above. I would like to avoid having to launch lldb-server with any arguments so that we continue to work with older lldb-servers.

So maybe we just rely on defaults for now and avoid having to add any new arguments to "process attach" and to the launching of the lldb-server? Let me know your thoughts.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93895