careful constants map handling
Setting up the Windows JIT made me notice that we can
modify the constants SkTHashMap while using it, e.g.
a->vfmaddps132(dst(x), reg(y), any(z))
if any() returns a Label* Operand pointing to a constant, and
reg() needs to load from a constant too, and the second to touch
the constants map causes a hash table resize, the other is left
with a Label* that's pointing into the old table.
The fix is to (mostly) never modify the constants table when
looking up constants, instead using the Op::splat to seed the
table. A couple miscellaneous constants like 0xffff'ffff as
used by assert_true are not dangerous so I've left them alone.
This is the sort of bug ASAN would catch, except we turn off
the JIT when building for ASAN. I think I can tighten that up
so that we still JIT under ASAN, but never use the JIT'd code.
That'd let us catch bugs like this in the JIT itself.
Change-Id: I91d8f6fda721a6e3c9e3629d694a73fd1b2bf8f1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/299879
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
diff --git a/src/core/SkVM.cpp b/src/core/SkVM.cpp
index 654ab8f..66fb40e 100644
--- a/src/core/SkVM.cpp
+++ b/src/core/SkVM.cpp
@@ -2789,7 +2789,7 @@
if (instructions[v].immy == 0) {
a->vpxor(r,r,r);
} else {
- a->vmovups(r, &constants[instructions[v].immy]);
+ a->vmovups(r, constants.find(instructions[v].immy));
}
} else {
SkASSERT(stack_slot[v] != NA);
@@ -2821,7 +2821,7 @@
if (instructions[v].immy == 0) {
a->eor16b(r,r,r);
} else {
- a->ldrq(r, &constants[instructions[v].immy]);
+ a->ldrq(r, constants.find(instructions[v].immy));
}
} else {
SkASSERT(stack_slot[v] != NA);
@@ -2982,7 +2982,7 @@
return (Reg)found;
}
if (instructions[v].op == Op::splat) {
- return &constants[instructions[v].immy];
+ return constants.find(instructions[v].immy);
}
return A::Mem{A::rsp, stack_slot[v]*K*4};
};
@@ -2995,8 +2995,10 @@
#endif
switch (op) {
- // Splats are handled above.
- case Op::splat: break;
+ case Op::splat:
+ // Make sure splat constants can be found by load_from_memory() or any().
+ (void)constants[immy];
+ break;
#if defined(__x86_64__)
case Op::assert_true: {
