Title: [165162] trunk
- Revision
- 165162
- Author
- [email protected]
- Date
- 2014-03-05 19:17:28 -0800 (Wed, 05 Mar 2014)
Log Message
llint_slow_path_check_has_instance() should not adjust PC before accessing operands.
<https://webkit.org/b/129768>
Reviewed by Mark Hahnenberg.
Source/_javascript_Core:
When evaluating "a instanceof b" where b is an object that ImplementsHasInstance
and OverridesHasInstance (e.g. a bound function), the LLINT will take the slow
path llint_slow_path_check_has_instance(), and execute a code path that does the
following:
1. Adjusts the byte code PC to the jump target PC.
2. For the purpose of storing the result, get the result registerIndex from the
1st operand using the PC as if the PC is still pointing to op_check_has_instance
bytecode.
The result is that whatever value resides after where the jump target PC is will
be used as a result register value. Depending on what that value is, the result
can be:
1. the code coincidently works correctly
2. memory corruption
3. crashes
The fix is to only adjust the byte code PC after we have stored the result.
* llint/LLIntSlowPaths.cpp:
(llint_slow_path_check_has_instance):
LayoutTests:
* js/instanceof-operator-expected.txt:
* js/script-tests/instanceof-operator.js:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (165161 => 165162)
--- trunk/LayoutTests/ChangeLog 2014-03-06 03:06:37 UTC (rev 165161)
+++ trunk/LayoutTests/ChangeLog 2014-03-06 03:17:28 UTC (rev 165162)
@@ -1,3 +1,13 @@
+2014-03-05 Mark Lam <[email protected]>
+
+ llint_slow_path_check_has_instance() should not adjust PC before accessing operands.
+ <https://webkit.org/b/129768>
+
+ Reviewed by Mark Hahnenberg.
+
+ * js/instanceof-operator-expected.txt:
+ * js/script-tests/instanceof-operator.js:
+
2014-03-05 Oliver Hunt <[email protected]>
Support caching of custom setters
Modified: trunk/LayoutTests/js/instanceof-operator-expected.txt (165161 => 165162)
--- trunk/LayoutTests/js/instanceof-operator-expected.txt 2014-03-06 03:06:37 UTC (rev 165161)
+++ trunk/LayoutTests/js/instanceof-operator-expected.txt 2014-03-06 03:17:28 UTC (rev 165162)
@@ -4,6 +4,7 @@
PASS getterCalled is false
+PASS foo() is false
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/js/script-tests/instanceof-operator.js (165161 => 165162)
--- trunk/LayoutTests/js/script-tests/instanceof-operator.js 2014-03-06 03:06:37 UTC (rev 165161)
+++ trunk/LayoutTests/js/script-tests/instanceof-operator.js 2014-03-06 03:17:28 UTC (rev 165162)
@@ -8,3 +8,34 @@
} catch (e) {
}
shouldBeFalse("getterCalled");
+
+// Regression test for <https://webkit.org/b/129768>.
+// This test should not crash.
+function dummyFunction() {}
+var c = dummyFunction.bind();
+
+function foo() {
+ // To reproduce the issue of <https://webkit.org/b/129768>, we need to do
+ // an instanceof test against an object that has the following attributes:
+ // ImplementsHasInstance, and OverridesHasInstance. A bound function fits
+ // the bill.
+ var result = c instanceof c;
+
+ // This is where the op_check_has_instance bytecode jumps to after the
+ // instanceof test. At this location, we need the word at offset 1 to be
+ // a ridiculously large value that can't be a valid stack register index.
+ // To achieve that, we use an op_loop_hint followed by any other bytecode
+ // instruction. The op_loop_hint takes up exactly 1 word, and the word at
+ // offset 1 that follows after is the opcode of the next instruction. In
+ // the LLINT, that opcode value will be a pointer to the opcode handler
+ // which will be large and exactly what we need. Hence, we plant a loop
+ // here for the op_loop_hint, and have some instruction inside the loop.
+ while (true) {
+ var dummy2 = 123456789;
+ break;
+ }
+ return result;
+}
+
+shouldBeFalse("foo()");
+
Modified: trunk/Source/_javascript_Core/ChangeLog (165161 => 165162)
--- trunk/Source/_javascript_Core/ChangeLog 2014-03-06 03:06:37 UTC (rev 165161)
+++ trunk/Source/_javascript_Core/ChangeLog 2014-03-06 03:17:28 UTC (rev 165162)
@@ -1,3 +1,31 @@
+2014-03-05 Mark Lam <[email protected]>
+
+ llint_slow_path_check_has_instance() should not adjust PC before accessing operands.
+ <https://webkit.org/b/129768>
+
+ Reviewed by Mark Hahnenberg.
+
+ When evaluating "a instanceof b" where b is an object that ImplementsHasInstance
+ and OverridesHasInstance (e.g. a bound function), the LLINT will take the slow
+ path llint_slow_path_check_has_instance(), and execute a code path that does the
+ following:
+ 1. Adjusts the byte code PC to the jump target PC.
+ 2. For the purpose of storing the result, get the result registerIndex from the
+ 1st operand using the PC as if the PC is still pointing to op_check_has_instance
+ bytecode.
+
+ The result is that whatever value resides after where the jump target PC is will
+ be used as a result register value. Depending on what that value is, the result
+ can be:
+ 1. the code coincidently works correctly
+ 2. memory corruption
+ 3. crashes
+
+ The fix is to only adjust the byte code PC after we have stored the result.
+
+ * llint/LLIntSlowPaths.cpp:
+ (llint_slow_path_check_has_instance):
+
2014-03-05 Ryosuke Niwa <[email protected]>
Another build fix attempt after r165141.
Modified: trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp (165161 => 165162)
--- trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2014-03-06 03:06:37 UTC (rev 165161)
+++ trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2014-03-06 03:17:28 UTC (rev 165162)
@@ -121,6 +121,14 @@
LLINT_END_IMPL(); \
} while (false)
+#define LLINT_RETURN_WITH_PC_ADJUSTMENT(value, pcAdjustment) do { \
+ JSValue __r_returnValue = (value); \
+ LLINT_CHECK_EXCEPTION(); \
+ LLINT_OP(1) = __r_returnValue; \
+ pc += (pcAdjustment); \
+ LLINT_END_IMPL(); \
+ } while (false)
+
#define LLINT_RETURN_PROFILED(opcode, value) do { \
JSValue __rp_returnValue = (value); \
LLINT_CHECK_EXCEPTION(); \
@@ -535,8 +543,8 @@
JSObject* baseObject = asObject(baseVal);
ASSERT(!baseObject->structure()->typeInfo().implementsDefaultHasInstance());
if (baseObject->structure()->typeInfo().implementsHasInstance()) {
- pc += pc[4].u.operand;
- LLINT_RETURN(jsBoolean(baseObject->methodTable()->customHasInstance(baseObject, exec, value)));
+ JSValue result = jsBoolean(baseObject->methodTable()->customHasInstance(baseObject, exec, value));
+ LLINT_RETURN_WITH_PC_ADJUSTMENT(result, pc[4].u.operand);
}
}
LLINT_THROW(createInvalidParameterError(exec, "instanceof", baseVal));
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
