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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2bbf724feea9: Implement vAttachWait in lldb-server (authored by augusto2112, committed by labath). Repository: rG LLVM Github Monorepo CHANGES SI

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

augusto2112 added a comment. @clayborg @labath if possible, could one of you merge this please? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93895/new/ https://reviews.llvm.org/D93895 ___ lldb-commits m

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

augusto2112 added a comment. > Looks good, modulo the inline comment. Ok! Can I correct the issue pointed out on the comment on the next (vAttachOrWait) patch? > Do you have commit access or you need someone to commit this for you? I don't, if one of you could do it that'd be great :) Reposi

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

labath accepted this revision. labath added a comment. Looks good, modulo the inline comment. Do you have commit access or you need someone to commit this for you? Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:350 + match_info.GetPro

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

jingham added a comment. In D93895#2488372 , @clayborg wrote: > In D93895#2488249 , @jingham wrote: > >> looks good to me too. When you get around to the wait times & intervals I'd >> argue for not doing that as a

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

clayborg added a comment. In D93895#2488249 , @jingham wrote: > looks good to me too. When you get around to the wait times & intervals I'd > argue for not doing that as a GDBRemote specific addition, as Greg was > suggesting above. There's nothing gdb

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

jingham added a comment. looks good to me too. When you get around to the wait times & intervals I'd argue for not doing that as a GDBRemote specific addition, as Greg was suggesting above. There's nothing gdb-remote specific about how long you want the debug agent to wait around for some pro

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

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. LGTM. Nice and simple. We can do another patch for vAttachOrWait as it will be very simple modifications and very similar to this patch. I am find avoiding all of the polling interval and

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

augusto2112 updated this revision to Diff 314896. augusto2112 marked 5 inline comments as done. augusto2112 added a comment. Changes test to launch a process before and after we ask for the attach. Minor code fixes as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

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

labath added a comment. Thanks. This talk of all these packets has made me realize I did not completely understand the purpose of this packet. To really test the exclusion functionality of this packet, I guess we should be launching two instances of the inferior (one before sending the packet,

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

augusto2112 updated this revision to Diff 314874. augusto2112 added a comment. Back to implementation of vAttachWait Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93895/new/ https://reviews.llvm.org/D93895 Files: lldb/source/Plugins/Process/gdb-

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

labath added a comment. In D93895#2481990 , @augusto2112 wrote: >> I don't think it's a good idea to stuff all of this into a single patch. Can >> we go back to the version which just implements the basic vAttachWait packet >> (we can bikeshed on what th

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

augusto2112 added a comment. > I don't think it's a good idea to stuff all of this into a single patch. Can > we go back to the version which just implements the basic vAttachWait packet > (we can bikeshed on what the default interval should be)? I believe new > commands/options/packets should

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

labath added a comment. I don't think it's a good idea to stuff all of this into a single patch. Can we go back to the version which just implements the basic vAttachWait packet (we can bikeshed on what the default interval should be)? I believe new commands/options/packets should be done in se

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

augusto2112 added a comment. Ok, that makes sense! Let me summarize the work that needs to be done, correct me if I get something wrong: - Change back `vAttachWait` and `vAttachOrWait` to the original format of sending only the process name. - Add default values for "waitfor-interval-usec" a

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

clayborg added a comment. In D93895#2480578 , @augusto2112 wrote: > I think I get your point. If we pass the extra options in the packet, the > validation on older lldb-server versions will reject the message. > >> Another option would be to have lldb-ser

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

clayborg added a comment. In D93895#2480554 , @augusto2112 wrote: >> 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. > > > >> S

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

augusto2112 added a comment. I think I get your point. If we pass the extra options in the packet, the validation on older lldb-server versions will reject the message. > Another option would be to have lldb-server check for environment variables > for default values for --waitfor-interval and

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

augusto2112 added a comment. > 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

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

clayborg added a comment. Another option would be to have lldb-server check for environment variables for default values for --waitfor-interval and --waitfor-duration. If they are set, they become the new default values. Of course a user can launch the lldb-server manually with the options set

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

clayborg added a comment. In 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 alre

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

augusto2112 added a comment. > 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 that implementing vAttachOrWait was already 90% done, so I did just that :) The current patch as-is supports vAttachOr

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

clayborg added a comment. In D93895#2479845 , @augusto2112 wrote: > Question: why does lldb queries if vAttachOrWait is supported, but not > vAttachWait? Does it make sense to keep this query? Or should I remove it? "vAttachOrWait" allows you to attach t

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

augusto2112 added a comment. Question: why does lldb queries if vAttachOrWait is supported, but not vAttachWait? Does it make sense to keep this query? Or should I remove it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93895/new/ https://reviews

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

augusto2112 marked an inline comment as done. augusto2112 added a comment. @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, s

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

augusto2112 updated this revision to Diff 314641. augusto2112 added a comment. Herald added a subscriber: dang. Adds support to 'vAttachOrWait', as well as the 'waitfor-interval' and 'waitfor-duration' flags. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

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

augusto2112 added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:344 + // TODO: Make the polling interval configurable. + std::chrono::milliseconds polling_interval = std::chrono::seconds(1); + clayborg

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

clayborg added a comment. Patch looks good, just a question of what timeout to use by default and possible adding an option to lldb-server in case users want faster or slower polling. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:344

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

augusto2112 updated this revision to Diff 314258. augusto2112 added a comment. Fix issues pointed out in comments left in previous patch, and clean the code a bit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93895/new/ https://reviews.llvm.org/D9

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

augusto2112 added a comment. Hi @labath. Ok, I believe the test is passing now. Thank you for all the help today! Question: the original author left a TODO: // TODO: Make the polling interval configurable std::chrono::milliseconds waitfor_interval = std::chrono::seconds(1); Is this a lot

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

augusto2112 updated this revision to Diff 314231. augusto2112 added a comment. Re-include deleted files. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93895/new/ https://reviews.llvm.org/D93895 Files: lldb/source/Plugins/Process/gdb-remote/GDBRe

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

augusto2112 updated this revision to Diff 314230. augusto2112 added a comment. Update test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93895/new/ https://reviews.llvm.org/D93895 Files: lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.p

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

augusto2112 added a comment. Hi @labath. I see! I changed it to `lldbgdbserverutils.gdbremote_hex_encode_string`. Looking at the logs, I found that the checksum I inserted was wrong, so I've corrected that as well (is there a way I can calculate the checksum on the test, instead of hard-coding

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

labath added a comment. Thanks for picking this up. I'm not sure if this is the (only) reason for the failure, but I have a feeling you're encoding the packet incorrectly -- the application name should be hex-encoded. Try using `lldbgdbserverutils.gdbremote_hex_encode_string` instead. Reposit

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

augusto2112 updated this revision to Diff 313954. augusto2112 added a comment. Fix formatting issues Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93895/new/ https://reviews.llvm.org/D93895 Files: lldb/source/Plugins/Process/gdb-remote/GDBRemote

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

augusto2112 added a comment. Hi guys. I recently ran into an issue where I couldn't use --waitfor on Linux. I found out that there's a patch that adds the functionality which was only missing a test (https://reviews.llvm.org/D47879), so I decided to pick up from where it left. @labath I added

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

augusto2112 created this revision. augusto2112 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D93895 Files: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationSer