Fix conditional breakpoints being triggered unconditionally during stepping and hijacking the step

[ynwarcs]

This should fix #20.

The fix is pretty straightforward but there's still room for improvement:

• Conditional breakpoints with a false condition will still trigger if they happen to occur on the same thread. I'll try to fix this too but it seems a bit more complicated. We'd need to have TitanEngine able to check the breakpoint condition (e.g. through a callback into x64) and decide whether to skip based on that.
• We still can't step from multiple different threads at the same time (Visual Studio allows this), as only one stepping thread ID will be saved. This could be circumvented by storing a vector of thread IDs awaiting a step, rather than a single TID, but I'm not sure if it's worth it?

[ynwarcs]

Maybe we should only clear the field if the unhandled exception comes from the same thread that's awaiting the step to complete?

[mrexodia]

While this is probably an improvement, I think the proper solution is to track the (internal) stepping state on a per-thread basis. This is how GleeBug implements this and I did not observe the same bug with TitanEngine. Similar bugs exist with hardware breakpoints, because the next debug event can always come from a different thread.

I don't remember from the top of my head, but I think you can trigger a similar bug by just stepping around near a CreateThread call (maybe with a breakpoint at LdrInitializeThunk). At some point the threads will switch and the state becomes corrupted.

[ynwarcs]

You're right, I hadn't really considered other stuff that could hijack the step. After having a brief look, I see that GleeBug's implementation seems quite more sophisticated than Titan's, so I assume Titan would have to undergo similar changes to cover all edge cases. Do you expect that it'd take a lot of effort to port the implementation or write an alternate one from scratch here? Is it something you'd be open to accepting, seeing as it would be a serious risk to breaking existing behaviours that other folks may be relying on?

As for my personal woes with this bug, I can use my forked version with this fix + some other junk for the time being, as it works pretty well for my needs. But I did notice some other bugs that happen from time to time, that could potentially be fixed with a rework of stepping implementation. Alternatively, I could give GleeBug a go but I'm not familiar with it and I don't see any documentation. Can it be used with x64dbg, e.g. as a drop-in replacement for titan, or is it meant to serve a different purpose?

I also just flagged the same bug in WinDBG (microsoftfeedback/WinDbg-Feedback#233), and seeing as it's been present forever, I guess not many folks are affected by it and it doesn't need to be fixed asap.

[mrexodia]

You can switch to GleeBug in the x64dbg settings. I also concluded that it is a lot of work to backport the fixed between TitanEngine and GleeBug, so my goal eventually is to just turn on GleeBug per default 🤷🏻‍♂️

The main issue right now is there are no proper tests to make sure GleeBug and TitanEngine work the same, which is kind of annoying. Scenarios like this also need to be tested and it's just a lot of work. Eventually I'll spend some time on it, I have a minimal project here that can be good for testing https://github.com/mrexodia/DebugLoopRace/tree/main