in lengthWithState check for negative values before calling asSize, otherwise OverflowError is thrown instead of ValueError

Author: tomasstupka

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/PythonAbstractObject.java

@@ -76,6 +76,7 @@
 
 import com.oracle.graal.python.PythonLanguage;
 import com.oracle.graal.python.builtins.PythonBuiltinClassType;
+import static com.oracle.graal.python.builtins.PythonBuiltinClassType.OverflowError;
 import com.oracle.graal.python.builtins.modules.BuiltinFunctions;
 import com.oracle.graal.python.builtins.modules.MathGuards;
 import com.oracle.graal.python.builtins.modules.WarningsModuleBuiltins.WarnNode;
@@ -645,25 +646,32 @@ public boolean isSequence(@CachedLibrary("this") PythonObjectLibrary plib,
     public int lengthWithState(ThreadState state,
                     @CachedLibrary("this") PythonObjectLibrary plib,
                     @Shared("methodLib") @CachedLibrary(limit = "2") PythonObjectLibrary methodLib,
-                    @Shared("gotState") @Cached ConditionProfile gotState,
                     @Exclusive @Cached ConditionProfile hasLen,
                     @Exclusive @Cached ConditionProfile ltZero,
                     @Shared("raise") @Cached PRaiseNode raiseNode,
-                    @Exclusive @CachedLibrary(limit = "1") PythonObjectLibrary lib) {
+                    @Exclusive @CachedLibrary(limit = "1") PythonObjectLibrary lib,
+                    @Exclusive @Cached CastToJavaLongLossyNode toLong,
+                    @Exclusive @Cached ConditionProfile ignoreOverflow,
+                    @Exclusive @Cached BranchProfile overflow) {
         Object lenFunc = plib.lookupAttributeOnType(this, __LEN__);
         if (hasLen.profile(lenFunc != PNone.NO_VALUE)) {
             Object lenResult = methodLib.callUnboundMethodWithState(lenFunc, state, this);
-            int len;
-            if (gotState.profile(state == null)) {
-                len = lib.asSize(lenResult);
-            } else {
-                len = lib.asSizeWithState(lenResult, state);
-            }
-            if (ltZero.profile(len < 0)) {
-                throw raiseNode.raise(PythonBuiltinClassType.ValueError, ErrorMessages.LEN_SHOULD_RETURN_MT_ZERO);
-            } else {
-                return len;
+            // the following mimics typeobject.c#slot_sq_length()
+            // - PyNumber_Index is called first
+            // - checked if negative
+            // - PyNumber_AsSsize_t is called, in scope of which PyNumber_Index is called again
+            lenResult = lib.asIndexWithState(lenResult, state);
+            long longResult;
+            try {
+                longResult = toLong.execute(lenResult); // this is a lossy cast
+                if (ltZero.profile(longResult < 0)) {
+                    throw raiseNode.raise(PythonBuiltinClassType.ValueError, ErrorMessages.LEN_SHOULD_RETURN_MT_ZERO);
+                }
+            } catch (CannotCastException e) {
+                CompilerDirectives.transferToInterpreterAndInvalidate();
+                throw new IllegalStateException("cannot cast index to long - must not happen because then the #asIndex message impl should have raised");
             }
+            return longToInt(longResult, overflow, ignoreOverflow, OverflowError, raiseNode, lenResult);
         } else {
             throw raiseNode.raiseHasNoLength(this);
         }
@@ -1105,6 +1113,10 @@ public int asSizeWithState(Object type, ThreadState state,
             CompilerDirectives.transferToInterpreterAndInvalidate();
             throw new IllegalStateException("cannot cast index to long - must not happen because then the #asIndex message impl should have raised");
         }
+        return longToInt(longResult, overflow, ignoreOverflow, type, raise, result);
+    }
+
+    private static int longToInt(long longResult, BranchProfile overflow, ConditionProfile ignoreOverflow, Object type, PRaiseNode raise, Object result) throws PException {
         try {
             return PInt.intValueExact(longResult);
         } catch (OverflowException e) {

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/dict/PDict.java

@@ -53,6 +53,8 @@
 import com.oracle.truffle.api.library.ExportMessage;
 import com.oracle.truffle.api.object.Shape;
 import com.oracle.truffle.api.profiles.ConditionProfile;
+import com.oracle.graal.python.nodes.util.CastToJavaLongLossyNode;
+import com.oracle.truffle.api.profiles.BranchProfile;
 
 @ExportLibrary(PythonObjectLibrary.class)
 public final class PDict extends PHashingCollection {
@@ -143,13 +145,15 @@ static int doBuiltin(PDict self, ThreadState state,
         static int doSubclassed(PDict self, ThreadState state,
                         @CachedLibrary("self") PythonObjectLibrary plib,
                         @Shared("methodLib") @CachedLibrary(limit = "2") PythonObjectLibrary methodLib,
-                        @Shared("gotState") @Cached ConditionProfile gotState,
                         @Exclusive @Cached ConditionProfile hasLen,
                         @Exclusive @Cached ConditionProfile ltZero,
                         @Shared("raise") @Cached PRaiseNode raiseNode,
-                        @Exclusive @CachedLibrary(limit = "1") PythonObjectLibrary lib) {
+                        @Exclusive @CachedLibrary(limit = "1") PythonObjectLibrary lib,
+                        @Exclusive @Cached CastToJavaLongLossyNode toLong,
+                        @Exclusive @Cached ConditionProfile ignoreOverflow,
+                        @Exclusive @Cached BranchProfile overflow) {
             // call the generic implementation in the superclass
-            return self.lengthWithState(state, plib, methodLib, gotState, hasLen, ltZero, raiseNode, lib);
+            return self.lengthWithState(state, plib, methodLib, hasLen, ltZero, raiseNode, lib, toLong, ignoreOverflow, overflow);
         }
     }
 

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/str/PString.java

@@ -41,6 +41,7 @@
 import com.oracle.graal.python.nodes.object.IsBuiltinClassProfile;
 import com.oracle.graal.python.nodes.util.CannotCastException;
 import com.oracle.graal.python.nodes.util.CastToJavaIntExactNode;
+import com.oracle.graal.python.nodes.util.CastToJavaLongLossyNode;
 import com.oracle.graal.python.nodes.util.CastToJavaStringNode;
 import com.oracle.graal.python.runtime.sequence.PSequence;
 import com.oracle.graal.python.runtime.sequence.storage.SequenceStorage;
@@ -57,6 +58,7 @@
 import com.oracle.truffle.api.library.ExportMessage;
 import com.oracle.truffle.api.library.ExportMessage.Ignore;
 import com.oracle.truffle.api.object.Shape;
+import com.oracle.truffle.api.profiles.BranchProfile;
 import com.oracle.truffle.api.profiles.ConditionProfile;
 
 @ExportLibrary(InteropLibrary.class)
@@ -160,13 +162,15 @@ static int nativeStringMat(@SuppressWarnings("unused") PString self, @SuppressWa
         static int subclassedString(PString self, ThreadState state,
                         @CachedLibrary("self") PythonObjectLibrary plib,
                         @Shared("methodLib") @CachedLibrary(limit = "2") PythonObjectLibrary methodLib,
-                        @Shared("gotState") @Cached ConditionProfile gotState,
                         @Exclusive @Cached ConditionProfile hasLen,
                         @Exclusive @Cached ConditionProfile ltZero,
                         @Shared("raise") @Cached PRaiseNode raiseNode,
-                        @Exclusive @CachedLibrary(limit = "1") PythonObjectLibrary lib) {
+                        @Exclusive @CachedLibrary(limit = "1") PythonObjectLibrary lib,
+                        @Exclusive @Cached CastToJavaLongLossyNode toLong,
+                        @Exclusive @Cached ConditionProfile ignoreOverflow,
+                        @Exclusive @Cached BranchProfile overflow) {
             // call the generic implementation in the superclass
-            return self.lengthWithState(state, plib, methodLib, gotState, hasLen, ltZero, raiseNode, lib);
+            return self.lengthWithState(state, plib, methodLib, hasLen, ltZero, raiseNode, lib, toLong, ignoreOverflow, overflow);
         }
 
         static NativeCharSequence getNativeCharSequence(PString self) {