[GR-23097] SignalNode hanles PInt as the signalnum and produces correct errors on invalid arguments.

PullRequest: graalpython/936

Author: steve-s

graalpython/com.oracle.graal.python.test/src/tests/test_signal.py

@@ -1,4 +1,4 @@
-# Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
 # The Universal Permissive License (UPL), Version 1.0
@@ -37,8 +37,21 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 
+import unittest
 import sys
 
+class SignalTests(unittest.TestCase):
+    def test_args_validation(self):
+        try:
+            import _signal
+        except ImportError:
+            import signal as _signal
+        self.assertRaises(OverflowError, lambda : _signal.signal(0x8000000000000000, 0))
+        self.assertRaises(OverflowError, lambda : _signal.signal(0x800000000000000, 0))
+        self.assertRaises(TypeError, lambda : _signal.signal(_signal.SIGALRM, 0x100))
+        self.assertRaises(TypeError, lambda : _signal.signal(_signal.SIGALRM, 'string'))
+
+
 def test_alarm2():
     try:
         import _signal

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

@@ -40,20 +40,18 @@
  */
 package com.oracle.graal.python.builtins.modules;
 
-import static java.lang.StrictMath.toIntExact;
-
 import java.util.List;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentLinkedDeque;
 import java.util.concurrent.Semaphore;
 
 import com.oracle.graal.python.builtins.Builtin;
 import com.oracle.graal.python.builtins.CoreFunctions;
-import com.oracle.graal.python.builtins.PythonBuiltinClassType;
 import com.oracle.graal.python.builtins.PythonBuiltins;
 import com.oracle.graal.python.builtins.objects.PNone;
 import com.oracle.graal.python.builtins.objects.ints.PInt;
 import com.oracle.graal.python.builtins.objects.module.PythonModule;
+import com.oracle.graal.python.builtins.objects.object.PythonObjectLibrary;
 import com.oracle.graal.python.nodes.attributes.ReadAttributeFromObjectNode;
 import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
 import com.oracle.graal.python.nodes.function.PythonBuiltinNode;
@@ -69,6 +67,7 @@
 import com.oracle.truffle.api.dsl.NodeFactory;
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.dsl.TypeSystemReference;
+import com.oracle.truffle.api.library.CachedLibrary;
 import com.oracle.truffle.api.object.HiddenKey;
 
 @CoreFunctions(defineModule = "_signal")
@@ -205,23 +204,25 @@ Object defaultIntHandler(@SuppressWarnings("unused") Object[] args) {
     @Builtin(name = "signal", minNumOfPositionalArgs = 3, declaresExplicitSelf = true)
     @GenerateNodeFactory
     abstract static class SignalNode extends PythonTernaryBuiltinNode {
-        private int getSignum(long signum) {
-            try {
-                return toIntExact(signum);
-            } catch (ArithmeticException ae) {
-                throw raise(PythonBuiltinClassType.OverflowError, "Python int too large to convert to C int");
-            }
+
+        @Specialization(guards = "!idNumLib.isCallable(idNum)", limit = "1")
+        Object signalId(@SuppressWarnings("unused") PythonModule self, Object signal, Object idNum,
+                        @SuppressWarnings("unused") @CachedLibrary("idNum") PythonObjectLibrary idNumLib,
+                        @CachedLibrary("signal") PythonObjectLibrary signalLib) {
+            // Note: CPython checks if id is the same reference as SIG_IGN/SIG_DFL constants, which
+            // are instances of Handlers enum
+            // The -1 fallback will be correctly reported as an error later on
+            int id = idNum instanceof Integer ? (int) idNum : -1;
+            return signal(signalLib.asSize(signal), id);
         }
 
-        @Specialization
         @TruffleBoundary
-        Object signal(@SuppressWarnings("unused") PythonModule self, long signalNumber, int id) {
-            int signum = getSignum(signalNumber);
+        private Object signal(int signum, int id) {
             Object retval;
             try {
                 retval = Signals.setSignalHandler(signum, id);
             } catch (IllegalArgumentException e) {
-                throw raise(PythonErrorType.ValueError, e);
+                throw raise(PythonErrorType.TypeError, "TypeError: signal handler must be signal.SIG_IGN, signal.SIG_DFL, or a callable object");
             }
             if ((int) retval == Signals.SIG_UNKNOWN) {
                 if (signalHandlers.containsKey(signum)) {
@@ -234,12 +235,17 @@ Object signal(@SuppressWarnings("unused") PythonModule self, long signalNumber,
             return retval;
         }
 
-        @Specialization
-        @TruffleBoundary
-        Object signal(PythonModule self, long signalNumber, Object handler,
+        @Specialization(guards = "handlerLib.isCallable(handler)", limit = "1")
+        Object signalHandler(PythonModule self, Object signal, Object handler,
+                        @SuppressWarnings("unused") @CachedLibrary("handler") PythonObjectLibrary handlerLib,
+                        @CachedLibrary("signal") PythonObjectLibrary signalLib,
                         @Cached("create()") ReadAttributeFromObjectNode readQueueNode,
                         @Cached("create()") ReadAttributeFromObjectNode readSemaNode) {
-            int signum = getSignum(signalNumber);
+            return signal(self, signalLib.asSize(signal), handler, readQueueNode, readSemaNode);
+        }
+
+        @TruffleBoundary
+        private Object signal(PythonModule self, int signum, Object handler, ReadAttributeFromObjectNode readQueueNode, ReadAttributeFromObjectNode readSemaNode) {
             ConcurrentLinkedDeque<SignalTriggerAction> queue = getQueue(self, readQueueNode);
             Semaphore semaphore = getSemaphore(self, readSemaNode);
             Object retval;