Skip to content

Commit 3feb308

Browse files
tomasstupkatomasstupka
committed
[GR-45964] Don't throw shouldNotReachHere for unsupported foreign member/array access.
PullRequest: graalpython/2773
2 parents 19a7027 + 4eb4377 commit 3feb308

File tree

3 files changed

+202
-22
lines changed

3 files changed

+202
-22
lines changed

‎graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/interop/JavaInteropTest.java

Lines changed: 162 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -58,6 +58,8 @@
5858
import org.graalvm.polyglot.PolyglotException;
5959
import org.graalvm.polyglot.Source;
6060
import org.graalvm.polyglot.Value;
61+
import org.graalvm.polyglot.proxy.ProxyArray;
62+
import org.graalvm.polyglot.proxy.ProxyHashMap;
6163
import org.junit.After;
6264
import org.junit.Before;
6365
import org.junit.Test;
@@ -586,6 +588,165 @@ public void multiplyStrBool() {
586588
assertEquals(context.eval("python", "javaStr * javaBool").asString(), "test");
587589
assertEquals(context.eval("python", "javaBool * javaStr").asString(), "test");
588590
}
591+
592+
public class UnsupportedProxyHashMap implements ProxyHashMap {
593+
@Override
594+
public long getHashSize() {
595+
return 0;
596+
}
597+
598+
@Override
599+
public boolean hasHashEntry(Value key) {
600+
return key.asString().endsWith("_exists");
601+
}
602+
603+
@Override
604+
public Object getHashValue(Value key) {
605+
throw new UnsupportedOperationException("map is not readable");
606+
}
607+
608+
@Override
609+
public void putHashEntry(Value key, Value value) {
610+
throw new UnsupportedOperationException("map is read only");
611+
}
612+
613+
@Override
614+
public boolean removeHashEntry(Value key) {
615+
throw new UnsupportedOperationException("map is read only");
616+
}
617+
618+
@Override
619+
public Object getHashEntriesIterator() {
620+
return null;
621+
}
622+
}
623+
624+
public class UnsupportedProxyArray implements ProxyArray {
625+
626+
@Override
627+
public Object get(long index) {
628+
throw new UnsupportedOperationException("array is not readable");
629+
}
630+
631+
@Override
632+
public void set(long index, Value value) {
633+
throw new UnsupportedOperationException("array is read only");
634+
}
635+
636+
@Override
637+
public boolean remove(long index) {
638+
throw new UnsupportedOperationException("array is read only");
639+
}
640+
641+
@Override
642+
public long getSize() {
643+
return 1;
644+
}
645+
}
646+
647+
@Test
648+
public void modifyUnsupportedProxyHashMap() throws IOException {
649+
Source suitePy = Source.newBuilder("python", "" +
650+
"def foo(obj):\n" +
651+
" try:\n" +
652+
" obj['foo_exists']=42\n" +
653+
" except AttributeError as e:\n" +
654+
" if(not str(e).endswith('not writable')):" +
655+
" raise e\n" +
656+
"foo",
657+
"suite.py").build();
658+
Value foo = context.eval(suitePy);
659+
foo.execute(new UnsupportedProxyHashMap());
660+
}
661+
662+
@Test
663+
public void putUnsupportedProxyHashMap() throws IOException {
664+
Source suitePy = Source.newBuilder("python", "" +
665+
"def foo(obj):\n" +
666+
" try:\n" +
667+
" obj['foo']=42\n" +
668+
" except AttributeError as e:\n" +
669+
" if(not str(e).endswith('not insertable')):" +
670+
" raise e\n" +
671+
"foo",
672+
"suite.py").build();
673+
Value foo = context.eval(suitePy);
674+
foo.execute(new UnsupportedProxyHashMap());
675+
}
676+
677+
@Test
678+
public void removeUnsupportedProxyHashMap() throws IOException {
679+
Source suitePy = Source.newBuilder("python", "" +
680+
"def foo(obj):\n" +
681+
" try:\n" +
682+
" del obj['foo']\n" +
683+
" except KeyError as e:\n" +
684+
" pass\n" +
685+
"foo",
686+
"suite.py").build();
687+
Value foo = context.eval(suitePy);
688+
foo.execute(new UnsupportedProxyHashMap());
689+
}
690+
691+
@Test
692+
public void removeExistingUnsupportedProxyHashMap() throws IOException {
693+
Source suitePy = Source.newBuilder("python", "" +
694+
"def foo(obj):\n" +
695+
" try:\n" +
696+
" del obj['foo_exists']\n" +
697+
" except AttributeError as e:\n" +
698+
" if(not str(e).endswith('not removable')):" +
699+
" raise e\n" +
700+
"foo",
701+
"suite.py").build();
702+
Value foo = context.eval(suitePy);
703+
foo.execute(new UnsupportedProxyHashMap());
704+
}
705+
706+
@Test
707+
public void getUnsupportedArrayElement() throws IOException {
708+
Source suitePy = Source.newBuilder("python", "" +
709+
"def foo(obj):\n" +
710+
" try:\n" +
711+
" return obj[0]\n" +
712+
" except IndexError as e:\n" +
713+
" if(not str(e).endswith('not readable')):" +
714+
" raise e\n" +
715+
"foo",
716+
"suite.py").build();
717+
Value foo = context.eval(suitePy);
718+
foo.execute(new UnsupportedProxyArray());
719+
}
720+
721+
@Test
722+
public void setUnsupportedArrayElement() throws IOException {
723+
Source suitePy = Source.newBuilder("python", "" +
724+
"def foo(obj):\n" +
725+
" try:\n" +
726+
" obj[0] = 42\n" +
727+
" except IndexError as e:\n" +
728+
" if(not str(e).endswith('not writable')):" +
729+
" raise e\n" +
730+
"foo",
731+
"suite.py").build();
732+
Value foo = context.eval(suitePy);
733+
foo.execute(new UnsupportedProxyArray());
734+
}
735+
736+
@Test
737+
public void removeUnsupportedArrayElement() throws IOException {
738+
Source suitePy = Source.newBuilder("python", "" +
739+
"def foo(obj):\n" +
740+
" try:\n" +
741+
" del obj[0]\n" +
742+
" except IndexError as e:\n" +
743+
" if(not str(e).endswith('not removable')):" +
744+
" raise e\n" +
745+
"foo",
746+
"suite.py").build();
747+
Value foo = context.eval(suitePy);
748+
foo.execute(new UnsupportedProxyArray());
749+
}
589750
}
590751

