- Revision
- 291756
- Author
- ysuz...@apple.com
- Date
- 2022-03-23 11:47:18 -0700 (Wed, 23 Mar 2022)
Log Message
[JSC][MSVC] custom getter creation needs to include classInfo since MSVC ICF is not "safe" variant
https://bugs.webkit.org/show_bug.cgi?id=238030
Reviewed by Alexey Shvayka.
MSVC performs very aggressive ICF (identical code folding) and it even merges the identical two functions
into one even though a pointer to this function is used. This means MSVC's ICF is not "safe"[1], and custom
function weakmap is broken on MSVC since it is assuming function pointers are different for different functions.
Unfortunately, it seems that there is no attribute / annotation to prevent this behavior, so we need to workaround it.
Since JSCustomGetterFunction does separate thing based on attached DOMAttribute, we need to include const ClassInfo*
into a key of JSCustomGetterFunction weakmap to ensure that two identical functions with different const ClassInfo*
do not get the same JSCustomGetterFunction.
[1]: https://static.googleusercontent.com/media/research.google.com/en//pubs/archive/36912.pdf
* runtime/JSCustomGetterFunction.h:
* runtime/JSCustomSetterFunction.h:
* runtime/JSGlobalObject.h:
* runtime/JSGlobalObjectInlines.h:
(JSC::JSGlobalObject::WeakCustomGetterOrSetterHash<T>::hash):
* runtime/JSObject.cpp:
(JSC::WeakCustomGetterOrSetterHashTranslator::hash):
(JSC::WeakCustomGetterOrSetterHashTranslator::equal):
(JSC::createCustomGetterFunction):
(JSC::createCustomSetterFunction):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (291755 => 291756)
--- trunk/Source/_javascript_Core/ChangeLog 2022-03-23 18:37:21 UTC (rev 291755)
+++ trunk/Source/_javascript_Core/ChangeLog 2022-03-23 18:47:18 UTC (rev 291756)
@@ -1,3 +1,31 @@
+2022-03-23 Yusuke Suzuki <ysuz...@apple.com>
+
+ [JSC][MSVC] custom getter creation needs to include classInfo since MSVC ICF is not "safe" variant
+ https://bugs.webkit.org/show_bug.cgi?id=238030
+
+ Reviewed by Alexey Shvayka.
+
+ MSVC performs very aggressive ICF (identical code folding) and it even merges the identical two functions
+ into one even though a pointer to this function is used. This means MSVC's ICF is not "safe"[1], and custom
+ function weakmap is broken on MSVC since it is assuming function pointers are different for different functions.
+ Unfortunately, it seems that there is no attribute / annotation to prevent this behavior, so we need to workaround it.
+ Since JSCustomGetterFunction does separate thing based on attached DOMAttribute, we need to include const ClassInfo*
+ into a key of JSCustomGetterFunction weakmap to ensure that two identical functions with different const ClassInfo*
+ do not get the same JSCustomGetterFunction.
+
+ [1]: https://static.googleusercontent.com/media/research.google.com/en//pubs/archive/36912.pdf
+
+ * runtime/JSCustomGetterFunction.h:
+ * runtime/JSCustomSetterFunction.h:
+ * runtime/JSGlobalObject.h:
+ * runtime/JSGlobalObjectInlines.h:
+ (JSC::JSGlobalObject::WeakCustomGetterOrSetterHash<T>::hash):
+ * runtime/JSObject.cpp:
+ (JSC::WeakCustomGetterOrSetterHashTranslator::hash):
+ (JSC::WeakCustomGetterOrSetterHashTranslator::equal):
+ (JSC::createCustomGetterFunction):
+ (JSC::createCustomSetterFunction):
+
2022-03-23 Chris Dumez <cdu...@apple.com>
Avoid unnecessary String constructor under FunctionExecutable::toStringSlow()
Modified: trunk/Source/_javascript_Core/runtime/JSCustomGetterFunction.h (291755 => 291756)
--- trunk/Source/_javascript_Core/runtime/JSCustomGetterFunction.h 2022-03-23 18:37:21 UTC (rev 291755)
+++ trunk/Source/_javascript_Core/runtime/JSCustomGetterFunction.h 2022-03-23 18:47:18 UTC (rev 291756)
@@ -59,6 +59,12 @@
CustomFunctionPointer getter() const { return m_getter; };
CustomFunctionPointer customFunctionPointer() const { return m_getter; };
std::optional<DOMAttributeAnnotation> domAttribute() const { return m_domAttribute; };
+ const ClassInfo* slotBaseClassInfoIfExists() const
+ {
+ if (m_domAttribute)
+ return m_domAttribute->classInfo;
+ return nullptr;
+ }
private:
JSCustomGetterFunction(VM&, NativeExecutable*, JSGlobalObject*, Structure*, const PropertyName&, CustomFunctionPointer, std::optional<DOMAttributeAnnotation>);
Modified: trunk/Source/_javascript_Core/runtime/JSCustomSetterFunction.h (291755 => 291756)
--- trunk/Source/_javascript_Core/runtime/JSCustomSetterFunction.h 2022-03-23 18:37:21 UTC (rev 291755)
+++ trunk/Source/_javascript_Core/runtime/JSCustomSetterFunction.h 2022-03-23 18:47:18 UTC (rev 291756)
@@ -58,6 +58,7 @@
const Identifier& propertyName() const { return m_propertyName; }
CustomFunctionPointer setter() const { return m_setter; };
CustomFunctionPointer customFunctionPointer() const { return m_setter; };
+ const ClassInfo* slotBaseClassInfoIfExists() const { return nullptr; }
private:
JSCustomSetterFunction(VM&, NativeExecutable*, JSGlobalObject*, Structure*, const PropertyName&, CustomFunctionPointer);
Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObject.h (291755 => 291756)
--- trunk/Source/_javascript_Core/runtime/JSGlobalObject.h 2022-03-23 18:37:21 UTC (rev 291755)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObject.h 2022-03-23 18:47:18 UTC (rev 291756)
@@ -599,7 +599,7 @@
struct WeakCustomGetterOrSetterHash {
static unsigned hash(const Weak<T>&);
static bool equal(const Weak<T>&, const Weak<T>&);
- static unsigned hash(const PropertyName&, typename T::CustomFunctionPointer);
+ static unsigned hash(const PropertyName&, typename T::CustomFunctionPointer, const ClassInfo*);
static constexpr bool safeToCompareToEmptyOrDeleted = false;
};
Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObjectInlines.h (291755 => 291756)
--- trunk/Source/_javascript_Core/runtime/JSGlobalObjectInlines.h 2022-03-23 18:37:21 UTC (rev 291755)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObjectInlines.h 2022-03-23 18:47:18 UTC (rev 291756)
@@ -34,6 +34,7 @@
#include "JSFunction.h"
#include "LinkTimeConstant.h"
#include "ObjectPrototype.h"
+#include <wtf/Hasher.h>
namespace JSC {
@@ -153,7 +154,7 @@
{
if (!value)
return 0;
- return hash(value->propertyName(), value->customFunctionPointer());
+ return hash(value->propertyName(), value->customFunctionPointer(), value->slotBaseClassInfoIfExists());
}
template<typename T>
@@ -165,12 +166,11 @@
}
template<typename T>
-inline unsigned JSGlobalObject::WeakCustomGetterOrSetterHash<T>::hash(const PropertyName& propertyName, typename T::CustomFunctionPointer functionPointer)
+inline unsigned JSGlobalObject::WeakCustomGetterOrSetterHash<T>::hash(const PropertyName& propertyName, typename T::CustomFunctionPointer functionPointer, const ClassInfo* classInfo)
{
- unsigned hash = DefaultHash<typename T::CustomFunctionPointer>::hash(functionPointer);
if (!propertyName.isNull())
- hash = WTF::pairIntHash(hash, propertyName.uid()->existingSymbolAwareHash());
- return hash;
+ return WTF::computeHash(functionPointer, propertyName.uid()->existingSymbolAwareHash(), classInfo);
+ return WTF::computeHash(functionPointer, classInfo);
}
} // namespace JSC
Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (291755 => 291756)
--- trunk/Source/_javascript_Core/runtime/JSObject.cpp 2022-03-23 18:37:21 UTC (rev 291755)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp 2022-03-23 18:47:18 UTC (rev 291756)
@@ -3621,11 +3621,11 @@
struct WeakCustomGetterOrSetterHashTranslator {
using BaseHash = JSGlobalObject::WeakCustomGetterOrSetterHash<T>;
- using Key = std::pair<PropertyName, typename T::CustomFunctionPointer>;
+ using Key = std::tuple<PropertyName, typename T::CustomFunctionPointer, const ClassInfo*>;
static unsigned hash(const Key& key)
{
- return BaseHash::hash(std::get<0>(key), std::get<1>(key));
+ return BaseHash::hash(std::get<0>(key), std::get<1>(key), std::get<2>(key));
}
static bool equal(const Weak<T>& a, const Key& b)
@@ -3632,7 +3632,7 @@
{
if (!a)
return false;
- return a->propertyName() == std::get<0>(b) && a->customFunctionPointer() == std::get<1>(b);
+ return a->propertyName() == std::get<0>(b) && a->customFunctionPointer() == std::get<1>(b) && a->slotBaseClassInfoIfExists() == std::get<2>(b);
}
};
@@ -3643,7 +3643,10 @@
// WeakGCSet::ensureValue's functor must not invoke GC since GC can modify WeakGCSet in the middle of HashSet::ensure.
// We use DeferGC here (1) not to invoke GC when executing WeakGCSet::ensureValue and (2) to avoid looking up HashSet twice.
DeferGC deferGC(vm);
- return globalObject->customGetterFunctionSet().ensureValue<Translator>(std::make_pair(propertyName, getValueFunc), [&] {
+ const ClassInfo* classInfo = nullptr;
+ if (domAttribute)
+ classInfo = domAttribute->classInfo;
+ return globalObject->customGetterFunctionSet().ensureValue<Translator>(std::tuple { propertyName, getValueFunc, classInfo }, [&] {
return JSCustomGetterFunction::create(vm, globalObject, propertyName, getValueFunc, domAttribute);
});
}
@@ -3655,7 +3658,7 @@
// WeakGCSet::ensureValue's functor must not invoke GC since GC can modify WeakGCSet in the middle of HashSet::ensure.
// We use DeferGC here (1) not to invoke GC when executing WeakGCSet::ensureValue and (2) to avoid looking up HashSet twice.
DeferGC deferGC(vm);
- return globalObject->customSetterFunctionSet().ensureValue<Translator>(std::make_pair(propertyName, putValueFunc), [&] {
+ return globalObject->customSetterFunctionSet().ensureValue<Translator>(std::tuple { propertyName, putValueFunc, nullptr }, [&] {
return JSCustomSetterFunction::create(vm, globalObject, propertyName, putValueFunc);
});
}
