Make Chrome robust against XMM register corruption
XMM7 is supposed to be preserved by functions according to the Windows
ABI, but some third-party products fail to honor that, leading to
crashes in Chrome. A fix to a known-buggy third-party product has been
released but may take a while to propagate. This change tells the
compiler not to assume that the non-volatile XMM registers are preserved
after calls to run tasks. This forces the compiler to preserve the
potentially corrupted registers, which should stop the crashes.
This fix was found to be both necessary and sufficient by looking at the
disassembly in official builds and confirming that after this change XMM
registers are cleared shortly before use, and preserved/restored at
function start/stop. Previous attempts that disabled inlining of
problematic functions fixed specific instances but were not sufficient
to fix all problems and were not robust against future code changes.
See crrev.com/c/4087156 as an example of a not-quite-complete fix.
Bug: 1218384
Change-Id: Iff64204559594036c91a90abdf1d4a00501e2d00
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4087650
Auto-Submit: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1081511}
diff --git a/base/task/common/task_annotator.cc b/base/task/common/task_annotator.cc
index 3c0e31a6..9821fd2 100644
--- a/base/task/common/task_annotator.cc
+++ b/base/task/common/task_annotator.cc
@@ -19,6 +19,7 @@
#include "base/threading/thread_local.h"
#include "base/trace_event/base_tracing.h"
#include "base/tracing_buildflags.h"
+#include "build/build_config.h"
#if BUILDFLAG(ENABLE_BASE_TRACING)
#include "third_party/perfetto/protos/perfetto/trace/track_event/chrome_mojo_event_info.pbzero.h" // nogncheck
@@ -154,6 +155,26 @@
if (g_task_annotator_observer)
g_task_annotator_observer->BeforeRunTask(&pending_task);
std::move(pending_task.task).Run();
+#if BUILDFLAG(IS_WIN) && defined(ARCH_CPU_X86_FAMILY)
+ // Some tasks on some machines clobber the non-volatile XMM registers in
+ // violation of the Windows ABI. This empty assembly language block with
+ // clobber directives tells the compiler to assume that these registers
+ // may have lost their values. This ensures that this function will not rely
+ // on the registers retaining their values, and it ensures that it will
+ // restore the values when this function ends. This is needed because the
+ // code-gen for at least one caller of this function in official builds relies
+ // on an XMM register (usually XMM7, cleared to zero) maintaining its value as
+ // multiple tasks are run, which causes crashes if it is corrupted, since
+ // "zeroed" variables end up not being zeroed.
+ // The third-party issue is believed to be fixed but will take a while to
+ // propagate to users which is why this mitigation is needed.
+ // For details see https://crbug.com/1218384
+ asm(""
+ :
+ :
+ : "%xmm6", "%xmm7", "%xmm8", "%xmm9", "%xmm10", "%xmm11", "%xmm12",
+ "%xmm13", "%xmm14", "%xmm15");
+#endif
tls->Set(previous_pending_task);
