Skip to content

Commit 77034bc

Browse files
committed
fix foreign instanceof to check prototype hierarchy and extend testing
1 parent 99f9488 commit 77034bc

File tree

2 files changed

+27
-16
lines changed

2 files changed

+27
-16
lines changed

graal-js/src/com.oracle.truffle.js.test/src/com/oracle/truffle/js/test/interop/ForeignObjectPrototypeTest.java

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -206,21 +206,32 @@ public void testJSBuiltinCanBeOverwritten() {
206206

207207
@Test
208208
public void testForeignInstanceof() {
209-
testInstanceofIntl("Array", ProxyArray.fromArray("fun", "with", "proxy", "array"));
210-
testInstanceofIntl("Date", Instant.now());
211-
testInstanceofIntl("Map", new TestTruffleHash());
212-
testInstanceofIntl("String", new TestTruffleString());
213-
testInstanceofIntl("Boolean", new TestTruffleBoolean());
214-
testInstanceofIntl("Number", new TestTruffleNumber());
215-
testInstanceofIntl("Function", (ProxyExecutable) v -> true);
216-
testInstanceofIntl("Object", new Object());
209+
// test expected Instance
210+
Assert.assertTrue(testInstanceofIntl("Array", ProxyArray.fromArray("fun", "with", "proxy", "array")));
211+
// Assert.assertTrue(testInstanceofIntl("Date", Instant.now())); //see GR-39319
212+
Assert.assertTrue(testInstanceofIntl("Map", new TestTruffleHash()));
213+
Assert.assertTrue(testInstanceofIntl("String", new TestTruffleString()));
214+
Assert.assertTrue(testInstanceofIntl("Boolean", new TestTruffleBoolean()));
215+
Assert.assertTrue(testInstanceofIntl("Number", new TestTruffleNumber()));
216+
Assert.assertTrue(testInstanceofIntl("Function", (ProxyExecutable) v -> true));
217+
Assert.assertTrue(testInstanceofIntl("Object", new Object()));
218+
219+
// test non-matching instance
220+
Assert.assertFalse(testInstanceofIntl("RegExp", ProxyArray.fromArray("fun", "with", "proxy", "array")));
221+
Assert.assertFalse(testInstanceofIntl("RegExp", Instant.now()));
222+
Assert.assertFalse(testInstanceofIntl("RegExp", new TestTruffleHash()));
223+
Assert.assertFalse(testInstanceofIntl("RegExp", new TestTruffleString()));
224+
Assert.assertFalse(testInstanceofIntl("RegExp", new TestTruffleBoolean()));
225+
Assert.assertFalse(testInstanceofIntl("RegExp", new TestTruffleNumber()));
226+
Assert.assertFalse(testInstanceofIntl("RegExp", (ProxyExecutable) v -> true));
227+
Assert.assertFalse(testInstanceofIntl("RegExp", new Object()));
217228
}
218229

219-
private static void testInstanceofIntl(String prototype, Object obj) {
220-
String code = "(obj) => { return (obj instanceof " + prototype + "); }";
230+
private static boolean testInstanceofIntl(String prototype, Object obj) {
231+
String code = "(obj) => { return (obj instanceof " + prototype + ") && (obj instanceof Object); }";
221232
try (Context context = JSTest.newContextBuilder(ID).build()) {
222233
Value result = context.eval(ID, code).execute(obj);
223-
Assert.assertTrue(result.asBoolean());
234+
return result.asBoolean();
224235
}
225236
}
226237

graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/binary/InstanceofNode.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -232,24 +232,24 @@ protected boolean doNotCallable(Object obj, Object check) {
232232

233233
@Specialization(guards = {"isJSFunction(check)", "isBoundFunction(check)"})
234234
protected boolean doIsBound(Object obj, JSDynamicObject check,
235-
@Cached("create(context)") InstanceofNode instanceofNode) {
235+
@Cached("create(context)") @Shared("instanceofNode") InstanceofNode instanceofNode) {
236236
JSDynamicObject boundTargetFunction = JSFunction.getBoundTargetFunction(check);
237237
return instanceofNode.executeBoolean(obj, boundTargetFunction);
238238
}
239239

240240
@Specialization(guards = {"!isJSObject(left)", "isForeignObject(left)", "isJSFunction(right)", "!isBoundFunction(right)"})
241241
protected boolean doForeignObject(@SuppressWarnings("unused") Object left, @SuppressWarnings("unused") JSDynamicObject right,
242242
@Cached ForeignObjectPrototypeNode getForeignPrototypeNode,
243-
@Cached @Shared("getPrototype1Node") GetPrototypeNode getPrototype1Node,
244-
@Cached @Shared("invalidPrototypeBranch") BranchProfile invalidPrototypeBranch) {
243+
@Cached @Shared("invalidPrototypeBranch") BranchProfile invalidPrototypeBranch,
244+
@Cached("create(context)") @Shared("instanceofNode") InstanceofNode instanceofNode) {
245245
if (context.isOptionForeignObjectPrototype()) {
246246
Object rightProto = getConstructorPrototype(right, invalidPrototypeBranch);
247247
if (rightProto == getRealm().getDatePrototype()) {
248+
// necessary because of GR-39319
248249
return false;
249250
}
250251
Object foreignProto = getForeignPrototypeNode.execute(left);
251-
Object foreignProtoProto = getPrototype1Node.execute(foreignProto);
252-
return rightProto == foreignProtoProto;
252+
return instanceofNode.executeBoolean(foreignProto, right);
253253
} else {
254254
return false;
255255
}

0 commit comments

Comments
 (0)