Fix asFileDescriptorWithState: raise if the result is negative

Author: steve-s

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/PosixModuleBuiltins.java

@@ -2407,13 +2407,13 @@ public static OffsetConversionNode create() {
     }
 
     /**
-     * Equivalent of CPython's {@code fildes_converter()}. Always returns an {@code int}.
+     * Equivalent of CPython's {@code fildes_converter()}, which in turn delegates to
+     * {@code PyObject_AsFileDescriptor}. Always returns an {@code int}.
      */
     public abstract static class FileDescriptorConversionNode extends ArgumentCastNodeWithRaise {
-
         @Specialization
         int doFdInt(int value) {
-            return value;
+            return PInt.asFileDescriptor(value, getRaiseNode());
         }
 
         @Specialization(guards = "!isInt(value)", limit = "3")

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

@@ -863,7 +863,7 @@ public int asFileDescriptorWithState(ThreadState state,
         Object result = methodLib.callObjectWithState(filenoFunc, state);
         if (isIntProfile.profileObject(result, PythonBuiltinClassType.PInt)) {
             try {
-                return castToJavaIntNode.execute(result);
+                return PInt.asFileDescriptor(castToJavaIntNode.execute(result), raiseNode);
             } catch (PException e) {
                 e.expect(PythonBuiltinClassType.TypeError, isAttrError);
                 throw raiseNode.raise(PythonBuiltinClassType.OverflowError, ErrorMessages.PYTHON_INT_TOO_LARGE_TO_CONV_TO, "int");

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/ints/PInt.java

@@ -251,11 +251,21 @@ public Object asIndexWithState(@SuppressWarnings("unused") ThreadState threadSta
     public int asFileDescriptorWithState(@SuppressWarnings("unused") ThreadState state,
                     @Exclusive @Cached PRaiseNode raiseNode,
                     @Exclusive @Cached CastToJavaIntExactNode castToJavaIntNode) {
+        int result;
         try {
-            return castToJavaIntNode.execute(this);
+            result = castToJavaIntNode.execute(this);
         } catch (PException e) {
             throw raiseNode.raise(PythonBuiltinClassType.OverflowError, ErrorMessages.PYTHON_INT_TOO_LARGE_TO_CONV_TO, "int");
         }
+        return asFileDescriptor(result, raiseNode);
+    }
+
+    @Ignore
+    public static int asFileDescriptor(int value, PRaiseNode raiseNode) {
+        if (value < 0) {
+            raiseNode.raise(PythonBuiltinClassType.ValueError, ErrorMessages.S_CANNOT_BE_NEGATIVE_INTEGER_D, "file descriptor", value);
+        }
+        return value;
     }
 
     @SuppressWarnings("static-method")

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/object/DefaultPythonIntegerExports.java

@@ -47,6 +47,7 @@
 import com.oracle.graal.python.builtins.objects.function.PArguments.ThreadState;
 import com.oracle.graal.python.builtins.objects.ints.PInt;
 import com.oracle.graal.python.builtins.objects.type.TypeNodes;
+import com.oracle.graal.python.nodes.PRaiseNode;
 import com.oracle.graal.python.nodes.classes.IsSubtypeNode;
 import com.oracle.graal.python.nodes.object.IsBuiltinClassProfile;
 import com.oracle.graal.python.runtime.PythonOptions;
@@ -257,8 +258,9 @@ static String asPStringWithState(Integer receiver, @SuppressWarnings("unused") T
     }
 
     @ExportMessage
-    static int asFileDescriptorWithState(Integer x, @SuppressWarnings("unused") ThreadState state) {
-        return x;
+    static int asFileDescriptorWithState(Integer x, @SuppressWarnings("unused") ThreadState state,
+                    @Cached PRaiseNode raiseNode) {
+        return PInt.asFileDescriptor(x, raiseNode);
     }
 
     @SuppressWarnings("static-method")

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/object/DefaultPythonLongExports.java

@@ -285,7 +285,7 @@ static int asFileDescriptorWithState(Long x, @SuppressWarnings("unused") ThreadS
                     @Exclusive @Cached CastToJavaIntExactNode castToJavaIntNode,
                     @Exclusive @Cached IsBuiltinClassProfile errorProfile) {
         try {
-            return castToJavaIntNode.execute(x);
+            return PInt.asFileDescriptor(castToJavaIntNode.execute(x), raiseNode);
         } catch (PException e) {
             e.expect(PythonBuiltinClassType.TypeError, errorProfile);
             // we need to convert the TypeError to an OverflowError

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

@@ -120,6 +120,7 @@ public abstract class ErrorMessages {
     public static final String CANNOT_ASSIGN_TO_COMPREHENSION = "cannot assign to %s comprehension";
     public static final String CANNOT_BE_INTEPRETED_AS_LONG = "%s cannot be interpreted as long (type %p)";
     public static final String CANNOT_BE_NEGATIVE = "%s cannot be negative";
+    public static final String S_CANNOT_BE_NEGATIVE_INTEGER_D = "%s cannot be negative integer (%d)";
     public static final String CANNOT_CALL_CTOR_OF = "cannot call constructor of %s";
     public static final String CANNOT_CLOSE_EXPORTED_PTRS_EXIST = "cannot close exported pointers exist";
     public static final String CANNOT_CONVERT_DICT_UPDATE_SEQ = "cannot convert dictionary update sequence element #%d to a sequence";