591752
@RunWith(Parameterized.class)

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/foreign/AccessForeignItemNodes.java

Lines changed: 34 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
*/
4141
package com.oracle.graal.python.builtins.objects.foreign;
4242

43+
import static com.oracle.graal.python.runtime.exception.PythonErrorType.AttributeError;
4344
import static com.oracle.graal.python.runtime.exception.PythonErrorType.IndexError;
4445
import static com.oracle.graal.python.runtime.exception.PythonErrorType.KeyError;
4546
import static com.oracle.graal.python.runtime.exception.PythonErrorType.TypeError;
@@ -208,16 +209,12 @@ Object doHashKey(Object object, Object key,
208209
try {
209210
return getToPythonNode().executeConvert(lib.readHashValue(object, key));
210211
} catch (UnsupportedMessageException | UnknownKeyException e) {
211-
throw CompilerDirectives.shouldNotReachHere(e);
212+
throw keyError(this, key, lib, switchEncodingNode);
212213
} finally {
213214
gil.acquire();
214215
}
215216
}
216-
try {
217-
throw raise(KeyError, switchEncodingNode.execute(lib.asTruffleString(lib.toDisplayString(key, true)), TS_ENCODING));
218-
} catch (UnsupportedMessageException e) {
219-
throw CompilerDirectives.shouldNotReachHere(e);
220-
}
217+
throw keyError(this, key, lib, switchEncodingNode);
221218
}
222219

