Title: [280659] trunk/Source/_javascript_Core
- Revision
- 280659
- Author
- [email protected]
- Date
- 2021-08-04 14:17:57 -0700 (Wed, 04 Aug 2021)
Log Message
[ARM64] Fix Zoom black screen during video meeting on Safari
https://bugs.webkit.org/show_bug.cgi?id=228776
Reviewed by Saam Barati.
The problem (rdar://81434487) reports that Zoom turns to a black screen during the video
meeting on Safari. The reproduction of this problem is verified and bisected to the previous patch
(https://bugs.webkit.org/show_bug.cgi?id=228057). Previously, we introduce a pattern
matching for instruction EON-with-shift on ARM64, where the pattern is d = n ^ ((m ShiftType amount) ^ -1).
x = m ShiftType amount
y = x ^ -1
z = n ^ y
We check canBeInternal() on x but not on y based on the computing cost analysis in that patch,
which is totally wrong. If the pattern matching is triggered, then the compiler would not emit
the corresponding Air of x after lowering, leading to data corruption or system crash since y
depends on x.
In the real world example (Zoom video meeting), we find the B3 IR:
...
Int32 b@528 = SShr(b@526, $31(b@527), Wasm: {opcode: I32ShrS, location: 0x26b})
Int32 b@529 = BitXor(b@528, $-1(b@144), Wasm: {opcode: I32Xor, location: 0x26e})
...
Int32 b@551 = BitXor(b@446, b@529, Wasm: {opcode: I32Xor, location: 0x28e})
...
After Lowering to Air:
...
Not32 %fp, %x2, b@529
...
XorNotRightShift32 %tmp199, %tmp211, $31, %tmp209, b@551
...
Since the implementation of the previous patch does commitInternal() on b@528, the operand of
b@529 turns to a frame pointer. To resolve this problem, we should either check canBeInternal()
on both b@528 and b@529 or not at all.
* b3/B3LowerToAir.cpp:
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (280658 => 280659)
--- trunk/Source/_javascript_Core/ChangeLog 2021-08-04 21:06:25 UTC (rev 280658)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-08-04 21:17:57 UTC (rev 280659)
@@ -1,3 +1,48 @@
+2021-08-04 Yijia Huang <[email protected]>
+
+ [ARM64] Fix Zoom black screen during video meeting on Safari
+ https://bugs.webkit.org/show_bug.cgi?id=228776
+
+ Reviewed by Saam Barati.
+
+ The problem (rdar://81434487) reports that Zoom turns to a black screen during the video
+ meeting on Safari. The reproduction of this problem is verified and bisected to the previous patch
+ (https://bugs.webkit.org/show_bug.cgi?id=228057). Previously, we introduce a pattern
+ matching for instruction EON-with-shift on ARM64, where the pattern is d = n ^ ((m ShiftType amount) ^ -1).
+
+ x = m ShiftType amount
+ y = x ^ -1
+ z = n ^ y
+
+ We check canBeInternal() on x but not on y based on the computing cost analysis in that patch,
+ which is totally wrong. If the pattern matching is triggered, then the compiler would not emit
+ the corresponding Air of x after lowering, leading to data corruption or system crash since y
+ depends on x.
+
+ In the real world example (Zoom video meeting), we find the B3 IR:
+
+ ...
+ Int32 b@528 = SShr(b@526, $31(b@527), Wasm: {opcode: I32ShrS, location: 0x26b})
+ Int32 b@529 = BitXor(b@528, $-1(b@144), Wasm: {opcode: I32Xor, location: 0x26e})
+ ...
+ Int32 b@551 = BitXor(b@446, b@529, Wasm: {opcode: I32Xor, location: 0x28e})
+ ...
+
+
+ After Lowering to Air:
+
+ ...
+ Not32 %fp, %x2, b@529
+ ...
+ XorNotRightShift32 %tmp199, %tmp211, $31, %tmp209, b@551
+ ...
+
+ Since the implementation of the previous patch does commitInternal() on b@528, the operand of
+ b@529 turns to a frame pointer. To resolve this problem, we should either check canBeInternal()
+ on both b@528 and b@529 or not at all.
+
+ * b3/B3LowerToAir.cpp:
+
2021-08-04 Commit Queue <[email protected]>
Unreviewed, reverting r280609.
Modified: trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp (280658 => 280659)
--- trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp 2021-08-04 21:06:25 UTC (rev 280658)
+++ trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp 2021-08-04 21:17:57 UTC (rev 280659)
@@ -3207,7 +3207,7 @@
XorNotLeftShift32, XorNotLeftShift64,
XorNotRightShift32, XorNotRightShift64,
XorNotUnsignedRightShift32, XorNotUnsignedRightShift64);
- if (!isValidForm(opcode, Arg::Tmp, Arg::Tmp, Arg::Imm, Arg::Tmp))
+ if (!isValidForm(opcode, Arg::Tmp, Arg::Tmp, Arg::Imm, Arg::Tmp) || !canBeInternal(right))
return false;
Value* mValue = shiftValue->child(0);
Value* amountValue = shiftValue->child(1);
@@ -3219,6 +3219,7 @@
return false;
append(opcode, tmp(nValue), tmp(mValue), imm(amountValue), tmp(m_value));
+ commitInternal(right);
commitInternal(shiftValue);
return true;
};
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
