Skip to content

Commit 78fdd8d

Browse files
committed
[GR-25276] Fix: PyLong_AsUnsigned* should raise an error if negative.
PullRequest: graalpython/1161
2 parents 3c44483 + 4550fd7 commit 78fdd8d

File tree

3 files changed

+144
-26
lines changed

3 files changed

+144
-26
lines changed

graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_long.py

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,11 @@ def _reference_aslong(args):
5656
def _reference_as_unsigned_long(args):
5757
# We cannot be sure if we are on 32-bit or 64-bit architecture. So, assume the smaller one.
5858
n = args[0]
59-
if n > 0x7fffffff or n < 0:
59+
if n > 0xffffffff or n < 0:
6060
if sys.version_info.minor >= 6:
61-
raise SystemError
61+
exc = SystemError()
62+
exc.__cause__ = OverflowError()
63+
raise exc
6264
else:
6365
return -1
6466
return int(n)
@@ -171,11 +173,13 @@ def compile_module(self, name):
171173
_reference_as_unsigned_long,
172174
lambda: (
173175
(0,),
174-
# TODO disable because CPython 3.4.1 does not correctly catch exceptions from native
175-
# (-1,),
176+
(-1,),
177+
(-2,),
176178
(0x7fffffff,),
179+
(0xffffffff,),
180+
# we could use larger values on 64-bit systems but how should we know?
177181
),
178-
resultspec="l",
182+
resultspec="k",
179183
argspec='O',
180184
arguments=["PyObject* obj"],
181185
cmpfunc=unhandled_error_compare

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/common/CExtCommonNodes.java

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

43+
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.OverflowError;
4344
import static com.oracle.graal.python.runtime.exception.PythonErrorType.LookupError;
4445
import static com.oracle.graal.python.runtime.exception.PythonErrorType.SystemError;
4546
import static com.oracle.graal.python.runtime.exception.PythonErrorType.TypeError;
@@ -74,7 +75,6 @@
7475
import com.oracle.graal.python.nodes.util.CastToJavaLongLossyNode;
7576
import com.oracle.graal.python.nodes.util.CastToJavaStringNode;
7677
import com.oracle.graal.python.runtime.PythonOptions;
77-
import com.oracle.graal.python.runtime.exception.PythonErrorType;
7878
import com.oracle.graal.python.runtime.object.PythonObjectFactory;
7979
import com.oracle.graal.python.util.OverflowException;
8080
import com.oracle.truffle.api.CompilerAsserts;
@@ -101,6 +101,7 @@
101101
import com.oracle.truffle.api.nodes.Node;
102102
import com.oracle.truffle.api.nodes.UnexpectedResultException;
103103
import com.oracle.truffle.api.profiles.BranchProfile;
104+
import com.oracle.truffle.api.profiles.ConditionProfile;
104105

