-
Alex Kodat authored
When an Isolate in a multi-threaded environment is being debugged and a thread does a Step Over (StepNext internally) one-shot breaks are created in the code at the stack frame where the StepNext occurred. However, if the stepped-over statement had a function call and the called function (or some function that it called) unlocked the Isolate (via a C++ function call) and another thread then locked the Isolate, an ArchiveDebug would be done which would save the fact that a StepNext is active and the call frame depth of the StepNext. The one-shot breaks would then be cleared to avoid stopping the now running thread. When the original thread that did the StepNext relocks the Isolate, a RestoreDebug is done which, seeing that a StepNext was active calls PrepareDebug which assumes that the StepNext must be for the current JS frame which is usually correct, but not in this case. This results in the StepNext break actually occurring in the function that called the C++ function not in the function where the StepNext was originally done. In addition, the function where the break now happens must necessarily be deoptimized if optimized, and debug code and a source map table created if one doesn't already exists though this is largely invisible to the user. Occasionally, a crash/core dump also occurs because the stack guard is restored after the debugging environment is restored in the RestoreThread code which can prevent the compiler from being called to generate the source map table (for the incorrect function) since the stack guard is another thread's stack guard, and so might appear that the stack guard has been gone past so the compiler is not called, resulting in there being no source map table. But PrepareStep ends up calling the BreakIterator (via the DebugInfo constructor) which assumes there is a source map table so we get a crash. The fix is to have PrepareStep to skip to the frame where the StepNext was done before doing its thing. Since the only PrepareStepcaller that requires a frame other than the current frame, is RestoreDebug, a target frame parameter was added to PrepareStep that's set by RestoreDebug and defaults to -1 indicating to use the current frame for all other callers. While this made the order of the debug environment and stack guard no longer cause an obvious problem, it still felt wrong to defer restoration of the stack guard until after something as potentially complex as PrepareStep might be called, so the order of RestoreDebug and RestoreStackGuard calls were reversed. Bug: v8:10902 Change-Id: I174e254e72414c827e113aec142f1d329ebe73d8 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2405932 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Yang Guo <yangguo@chromium.org> Cr-Commit-Position: refs/heads/master@{#70152}
9329f558