Title: [206844] trunk/Source/_javascript_Core
- Revision
- 206844
- Author
- [email protected]
- Date
- 2016-10-05 21:24:57 -0700 (Wed, 05 Oct 2016)
Log Message
[JSC] Do not construct Simple GetByIdStatus against self-custom-accessor case
https://bugs.webkit.org/show_bug.cgi?id=162993
Reviewed by Filip Pizlo.
We accidentally created a Simple GetByIdStatus against self-custom-accessor case: the object has own custom accessor property and get_by_id hits.
If we returned such a result, the GetById will be turned to GetByOffset and it looks up incorrect thing like CustomGetterSetter object.
We do not hit this bug before since maybe there is no object that has own custom-accessor and this custom-accessor does not raise an error.
For example, "Node.prototype" has "firstChild" custom accessor. But since "Node.prototype" itself does not have Node::info(), "Node.prototype.firstChild"
access always raises an error. I guess all the custom accessors follow this pattern. This bug is uncovered when testing DOMJIT (This bug causes crash and
it can occur even if we disabled DOMJIT).
But such a assumption is not guaranteed. In this patch, we fix this by not returning Simple GetById.
* bytecode/GetByIdStatus.cpp:
(JSC::GetByIdStatus::computeFromLLInt):
(JSC::GetByIdStatus::computeForStubInfoWithoutExitSiteFeedback):
(JSC::GetByIdStatus::computeFor):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (206843 => 206844)
--- trunk/Source/_javascript_Core/ChangeLog 2016-10-06 04:05:35 UTC (rev 206843)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-10-06 04:24:57 UTC (rev 206844)
@@ -1,3 +1,24 @@
+2016-10-05 Yusuke Suzuki <[email protected]>
+
+ [JSC] Do not construct Simple GetByIdStatus against self-custom-accessor case
+ https://bugs.webkit.org/show_bug.cgi?id=162993
+
+ Reviewed by Filip Pizlo.
+
+ We accidentally created a Simple GetByIdStatus against self-custom-accessor case: the object has own custom accessor property and get_by_id hits.
+ If we returned such a result, the GetById will be turned to GetByOffset and it looks up incorrect thing like CustomGetterSetter object.
+ We do not hit this bug before since maybe there is no object that has own custom-accessor and this custom-accessor does not raise an error.
+ For example, "Node.prototype" has "firstChild" custom accessor. But since "Node.prototype" itself does not have Node::info(), "Node.prototype.firstChild"
+ access always raises an error. I guess all the custom accessors follow this pattern. This bug is uncovered when testing DOMJIT (This bug causes crash and
+ it can occur even if we disabled DOMJIT).
+
+ But such a assumption is not guaranteed. In this patch, we fix this by not returning Simple GetById.
+
+ * bytecode/GetByIdStatus.cpp:
+ (JSC::GetByIdStatus::computeFromLLInt):
+ (JSC::GetByIdStatus::computeForStubInfoWithoutExitSiteFeedback):
+ (JSC::GetByIdStatus::computeFor):
+
2016-10-05 Saam Barati <[email protected]>
PCToCodeOriginMap builder should use labelIgnoringWatchpoints() inside the DFG
Modified: trunk/Source/_javascript_Core/bytecode/GetByIdStatus.cpp (206843 => 206844)
--- trunk/Source/_javascript_Core/bytecode/GetByIdStatus.cpp 2016-10-06 04:05:35 UTC (rev 206843)
+++ trunk/Source/_javascript_Core/bytecode/GetByIdStatus.cpp 2016-10-06 04:24:57 UTC (rev 206844)
@@ -97,10 +97,12 @@
if (structure->takesSlowPathInDFGForImpureProperty())
return GetByIdStatus(NoInformation, false);
- unsigned attributesIgnored;
- PropertyOffset offset = structure->getConcurrently(uid, attributesIgnored);
+ unsigned attributes;
+ PropertyOffset offset = structure->getConcurrently(uid, attributes);
if (!isValidOffset(offset))
return GetByIdStatus(NoInformation, false);
+ if (attributes & CustomAccessor)
+ return GetByIdStatus(NoInformation, false);
return GetByIdStatus(Simple, false, GetByIdVariant(StructureSet(structure), offset));
}
@@ -176,11 +178,13 @@
Structure* structure = stubInfo->u.byIdSelf.baseObjectStructure.get();
if (structure->takesSlowPathInDFGForImpureProperty())
return GetByIdStatus(slowPathState, true);
- unsigned attributesIgnored;
+ unsigned attributes;
GetByIdVariant variant;
- variant.m_offset = structure->getConcurrently(uid, attributesIgnored);
+ variant.m_offset = structure->getConcurrently(uid, attributes);
if (!isValidOffset(variant.m_offset))
return GetByIdStatus(slowPathState, true);
+ if (attributes & CustomAccessor)
+ return GetByIdStatus(slowPathState, true);
variant.m_structureSet.add(structure);
bool didAppend = result.appendVariant(variant);
@@ -343,6 +347,8 @@
return GetByIdStatus(TakesSlowPath); // It's probably a prototype lookup. Give up on life for now, even though we could totally be way smarter about it.
if (attributes & Accessor)
return GetByIdStatus(MakesCalls); // We could be smarter here, like strength-reducing this to a Call.
+ if (attributes & CustomAccessor)
+ return GetByIdStatus(TakesSlowPath);
if (!result.appendVariant(GetByIdVariant(structure, offset)))
return GetByIdStatus(TakesSlowPath);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
