Add check JNI tests for byte and 16-bit primitives.
Make boolean checking check 32-bit values in case of truncation.
Add unit test for out-of-bound primitive call values.
Make reflection's ArgArray anonymous as per more recent cppstyle and
address other minor style issues.
Test: mma -j32 test-art-host; boot (eng) emulator and check logcat is clean
Bug: 73656264
Change-Id: If7947e24ec3fe3303c1d3d0545605e82f651de25
diff --git a/runtime/check_jni.cc b/runtime/check_jni.cc
index 05f099f..02580cc 100644
--- a/runtime/check_jni.cc
+++ b/runtime/check_jni.cc
@@ -61,6 +61,10 @@
static constexpr uint64_t kCriticalWarnTimeUs = MsToUs(16);
static_assert(kCriticalWarnTimeUs > 0, "No JNI critical warn time set");
+// True if primitives within specific ranges cause a fatal error,
+// otherwise just warn.
+static constexpr bool kBrokenPrimitivesAreFatal = kIsDebugBuild;
+
// Flags passed into ScopedCheck.
static constexpr uint16_t kFlag_Default = 0x0000;
@@ -206,10 +210,12 @@
JniValueType o;
if (type_ == kTypeVaList) {
switch (fmt) {
- case 'Z': o.Z = static_cast<jboolean>(va_arg(vargs_, jint)); break;
- case 'B': o.B = static_cast<jbyte>(va_arg(vargs_, jint)); break;
- case 'C': o.C = static_cast<jchar>(va_arg(vargs_, jint)); break;
- case 'S': o.S = static_cast<jshort>(va_arg(vargs_, jint)); break;
+ // Assign a full int for va_list values as this is what is done in reflection.cc.
+ // TODO(b/73656264): avoid undefined behavior.
+ case 'Z': FALLTHROUGH_INTENDED;
+ case 'B': FALLTHROUGH_INTENDED;
+ case 'C': FALLTHROUGH_INTENDED;
+ case 'S': FALLTHROUGH_INTENDED;
case 'I': o.I = va_arg(vargs_, jint); break;
case 'J': o.J = va_arg(vargs_, jlong); break;
case 'F': o.F = static_cast<jfloat>(va_arg(vargs_, jdouble)); break;
@@ -224,10 +230,14 @@
jvalue v = ptr_[cnt_];
cnt_++;
switch (fmt) {
- case 'Z': o.Z = v.z; break;
- case 'B': o.B = v.b; break;
- case 'C': o.C = v.c; break;
- case 'S': o.S = v.s; break;
+ // Copy just the amount of the jvalue necessary, as done in
+ // reflection.cc, but extend to an int to be consistent with
+ // var args in CheckNonHeapValue.
+ // TODO(b/73656264): avoid undefined behavior.
+ case 'Z': o.I = v.z; break;
+ case 'B': o.I = v.b; break;
+ case 'C': o.I = v.c; break;
+ case 'S': o.I = v.s; break;
case 'I': o.I = v.i; break;
case 'J': o.J = v.j; break;
case 'F': o.F = v.f; break;
@@ -911,17 +921,20 @@
switch (fmt) {
case 'p': // TODO: pointer - null or readable?
case 'v': // JavaVM*
- case 'B': // jbyte
- case 'C': // jchar
case 'D': // jdouble
case 'F': // jfloat
- case 'I': // jint
case 'J': // jlong
- case 'S': // jshort
+ case 'I': // jint
break; // Ignored.
case 'b': // jboolean, why two? Fall-through.
case 'Z':
- return CheckBoolean(arg.Z);
+ return CheckBoolean(arg.I);
+ case 'B': // jbyte
+ return CheckByte(arg.I);
+ case 'C': // jchar
+ return CheckChar(arg.I);
+ case 'S': // jshort
+ return CheckShort(arg.I);
case 'u': // utf8
if ((flags_ & kFlag_Release) != 0) {
return CheckNonNull(arg.u);
@@ -1152,14 +1165,54 @@
return true;
}
- bool CheckBoolean(jboolean z) {
+ bool CheckBoolean(jint z) {
if (z != JNI_TRUE && z != JNI_FALSE) {
+ // Note, broken booleans are always fatal.
AbortF("unexpected jboolean value: %d", z);
return false;
}
return true;
}
+ bool CheckByte(jint b) {
+ if (b < std::numeric_limits<jbyte>::min() ||
+ b > std::numeric_limits<jbyte>::max()) {
+ if (kBrokenPrimitivesAreFatal) {
+ AbortF("unexpected jbyte value: %d", b);
+ return false;
+ } else {
+ LOG(WARNING) << "Unexpected jbyte value: " << b;
+ }
+ }
+ return true;
+ }
+
+ bool CheckShort(jint s) {
+ if (s < std::numeric_limits<jshort>::min() ||
+ s > std::numeric_limits<jshort>::max()) {
+ if (kBrokenPrimitivesAreFatal) {
+ AbortF("unexpected jshort value: %d", s);
+ return false;
+ } else {
+ LOG(WARNING) << "Unexpected jshort value: " << s;
+ }
+ }
+ return true;
+ }
+
+ bool CheckChar(jint c) {
+ if (c < std::numeric_limits<jchar>::min() ||
+ c > std::numeric_limits<jchar>::max()) {
+ if (kBrokenPrimitivesAreFatal) {
+ AbortF("unexpected jchar value: %d", c);
+ return false;
+ } else {
+ LOG(WARNING) << "Unexpected jchar value: " << c;
+ }
+ }
+ return true;
+ }
+
bool CheckLengthPositive(jsize length) {
if (length < 0) {
AbortF("negative jsize: %d", length);
@@ -1813,7 +1866,7 @@
static jobject ToReflectedMethod(JNIEnv* env, jclass cls, jmethodID mid, jboolean isStatic) {
ScopedObjectAccess soa(env);
ScopedCheck sc(kFlag_Default, __FUNCTION__);
- JniValueType args[4] = {{.E = env}, {.c = cls}, {.m = mid}, {.b = isStatic}};
+ JniValueType args[4] = {{.E = env}, {.c = cls}, {.m = mid}, {.I = isStatic}};
if (sc.Check(soa, true, "Ecmb", args)) {
JniValueType result;
result.L = baseEnv(env)->ToReflectedMethod(env, cls, mid, isStatic);
@@ -1828,7 +1881,7 @@
static jobject ToReflectedField(JNIEnv* env, jclass cls, jfieldID fid, jboolean isStatic) {
ScopedObjectAccess soa(env);
ScopedCheck sc(kFlag_Default, __FUNCTION__);
- JniValueType args[4] = {{.E = env}, {.c = cls}, {.f = fid}, {.b = isStatic}};
+ JniValueType args[4] = {{.E = env}, {.c = cls}, {.f = fid}, {.I = isStatic}};
if (sc.Check(soa, true, "Ecfb", args)) {
JniValueType result;
result.L = baseEnv(env)->ToReflectedField(env, cls, fid, isStatic);
@@ -2113,7 +2166,7 @@
return GetFieldIDInternal(__FUNCTION__, env, c, name, sig, true);
}
-#define FIELD_ACCESSORS(jtype, name, ptype, shorty) \
+#define FIELD_ACCESSORS(jtype, name, ptype, shorty, slot_sized_shorty) \
static jtype GetStatic##name##Field(JNIEnv* env, jclass c, jfieldID fid) { \
return GetField(__FUNCTION__, env, c, fid, true, ptype).shorty; \
} \
@@ -2124,25 +2177,25 @@
\
static void SetStatic##name##Field(JNIEnv* env, jclass c, jfieldID fid, jtype v) { \
JniValueType value; \
- value.shorty = v; \
+ value.slot_sized_shorty = v; \
SetField(__FUNCTION__, env, c, fid, true, ptype, value); \
} \
\
static void Set##name##Field(JNIEnv* env, jobject obj, jfieldID fid, jtype v) { \
JniValueType value; \
- value.shorty = v; \
+ value.slot_sized_shorty = v; \
SetField(__FUNCTION__, env, obj, fid, false, ptype, value); \
}
- FIELD_ACCESSORS(jobject, Object, Primitive::kPrimNot, L)
- FIELD_ACCESSORS(jboolean, Boolean, Primitive::kPrimBoolean, Z)
- FIELD_ACCESSORS(jbyte, Byte, Primitive::kPrimByte, B)
- FIELD_ACCESSORS(jchar, Char, Primitive::kPrimChar, C)
- FIELD_ACCESSORS(jshort, Short, Primitive::kPrimShort, S)
- FIELD_ACCESSORS(jint, Int, Primitive::kPrimInt, I)
- FIELD_ACCESSORS(jlong, Long, Primitive::kPrimLong, J)
- FIELD_ACCESSORS(jfloat, Float, Primitive::kPrimFloat, F)
- FIELD_ACCESSORS(jdouble, Double, Primitive::kPrimDouble, D)
+ FIELD_ACCESSORS(jobject, Object, Primitive::kPrimNot, L, L)
+ FIELD_ACCESSORS(jboolean, Boolean, Primitive::kPrimBoolean, Z, I)
+ FIELD_ACCESSORS(jbyte, Byte, Primitive::kPrimByte, B, I)
+ FIELD_ACCESSORS(jchar, Char, Primitive::kPrimChar, C, I)
+ FIELD_ACCESSORS(jshort, Short, Primitive::kPrimShort, S, I)
+ FIELD_ACCESSORS(jint, Int, Primitive::kPrimInt, I, I)
+ FIELD_ACCESSORS(jlong, Long, Primitive::kPrimLong, J, J)
+ FIELD_ACCESSORS(jfloat, Float, Primitive::kPrimFloat, F, F)
+ FIELD_ACCESSORS(jdouble, Double, Primitive::kPrimDouble, D, D)
#undef FIELD_ACCESSORS
static void CallVoidMethodA(JNIEnv* env, jobject obj, jmethodID mid, jvalue* vargs) {