• Bruce Dawson's avatar
    Revert "Speculative fix to crashes from a CPU bug" · e84e9b92
    Bruce Dawson authored
    This reverts commit 10360127.
    
    Reason for revert: This fix only had moderate impact and the
    underlying CPU bug has now been addressed.
    
    Original change's description:
    > Speculative fix to crashes from a CPU bug
    > 
    > For the last few months Chrome has been seeing many "impossible" crashes
    > on Intel Gemini Lake, family 6 model 122 stepping 1 CPUs. These crashes
    > only happen with 64-bit Chrome and only happen in the prologue of two
    > functions. The crashes come and go across different Chrome versions.
    > Analysis of most of the crashes shows that the address of the crashing
    > instruction follows some patterns:
    > 
    > When crashing in GetFieldIndex() the last byte of the address is always
    > 1c, 5c, 9c, or dc.
    > 
    > When crashing in UpdateCaches (fewer unique samples) the last byte of
    > the address is always 5d or 9d.
    > 
    > The address of the function is 0xc or 0xd bytes earlier so the crashing
    > functions always start with an address that ends in 10, 50, 90, or d0.
    > 
    > Those addresses are for the crashes on a load of the __security_cookie.
    > The crashes also occasionally happen on the two instructions that follow
    > the __security_cookie load in which case the crashing instruction's
    > address has been seen to end with 23 or a3. This corresponds to a
    > function start address of 10 or 90.
    > 
    > Since the crash involves reading incorrect instruction bytes when
    > crossing a 16-byte boundary and since the crash appears to only happen
    > with particular 16-byte alignments it seems reasonable to force the
    > function's alignments to a multiple of 32 to see if this reliably
    > avoids the crashes. This change uses the gcc/clang __attribute__
    > directive to force 32-byte alignment. I have tested this change enough to
    > verify that it triggers the desired alignment (with up to 31 "int 3"
    > instructions added for padding) but since I have never reproduced this
    > crash I have no way of testing its efficacy.
    > 
    > Bug: chromium:968683, chromium:964273
    > Change-Id: Ia6e1c6d1e044b84d274817374b25523303e78b51
    > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1803775
    > Reviewed-by: Toon Verwaest <verwaest@chromium.org>
    > Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
    > Cr-Commit-Position: refs/heads/master@{#63804}
    
    TBR=brucedawson@chromium.org,verwaest@chromium.org
    
    # Not skipping CQ checks because original CL landed > 1 day ago.
    
    Bug: chromium:968683, chromium:964273
    Change-Id: I150ecfebeff95e8f63dbba74d78491867dc17736
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2134728
    Commit-Queue: Toon Verwaest <verwaest@chromium.org>
    Auto-Submit: Bruce Dawson <brucedawson@chromium.org>
    Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#66977}
    e84e9b92
lookup.cc 47.9 KB