105106
public abstract class CExtCommonNodes {
106107

@@ -398,27 +399,93 @@ public final int executeInt(Frame frame, Object o, int signed, long targetTypeSi
398399
}
399400

400401
@Specialization(guards = {"targetTypeSize == 4", "signed != 0", "fitsInInt32(nativeWrapper)"})
401-
static long doPrimitiveNativeWrapperToInt32(PrimitiveNativeWrapper nativeWrapper, @SuppressWarnings("unused") int signed, @SuppressWarnings("unused") long targetTypeSize) {
402+
@SuppressWarnings("unused")
403+
static int doWrapperToInt32(PrimitiveNativeWrapper nativeWrapper, int signed, long targetTypeSize) {
402404
return nativeWrapper.getInt();
403405
}
404406

405-
@Specialization(guards = {"targetTypeSize == 8", "fitsInInt64(nativeWrapper)"})
406-
static long doPrimitiveNativeWrapperToInt64(PrimitiveNativeWrapper nativeWrapper, @SuppressWarnings("unused") int signed, @SuppressWarnings("unused") long targetTypeSize) {
407+
@Specialization(guards = {"targetTypeSize == 8", "signed != 0", "fitsInInt64(nativeWrapper)"})
408+
@SuppressWarnings("unused")
409+
static long doWrapperToInt64(PrimitiveNativeWrapper nativeWrapper, int signed, long targetTypeSize) {
407410
return nativeWrapper.getLong();
408411
}
409412

410-
@Specialization(guards = "targetTypeSize == 4")
411-
static long doInt4(int obj, @SuppressWarnings("unused") int signed, @SuppressWarnings("unused") long targetTypeSize) {
412-
return obj;
413+
@Specialization(guards = {"targetTypeSize == 4", "signed == 0", "fitsInUInt32(nativeWrapper)"})
414+
@SuppressWarnings("unused")
415+
static int doWrapperToUInt32Pos(PrimitiveNativeWrapper nativeWrapper, int signed, long targetTypeSize) {
416+
return nativeWrapper.getInt();
413417
}
414418

415-
@Specialization(guards = "targetTypeSize == 8")
416-
static long doInt8(int obj, int signed, @SuppressWarnings("unused") long targetTypeSize) {
417-
if (signed != 0) {
418-
return obj;
419-
} else {
420-
return obj & 0xFFFFFFFFL;
419+
@Specialization(guards = {"targetTypeSize == 4", "signed == 0", "fitsInInt32(nativeWrapper)"}, replaces = "doWrapperToUInt32Pos")
420+
@SuppressWarnings("unused")
421+
static int doWrapperToUInt32(Frame frame, PrimitiveNativeWrapper nativeWrapper, int signed, long targetTypeSize,
422+
@Shared("raiseNativeNode") @Cached PRaiseNativeNode raiseNativeNode,
423+
@Cached ConditionProfile negativeProfile) {
424+
return doIntToUInt32(frame, nativeWrapper.getInt(), signed, targetTypeSize, raiseNativeNode, negativeProfile);
425+
}
426+
427+
@Specialization(guards = {"targetTypeSize == 8", "signed == 0", "fitsInUInt64(nativeWrapper)"})
428+
@SuppressWarnings("unused")
429+
static long doWrapperToUInt64Pos(PrimitiveNativeWrapper nativeWrapper, int signed, long targetTypeSize) {
430+
return nativeWrapper.getLong();
431+
}
432+
433+
@Specialization(guards = {"targetTypeSize == 8", "signed == 0", "fitsInInt64(nativeWrapper)"}, replaces = "doWrapperToUInt64Pos")
434+
@SuppressWarnings("unused")
435+
static long doWrapperToUInt64(Frame frame, PrimitiveNativeWrapper nativeWrapper, int signed, long targetTypeSize,
436+
@Shared("raiseNativeNode") @Cached PRaiseNativeNode raiseNativeNode,
437+
@Cached ConditionProfile negativeProfile) {
438+
long value = nativeWrapper.getLong();
439+
if (negativeProfile.profile(value < 0)) {
440+
return raiseNegativeValue(frame, raiseNativeNode);
441+
}
442+
return value;
443+
}
444+
445+
@Specialization(guards = {"targetTypeSize == 4", "signed != 0"})
446+
@SuppressWarnings("unused")
447+
static long doIntToInt32(int value, int signed, long targetTypeSize) {
448+
return value;
449+
}
450+
451+
@Specialization(guards = {"targetTypeSize == 4", "signed == 0", "value >= 0"})
452+
@SuppressWarnings("unused")
453+
static long doIntToUInt32Pos(int value, int signed, long targetTypeSize) {
454+
return value;
455+
}
456+
457+
@Specialization(guards = {"targetTypeSize == 4", "signed == 0"}, replaces = "doIntToUInt32Pos")
458+
@SuppressWarnings("unused")
459+
static int doIntToUInt32(Frame frame, int value, int signed, long targetTypeSize,
460+
@Shared("raiseNativeNode") @Cached PRaiseNativeNode raiseNativeNode,
461+
@Cached ConditionProfile negativeProfile) {
462+
if (negativeProfile.profile(value < 0)) {
463+
return raiseNegativeValue(frame, raiseNativeNode);
464+
}
465+
return value;
466+
}
467+
468+
@Specialization(guards = {"targetTypeSize == 8", "signed != 0"})
469+
@SuppressWarnings("unused")
470+
static long doIntToInt64(int value, int signed, long targetTypeSize) {
471+
return value;
472+
}
473+
474+
@Specialization(guards = {"targetTypeSize == 8", "signed == 0", "value >= 0"})
475+
@SuppressWarnings("unused")
476+
static long doIntToUInt64Pos(int value, int signed, long targetTypeSize) {
477+
return value;
478+
}
479+
480+
@Specialization(guards = {"targetTypeSize == 8", "signed == 0"}, replaces = "doIntToUInt64Pos")
481+
@SuppressWarnings("unused")
482+
static int doIntToUInt64(Frame frame, int value, int signed, long targetTypeSize,
483+
@Shared("raiseNativeNode") @Cached PRaiseNativeNode raiseNativeNode,
484+
@Cached ConditionProfile negativeProfile) {
485+
if (negativeProfile.profile(value < 0)) {
486+
return raiseNegativeValue(frame, raiseNativeNode);
421487
}
488+
return value;
422489
}
423490

424491
@Specialization(guards = {"targetTypeSize != 4", "targetTypeSize != 8"})
@@ -433,9 +500,27 @@ static long doLong4(Frame frame, @SuppressWarnings("unused") long obj, @Suppress
433500
return raiseTooLarge(frame, raiseNativeNode, targetTypeSize);
434501
}
435502

436-
@Specialization(guards = "targetTypeSize == 8")
437-
static long doLong8(long obj, @SuppressWarnings("unused") int signed, @SuppressWarnings("unused") long targetTypeSize) {
438-
return obj;
503+
@Specialization(guards = {"targetTypeSize == 8", "signed != 0"})
504+
@SuppressWarnings("unused")
505+
static long doLongToInt64(int value, int signed, long targetTypeSize) {
506+
return value;
507+
}
508+
509+
@Specialization(guards = {"targetTypeSize == 8", "signed == 0", "value >= 0"})
510+
@SuppressWarnings("unused")
511+
static long doLongToUInt64Pos(int value, int signed, long targetTypeSize) {
512+
return value;
513+
}
514+
515+
@Specialization(guards = {"targetTypeSize == 8", "signed == 0"}, replaces = "doLongToUInt64Pos")
516+
@SuppressWarnings("unused")
517+
static int doLongToUInt64(Frame frame, int value, int signed, long targetTypeSize,
518+
@Shared("raiseNativeNode") @Cached PRaiseNativeNode raiseNativeNode,
519+
@Cached ConditionProfile negativeProfile) {
520+
if (negativeProfile.profile(value < 0)) {
521+
return raiseNegativeValue(frame, raiseNativeNode);
522+
}
523+
return value;
439524
}
440525

441526
@Specialization(guards = "targetTypeSize == 8")
@@ -450,33 +535,41 @@ static long doPInt(Frame frame, @SuppressWarnings("unused") long obj, @SuppressW
450535
}
451536

452537
@Specialization(guards = "targetTypeSize == 4")
453-
static long doPInt4(Frame frame, PInt obj, int signed, @SuppressWarnings("unused") long targetTypeSize,
538+
@TruffleBoundary
539+
static int doPIntTo32Bit(PInt obj, int signed, @SuppressWarnings("unused") long targetTypeSize,
454540
@Shared("raiseNativeNode") @Cached PRaiseNativeNode raiseNativeNode) {
455541
try {
456542
if (signed != 0) {
457543
return obj.intValueExact();
458544
} else if (obj.bitCount() <= 32) {
545+
if (obj.isNegative()) {
546+
return raiseNegativeValue(raiseNativeNode);
547+
}
459548
return obj.intValue();
460549
}
461550
} catch (ArithmeticException e) {
462551
// fall through
463552
}
464-
return raiseTooLarge(frame, raiseNativeNode, targetTypeSize);
553+
return raiseTooLarge(raiseNativeNode, targetTypeSize);
465554
}
466555

467556
@Specialization(guards = "targetTypeSize == 8")
468-
static long doPInt8(Frame frame, PInt obj, int signed, @SuppressWarnings("unused") long targetTypeSize,
557+
@TruffleBoundary
558+
static long doPIntTo64Bit(PInt obj, int signed, @SuppressWarnings("unused") long targetTypeSize,
469559
@Shared("raiseNativeNode") @Cached PRaiseNativeNode raiseNativeNode) {
470560
try {
471561
if (signed != 0) {
472562
return obj.longValueExact();
473563
} else if (obj.bitCount() <= 64) {
564+
if (obj.isNegative()) {
565+
return raiseNegativeValue(raiseNativeNode);
566+
}
474567
return obj.longValue();
475568
}
476569
} catch (ArithmeticException e) {
477570
// fall through
478571
}
479-
return raiseTooLarge(frame, raiseNativeNode, targetTypeSize);
572+
return raiseTooLarge(raiseNativeNode, targetTypeSize);
480573
}
481574

482575
@Specialization(guards = {"targetTypeSize != 4", "targetTypeSize != 8"})
@@ -491,20 +584,40 @@ static Object doGeneric(Object obj, int signed, long targetTypeSize,
491584
return asNativePrimitiveNode.execute(obj, signed, (int) targetTypeSize, true);
492585
}
493586

587+
private static int raiseTooLarge(PRaiseNativeNode raiseNativeNode, long targetTypeSize) {
588+
return raiseNativeNode.raiseIntWithoutFrame(-1, OverflowError, ErrorMessages.PYTHON_INT_TOO_LARGE_TO_CONV_TO_C_TYPE, targetTypeSize);
589+
}
590+
494591
private static int raiseTooLarge(Frame frame, PRaiseNativeNode raiseNativeNode, long targetTypeSize) {
495-
return raiseNativeNode.raiseInt(frame, -1, PythonErrorType.OverflowError, ErrorMessages.PYTHON_INT_TOO_LARGE_TO_CONV_TO_C_TYPE, targetTypeSize);
592+
return raiseNativeNode.raiseInt(frame, -1, OverflowError, ErrorMessages.PYTHON_INT_TOO_LARGE_TO_CONV_TO_C_TYPE, targetTypeSize);
496593
}
497594

498595
private static Integer raiseUnsupportedSize(Frame frame, PRaiseNativeNode raiseNativeNode, long targetTypeSize) {
499596
return raiseNativeNode.raiseInt(frame, -1, SystemError, ErrorMessages.UNSUPPORTED_TARGET_SIZE, targetTypeSize);
500597
}
501598

599+
private static int raiseNegativeValue(PRaiseNativeNode raiseNativeNode) {
600+
return raiseNativeNode.raiseIntWithoutFrame(-1, OverflowError, ErrorMessages.CANNOT_CONVERT_NEGATIVE_VALUE_TO_UNSIGNED_INT);
601+
}
602+
603+
private static int raiseNegativeValue(Frame frame, PRaiseNativeNode raiseNativeNode) {
604+
return raiseNativeNode.raiseInt(frame, -1, OverflowError, ErrorMessages.CANNOT_CONVERT_NEGATIVE_VALUE_TO_UNSIGNED_INT);
605+
}
606+
502607
static boolean fitsInInt32(PrimitiveNativeWrapper nativeWrapper) {
503608
return nativeWrapper.isBool() || nativeWrapper.isByte() || nativeWrapper.isInt();
504609
}
505610

506611
static boolean fitsInInt64(PrimitiveNativeWrapper nativeWrapper) {
507612
return nativeWrapper.isIntLike() || nativeWrapper.isBool();
508613
}
614+
615+
static boolean fitsInUInt32(PrimitiveNativeWrapper nativeWrapper) {
616+
return (nativeWrapper.isBool() || nativeWrapper.isByte() || nativeWrapper.isInt()) && nativeWrapper.getInt() >= 0;
617+
}
618+
619+
static boolean fitsInUInt64(PrimitiveNativeWrapper nativeWrapper) {
620+
return (nativeWrapper.isIntLike() || nativeWrapper.isBool()) && nativeWrapper.getLong() >= 0;
621+
}
509622
}
510623
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -544,4 +544,5 @@ public abstract class ErrorMessages {
544544
public static final String CAPI_LOAD_ERROR = "Could not load C API from %s.";
545545
public static final String NATIVE_ACCESS_NOT_ALLOWED = "Cannot run any C extensions because native access is not allowed.";
546546
public static final String HPY_LOAD_ERROR = "Could not load HPy C API from %s.";
547+
public static final String CANNOT_CONVERT_NEGATIVE_VALUE_TO_UNSIGNED_INT = "can't convert negative value to unsigned int";
547548
}

0 commit comments

Comments
 (0)