223220
@Fallback
@@ -230,8 +227,10 @@ private Object readForeignIndex(Object object, long index, InteropLibrary libFor
230227
if (libForObject.isArrayElementReadable(object, index)) {
231228
try {
232229
return getToPythonNode().executeConvert(libForObject.readArrayElement(object, index));
233-
} catch (UnsupportedMessageException | InvalidArrayIndexException ex) {
234-
throw CompilerDirectives.shouldNotReachHere(ex);
230+
} catch (UnsupportedMessageException ex) {
231+
throw raise(IndexError, ErrorMessages.ITEM_S_OF_S_OBJ_IS_NOT_READABLE, index, object);
232+
} catch (InvalidArrayIndexException ex) {
233+
throw raise(IndexError, ErrorMessages.INVALID_INDEX_S, index);
235234
}
236235
}
237236
throw raise(IndexError, ErrorMessages.INVALID_INDEX_S, index);
@@ -324,20 +323,26 @@ Object doHashKey(Object object, Object key, Object value,
324323
try {
325324
lib.writeHashEntry(object, key, value);
326325
return PNone.NONE;
327-
} catch (UnsupportedMessageException | UnknownKeyException e) {
326+
} catch (UnknownKeyException e) {
328327
throw CompilerDirectives.shouldNotReachHere(e);
328+
} catch (UnsupportedMessageException e) {
329+
if (lib.isHashEntryExisting(object, key)) {
330+
return raise(AttributeError, ErrorMessages.ATTR_S_OF_S_OBJ_IS_NOT_WRITABLE, key, object);
331+
} else {
332+
return raise(AttributeError, ErrorMessages.ATTR_S_OF_S_OBJ_IS_NOT_INSERTABLE, key, object);
333+
}
329334
} catch (UnsupportedTypeException e) {
330335
throw raise(TypeError, ErrorMessages.TYPE_P_NOT_SUPPORTED_BY_FOREIGN_OBJ, value);
336+
} catch (Throwable e) {
337+
throw e;
331338
} finally {
332339
gil.acquire();
333340
}
334341
}
335342
wrongIndex.enter(inliningTarget);
336343
gil.release(true);
337344
try {
338-
throw raise(KeyError, switchEncodingNode.execute(lib.asTruffleString(lib.toDisplayString(key, true)), TS_ENCODING));
339-
} catch (UnsupportedMessageException e) {
340-
throw CompilerDirectives.shouldNotReachHere(e);
345+
throw keyError(this, key, lib, switchEncodingNode);
341346
} finally {
342347
gil.acquire();
343348
}
@@ -354,8 +359,10 @@ private void writeForeignIndex(Node inliningTarget, Object object, int idx, Obje
354359
try {
355360
libForObject.writeArrayElement(object, idx, value);
356361
return;
357-
} catch (InvalidArrayIndexException | UnsupportedMessageException e) {
358-
throw CompilerDirectives.shouldNotReachHere(e);
362+
} catch (InvalidArrayIndexException e) {
363+
throw raise(IndexError, ErrorMessages.INVALID_INDEX_S, idx);
364+
} catch (UnsupportedMessageException e) {
365+
throw raise(IndexError, ErrorMessages.ITEM_S_OF_S_OBJ_IS_NOT_WRITABLE, idx, object);
359366
} catch (UnsupportedTypeException e) {
360367
throw raise(TypeError, ErrorMessages.TYPE_P_NOT_SUPPORTED_BY_FOREIGN_OBJ, value);
361368
}
@@ -431,17 +438,15 @@ Object doHashKey(Object object, Object key,
431438
try {
432439
lib.removeHashEntry(object, key);
433440
return PNone.NONE;
434-
} catch (UnsupportedMessageException | UnknownKeyException e) {
441+
} catch (UnknownKeyException e) {
435442
throw CompilerDirectives.shouldNotReachHere(e);
443+
} catch (UnsupportedMessageException e) {
444+
return raise(AttributeError, ErrorMessages.ATTR_S_OF_S_OBJ_IS_NOT_REMOVABLE, key, object);
436445
} finally {
437446
gil.acquire();
438447
}
439448
}
440-
try {
441-
throw raise(KeyError, switchEncodingNode.execute(lib.asTruffleString(lib.toDisplayString(key, true)), TS_ENCODING));
442-
} catch (UnsupportedMessageException e) {
443-
throw CompilerDirectives.shouldNotReachHere(e);
444-
}
449+
throw keyError(this, key, lib, switchEncodingNode);
445450
}
446451

447452
@Fallback
@@ -458,7 +463,7 @@ private void removeForeignValue(Object object, int idx, InteropLibrary libForObj
458463
} catch (InvalidArrayIndexException ex) {
459464
throw raise(IndexError, ErrorMessages.INVALID_INDEX_S, idx);
460465
} catch (UnsupportedMessageException e) {
461-
throw CompilerDirectives.shouldNotReachHere(e);
466+
throw raise(IndexError, ErrorMessages.ITEM_S_OF_S_OBJ_IS_NOT_REMOVABLE, idx, object);
462467
}
463468
}
464469
throw raise(IndexError, ErrorMessages.INVALID_INDEX_S, idx);
@@ -468,4 +473,12 @@ public static RemoveForeignItemNode create() {
468473
return RemoveForeignItemNodeGen.create();
469474
}
470475
}
476+
477+
private static PException keyError(AccessForeignItemBaseNode node, Object key, InteropLibrary lib, TruffleString.SwitchEncodingNode switchEncodingNode) {
478+
try {
479+
return node.raise(KeyError, switchEncodingNode.execute(lib.asTruffleString(lib.toDisplayString(key, true)), TS_ENCODING));
480+
} catch (UnsupportedMessageException e) {
481+
throw CompilerDirectives.shouldNotReachHere(e);
482+
}
483+
}
471484
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/ErrorMessages.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,12 @@ public abstract class ErrorMessages {
104104
public static final TruffleString ATTR_S_OF_S_IS_NOT_READABLE = tsLiteral("attribute %s of %s objects is not readable");
105105
public static final TruffleString ATTR_S_OF_S_IS_NOT_WRITABLE = tsLiteral("attribute %s of %s is not writable");
106106
public static final TruffleString ATTR_S_OF_S_OBJ_IS_NOT_WRITABLE = tsLiteral("attribute %s of %s object is not writable");
107+
public static final TruffleString ATTR_S_OF_S_OBJ_IS_NOT_INSERTABLE = tsLiteral("attribute %s of %s object is not insertable");
108+
public static final TruffleString ATTR_S_OF_S_OBJ_IS_NOT_REMOVABLE = tsLiteral("attribute %s of %s object is not removable");
109+
public static final TruffleString ITEM_S_OF_S_OBJ_IS_NOT_READABLE = tsLiteral("item %s of %s object is not readable");
110+
public static final TruffleString ITEM_S_OF_S_OBJ_IS_NOT_WRITABLE = tsLiteral("item %s of %s object is not writable");
111+
public static final TruffleString ITEM_S_OF_S_OBJ_IS_NOT_REMOVABLE = tsLiteral("item %s of %s object is not removable");
112+
107113
public static final TruffleString ATTR_S_READONLY = tsLiteral("attribute %s is read-only");
108114
public static final TruffleString ATTR_VALUE_MUST_BE_BOOL = tsLiteral("attribute value type must be bool");
109115
public static final TruffleString B_REQUIRES_BYTES_OR_OBJ_THAT_IMPLEMENTS_S_NOT_P = tsLiteral("%%b requires a bytes-like object, or an object that implements __bytes__, not '%p'");

0 commit comments

Comments
 (0)