[Lldb-commits] [PATCH] D120319: Set error message if ValueObjectRegister fails to write back to register

ilya-nozhkin added a comment. In D120319#3348267 , @ChuanqiXu wrote: > What's your name and email address for this patch? Ilya Nozhkin nozhkin...@gmail.com CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120319/new/

[Lldb-commits] [PATCH] D120319: Set error message if ValueObjectRegister fails to write back to register

ilya-nozhkin added a comment. Could someone, please, commit these changes to the main repository? I still don't have commit access. I'll apply for it right after merging these changes (LLVM Developer Policy states that it'd be better to have several patches already submitted before applying

[Lldb-commits] [PATCH] D120319: Set error message if ValueObjectRegister fails to write back to register

ilya-nozhkin added a comment. In D120319#3342657 , @labath wrote: > Another possibility is to use the gdb-client test infrastructure to simulate > a server which refuses to write to a register It would be the most proper way, I agree, but I think that

[Lldb-commits] [PATCH] D120319: Set error message if ValueObjectRegister fails to write back to register

ilya-nozhkin updated this revision to Diff 411065. ilya-nozhkin added a comment. Replaced redundant check in the test with assert. Also, I've removed type annotations to be consistent with the code around. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120319/new/

[Lldb-commits] [PATCH] D120319: Set error message if ValueObjectRegister fails to write back to register

ilya-nozhkin updated this revision to Diff 411015. ilya-nozhkin edited the summary of this revision. ilya-nozhkin added a comment. Added a test based on core files debugging. `RegisterContextCorePOSIX_x86_64` indeed forbids writing registers. CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D120319: Set error message if ValueObjectRegister fails to write back to register

ilya-nozhkin created this revision. ilya-nozhkin added reviewers: labath, clayborg. Herald added a subscriber: pengfei. ilya-nozhkin requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. `SetValueFromCString` and `SetData` methods return false

[Lldb-commits] [PATCH] D119548: [lldb] Fix race condition between lldb-vscode and stop hooks executor

ilya-nozhkin added a comment. In D119548#3334776 , @labath wrote: > Do you need someone to commit this for you? (I can probably do that tomorrow) Yes, that would be nice. I don't have commit access. Repository: rG LLVM Github Monorepo CHANGES SINCE

[Lldb-commits] [PATCH] D119548: [lldb] Fix race condition between lldb-vscode and stop hooks executor

ilya-nozhkin added a comment. In D119548#3330226 , @clayborg wrote: > Try out the https://reviews.llvm.org/D119797 patch and see if this fixes this > issue? I have just updated it after responding to comments. Yes, it fixes this particular race. But I

[Lldb-commits] [PATCH] D119548: [lldb] Fix race condition between lldb-vscode and stop hooks executor

ilya-nozhkin updated this revision to Diff 409655. ilya-nozhkin added a comment. Made requested changes. I agree with @jingham that if the listener is passed via launch info then we don't need to activate it in `CreateProcess`. I mean, activating it in `CreateProcess` was a hack that allowed

[Lldb-commits] [PATCH] D119548: [lldb] Fix race condition between lldb-vscode and stop hooks executor

ilya-nozhkin updated this revision to Diff 409404. ilya-nozhkin added a comment. Herald added a subscriber: emaste. Applied requested changes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119548/new/ https://reviews.llvm.org/D119548 Files: lldb/include/lldb/Target/Process.h

[Lldb-commits] [PATCH] D119548: [lldb] Fix race condition between lldb-vscode and stop hooks executor

ilya-nozhkin updated this revision to Diff 409091. ilya-nozhkin edited the summary of this revision. ilya-nozhkin added a comment. Herald added a subscriber: mgrang. Implemented the approach suggested by @labath. I.e. now the target's hijacking listener is activated before `Process::Launch`.

[Lldb-commits] [PATCH] D119797: Fix race condition when launching and attaching.

ilya-nozhkin added inline comments. Comment at: lldb/tools/lldb-vscode/VSCode.cpp:9 +#include #include Incompatible with MSVC. Maybe use std::chrono instead? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D119548: [lldb] Fix race condition between lldb-vscode and stop hooks executor

ilya-nozhkin added a comment. In D119548#3321452 , @clayborg wrote: > Can anyone try out the following patch and see if this stops the race > condition > > https://reviews.llvm.org/D119797 > > It might solve the problem. Yeah, now the test passes and

[Lldb-commits] [PATCH] D119548: [lldb] Fix race condition between lldb-vscode and stop hooks executor

ilya-nozhkin added a comment. In D119548#3321376 , @clayborg wrote: > FYI: I have a patch coming up that will add the ability to not have to play > with the async mode when launching or attaching. I will try and post that > diff here, it should be up

[Lldb-commits] [PATCH] D119548: [lldb] Fix race condition between lldb-vscode and stop hooks executor

ilya-nozhkin added a comment. In D119548#3321357 , @jingham wrote: > SBTarget.Launch calls Target::Launch. That sets up a hijacker for the "stop > at the first instruction" event regardless of the Sync mode. The problem that it sets up this hijacker

[Lldb-commits] [PATCH] D119548: [lldb] Fix race condition between lldb-vscode and stop hooks executor

ilya-nozhkin added a comment. In D119548#3318995 , @labath wrote: > I think that could be achieved by "hijacking" the process events in the case > of a synchronous launch, similar to how we do it for synchronous resumes > (compare Process::Resume and

[Lldb-commits] [PATCH] D119548: [lldb] Fix race condition between lldb-vscode and stop hooks executor

ilya-nozhkin updated this revision to Diff 408401. ilya-nozhkin added a comment. Moved the body of this giant lambda to a separate method. Also, renamed the locking method because "do with locked synchronization" looks like the synchronized mode will be locked. "Do with locked synchronicity"

[Lldb-commits] [PATCH] D119548: [lldb] Fix race condition between lldb-vscode and stop hooks executor

ilya-nozhkin added a comment. In D119548#3318806 , @labath wrote: > it still seems like there is a zillion of other things that the two processes > (event handler thread and stop hook executor) race on. Stop hooks are executed in the event handler

[Lldb-commits] [PATCH] D119548: [lldb] Fix race condition between lldb-vscode and stop hooks executor

ilya-nozhkin created this revision. ilya-nozhkin added reviewers: jingham, clayborg. ilya-nozhkin requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. The race is between these two pieces of code that are executed in two separate lldb-vscode

[Lldb-commits] [PATCH] D86652: [lldb] Fix ProcessEventData::DoOnRemoval to support multiple listeners

ilya-nozhkin added a comment. Herald added a subscriber: danielkiss. I've realized that I just didn't know about stop hooks... So, sorry and thanks, it solves almost all my problems except for notifying about "resume" events which are important for me too. But I can use synchronous

[Lldb-commits] [PATCH] D86652: [lldb] Fix ProcessEventData::DoOnRemoval to support multiple listeners

ilya-nozhkin added a comment. Yeah, I understand the problem of two listeners trying to change process state simultaneously and I agree that informing other listeners once the primary one finished its job would be a great solution. But the problem is that primary listener just provides an

[Lldb-commits] [PATCH] D86652: [lldb] Fix ProcessEventData::DoOnRemoval to support multiple listeners

ilya-nozhkin created this revision. ilya-nozhkin added reviewers: jingham, clayborg, labath. ilya-nozhkin added a project: LLDB. Herald added subscribers: lldb-commits, JDevlieghere. ilya-nozhkin requested review of this revision. Process does something with its state and threads when an event is