• Alex Kodat's avatar
    [debug] Restore StepNext on correct frame for RestoreDebug · 9329f558
    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: 's avatarToon Verwaest <verwaest@chromium.org>
    Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70152}
    9329f558
v8threads.cc 10.5 KB