2010-07-15 Geoffrey Garen <ggaren@apple.com>

geoffreygaren · geoffreygaren · commit c8c1956014a8 · 2010-07-16T04:58:30.000Z
Reviewed by Maciej Stachowiak. Crash entering mail.yahoo.com https://bugs.webkit.org/show_bug.cgi?id=42394 * bytecompiler/BytecodeGenerator.cpp: (JSC::BytecodeGenerator::argumentNumberFor): Added a NULL check. If the identifier we're resolving is not a local variable, registerFor returns NULL. * bytecompiler/NodesCodegen.cpp: (JSC::FunctionBodyNode::emitBytecode): Unrelated to the crash, but I noticed this while working on it: No need to NULL-check returnNode, since an early return has already done so. 2010-07-15 Geoffrey Garen <ggaren@apple.com> Reviewed by Maciej Stachowiak. Test for https://bugs.webkit.org/show_bug.cgi?id=42394 Crash entering mail.yahoo.com * fast/js/numeric-compare.html: Added. * fast/js/script-tests/numeric-compare.js: Added. Canonical link: https://commits.webkit.org/54355@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@63515 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
@@ -1,3 +1,20 @@
+2010-07-15  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Maciej Stachowiak.
+
+        Crash entering mail.yahoo.com
+        https://bugs.webkit.org/show_bug.cgi?id=42394
+    
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::argumentNumberFor): Added a NULL check. If the
+        identifier we're resolving is not a local variable, registerFor returns
+        NULL.
+
+        * bytecompiler/NodesCodegen.cpp:
+        (JSC::FunctionBodyNode::emitBytecode): Unrelated to the crash, but I
+        noticed this while working on it: No need to NULL-check returnNode,
+        since an early return has already done so.
+
 2010-07-15  Martin Robinson  <mrobinson@igalia.com>
 
         Reviewed by Oliver Hunt.
diff --git a/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp b/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
@@ -2051,7 +2051,10 @@ void BytecodeGenerator::setIsNumericCompareFunction(bool isNumericCompareFunctio
 int BytecodeGenerator::argumentNumberFor(const Identifier& ident)
 {
     int parameterCount = m_parameters.size(); // includes 'this'
-    int index = registerFor(ident)->index() + RegisterFile::CallFrameHeaderSize + parameterCount;
+    RegisterID* registerID = registerFor(ident);
+    if (!registerID)
+        return 0;
+    int index = registerID->index() + RegisterFile::CallFrameHeaderSize + parameterCount;
     return (index > 0 && index < parameterCount) ? index : 0;
 }
 
diff --git a/JavaScriptCore/bytecompiler/NodesCodegen.cpp b/JavaScriptCore/bytecompiler/NodesCodegen.cpp
@@ -2036,7 +2036,7 @@ RegisterID* FunctionBodyNode::emitBytecode(BytecodeGenerator& generator, Registe
     }
 
     // If there is a return statment, and it is the only statement in the function, check if this is a numeric compare.
-    if (returnNode && static_cast<BlockNode*>(singleStatement)->singleStatement()) {
+    if (static_cast<BlockNode*>(singleStatement)->singleStatement()) {
         ExpressionNode* returnValueExpression = returnNode->value();
         if (returnValueExpression && returnValueExpression->isSubtract()) {
             ExpressionNode* lhsExpression = static_cast<SubNode*>(returnValueExpression)->lhs();
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2010-07-15  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Maciej Stachowiak.
+
+        Test for https://bugs.webkit.org/show_bug.cgi?id=42394
+        Crash entering mail.yahoo.com
+
+        * fast/js/numeric-compare.html: Added.
+        * fast/js/script-tests/numeric-compare.js: Added.
+
 2010-07-15  Adam Barth  <abarth@webkit.org>
 
         Followup to https://bugs.webkit.org/show_bug.cgi?id=42214
diff --git a/LayoutTests/fast/js/numeric-compare.html b/LayoutTests/fast/js/numeric-compare.html
@@ -0,0 +1,13 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="resources/js-test-style.css">
+<script src="resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="script-tests/numeric-compare.js"></script>
+<script src="resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/fast/js/script-tests/numeric-compare.js b/LayoutTests/fast/js/script-tests/numeric-compare.js
@@ -0,0 +1,8 @@
+description(
+'Tests that compiling a numeric comparison function does not crash'
+);
+
+var a, b;
+(function () { return a - b; })();
+
+var successfullyParsed = true;

Original file line number	Diff line number	Diff line change
`@@ -2051,7 +2051,10 @@ void BytecodeGenerator::setIsNumericCompareFunction(bool isNumericCompareFunctio`
`2051`	`2051`	`int BytecodeGenerator::argumentNumberFor(const Identifier& ident)`
`2052`	`2052`	`{`
`2053`	`2053`	`int parameterCount = m_parameters.size(); // includes 'this'`
`2054`		`- int index = registerFor(ident)->index() + RegisterFile::CallFrameHeaderSize + parameterCount;`
	`2054`	`+ RegisterID* registerID = registerFor(ident);`
	`2055`	`+ if (!registerID)`
	`2056`	`+ return 0;`
	`2057`	`+ int index = registerID->index() + RegisterFile::CallFrameHeaderSize + parameterCount;`
`2055`	`2058`	`return (index > 0 && index < parameterCount) ? index : 0;`
`2056`	`2059`	`}`
`2057`	`2060`
Original file line number	Diff line number	Diff line change
`@@ -2036,7 +2036,7 @@ RegisterID* FunctionBodyNode::emitBytecode(BytecodeGenerator& generator, Registe`
`2036`	`2036`	`}`
`2037`	`2037`
`2038`	`2038`	`// If there is a return statment, and it is the only statement in the function, check if this is a numeric compare.`
`2039`		`- if (returnNode && static_cast<BlockNode*>(singleStatement)->singleStatement()) {`
	`2039`	`+ if (static_cast<BlockNode*>(singleStatement)->singleStatement()) {`
`2040`	`2040`	`ExpressionNode* returnValueExpression = returnNode->value();`
`2041`	`2041`	`if (returnValueExpression && returnValueExpression->isSubtract()) {`
`2042`	`2042`	`ExpressionNode* lhsExpression = static_cast<SubNode*>(returnValueExpression)->lhs();`