[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/ https://reviews.llvm.org/D120

[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 for

[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 i

[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/ https://reviews.llvm.o

[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 https://reviews.llvm.org

[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 if

[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 t

[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 me

[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/i

[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`. The

[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 https://reviews.llvm.org/D11979

[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 VS

[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 to

[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 to

[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 Pr

[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" look

[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 threa

[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 thr

[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 `Process::No

[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 event

[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