From 76433275dbe39e5ced1c223b006d1b900b1937f6 Mon Sep 17 00:00:00 2001 From: Mathieu Chartier Date: Fri, 26 Sep 2014 14:32:37 -0700 Subject: [PATCH] Optimize get/set reflection performance Speedups mostly from reducing how often access checks are needed, and adding more inlining, and adding templates. Field_getInt from ~850ns -> 350ns. Field_setInt from ~900ns -> 370ns. Bug: 14063288 (cherry picked from commit ffc788cb7b5b9f53734d7bb8af2d5e45d885546b) Change-Id: I2441581ff3478c6ae43b6aa49939ff3f07555ec8 --- .../portable/portable_thread_entrypoints.cc | 1 + runtime/entrypoints/quick/callee_save_frame.h | 2 + runtime/entrypoints/quick/quick_jni_entrypoints.cc | 1 + runtime/mirror/art_field-inl.h | 11 ++ runtime/mirror/art_field.cc | 8 -- runtime/mirror/art_field.h | 4 +- runtime/native/java_lang_reflect_Field.cc | 115 +++++++++++---------- runtime/reflection-inl.h | 106 +++++++++++++++++++ runtime/reflection.cc | 95 ++--------------- runtime/reflection.h | 9 +- runtime/thread-inl.h | 1 - runtime/utils.cc | 8 +- runtime/utils_test.cc | 12 +++ 13 files changed, 217 insertions(+), 156 deletions(-) create mode 100644 runtime/reflection-inl.h diff --git a/runtime/entrypoints/portable/portable_thread_entrypoints.cc b/runtime/entrypoints/portable/portable_thread_entrypoints.cc index 7d5ccc225..ecbc65ee5 100644 --- a/runtime/entrypoints/portable/portable_thread_entrypoints.cc +++ b/runtime/entrypoints/portable/portable_thread_entrypoints.cc @@ -14,6 +14,7 @@ * limitations under the License. */ +#include "mirror/art_method-inl.h" #include "verifier/dex_gc_map.h" #include "stack.h" #include "thread-inl.h" diff --git a/runtime/entrypoints/quick/callee_save_frame.h b/runtime/entrypoints/quick/callee_save_frame.h index e573d6da8..e728f7dd1 100644 --- a/runtime/entrypoints/quick/callee_save_frame.h +++ b/runtime/entrypoints/quick/callee_save_frame.h @@ -18,7 +18,9 @@ #define ART_RUNTIME_ENTRYPOINTS_QUICK_CALLEE_SAVE_FRAME_H_ #include "base/mutex.h" +#include "gc_root-inl.h" #include "instruction_set.h" +#include "runtime-inl.h" #include "thread-inl.h" // Specific frame size code is in architecture-specific files. We include this to compile-time diff --git a/runtime/entrypoints/quick/quick_jni_entrypoints.cc b/runtime/entrypoints/quick/quick_jni_entrypoints.cc index 87f04bbbf..c2395352b 100644 --- a/runtime/entrypoints/quick/quick_jni_entrypoints.cc +++ b/runtime/entrypoints/quick/quick_jni_entrypoints.cc @@ -14,6 +14,7 @@ * limitations under the License. */ +#include "entrypoints/entrypoint_utils-inl.h" #include "mirror/object-inl.h" #include "thread-inl.h" #include "verify_object-inl.h" diff --git a/runtime/mirror/art_field-inl.h b/runtime/mirror/art_field-inl.h index d37fa41d3..03425cc30 100644 --- a/runtime/mirror/art_field-inl.h +++ b/runtime/mirror/art_field-inl.h @@ -25,6 +25,9 @@ #include "jvalue.h" #include "object-inl.h" #include "primitive.h" +#include "thread-inl.h" +#include "scoped_thread_state_change.h" +#include "well_known_classes.h" namespace art { namespace mirror { @@ -298,6 +301,14 @@ inline const DexFile* ArtField::GetDexFile() SHARED_LOCKS_REQUIRED(Locks::mutato return GetDexCache()->GetDexFile(); } +inline ArtField* ArtField::FromReflectedField(const ScopedObjectAccessAlreadyRunnable& soa, + jobject jlr_field) { + mirror::ArtField* f = soa.DecodeField(WellKnownClasses::java_lang_reflect_Field_artField); + mirror::ArtField* field = f->GetObject(soa.Decode(jlr_field))->AsArtField(); + DCHECK(field != nullptr); + return field; +} + } // namespace mirror } // namespace art diff --git a/runtime/mirror/art_field.cc b/runtime/mirror/art_field.cc index 3c7c6ce39..7e2007611 100644 --- a/runtime/mirror/art_field.cc +++ b/runtime/mirror/art_field.cc @@ -31,14 +31,6 @@ namespace mirror { // TODO: Get global references for these GcRoot ArtField::java_lang_reflect_ArtField_; -ArtField* ArtField::FromReflectedField(const ScopedObjectAccessAlreadyRunnable& soa, - jobject jlr_field) { - mirror::ArtField* f = soa.DecodeField(WellKnownClasses::java_lang_reflect_Field_artField); - mirror::ArtField* field = f->GetObject(soa.Decode(jlr_field))->AsArtField(); - DCHECK(field != nullptr); - return field; -} - void ArtField::SetClass(Class* java_lang_reflect_ArtField) { CHECK(java_lang_reflect_ArtField_.IsNull()); CHECK(java_lang_reflect_ArtField != NULL); diff --git a/runtime/mirror/art_field.h b/runtime/mirror/art_field.h index 885bcb06a..50299b670 100644 --- a/runtime/mirror/art_field.h +++ b/runtime/mirror/art_field.h @@ -47,8 +47,8 @@ class MANAGED ArtField FINAL : public Object { return sizeof(ArtField); } - static ArtField* FromReflectedField(const ScopedObjectAccessAlreadyRunnable& soa, - jobject jlr_field) + ALWAYS_INLINE static ArtField* FromReflectedField(const ScopedObjectAccessAlreadyRunnable& soa, + jobject jlr_field) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); Class* GetDeclaringClass() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); diff --git a/runtime/native/java_lang_reflect_Field.cc b/runtime/native/java_lang_reflect_Field.cc index ad88109a8..14c6d3879 100644 --- a/runtime/native/java_lang_reflect_Field.cc +++ b/runtime/native/java_lang_reflect_Field.cc @@ -23,19 +23,21 @@ #include "mirror/art_field-inl.h" #include "mirror/art_method-inl.h" #include "mirror/class-inl.h" -#include "reflection.h" +#include "reflection-inl.h" #include "scoped_fast_native_object_access.h" namespace art { -static bool VerifyFieldAccess(mirror::ArtField* field, mirror::Object* obj, bool is_set) +template +ALWAYS_INLINE inline static bool VerifyFieldAccess(Thread* self, mirror::ArtField* field, + mirror::Object* obj) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { - if (field->IsFinal() && is_set) { + if (kIsSet && field->IsFinal()) { ThrowIllegalAccessException(nullptr, StringPrintf("Cannot set final field: %s", PrettyField(field).c_str()).c_str()); return false; } - if (!VerifyAccess(obj, field->GetDeclaringClass(), field->GetAccessFlags())) { + if (!VerifyAccess(self, obj, field->GetDeclaringClass(), field->GetAccessFlags())) { ThrowIllegalAccessException(nullptr, StringPrintf("Cannot access field: %s", PrettyField(field).c_str()).c_str()); return false; @@ -43,9 +45,10 @@ static bool VerifyFieldAccess(mirror::ArtField* field, mirror::Object* obj, bool return true; } -static bool GetFieldValue(const ScopedFastNativeObjectAccess& soa, mirror::Object* o, - mirror::ArtField* f, Primitive::Type field_type, bool allow_references, - JValue* value) +template +ALWAYS_INLINE inline static bool GetFieldValue( + const ScopedFastNativeObjectAccess& soa, mirror::Object* o, mirror::ArtField* f, + Primitive::Type field_type, JValue* value) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { DCHECK_EQ(value->GetJ(), INT64_C(0)); switch (field_type) { @@ -74,7 +77,7 @@ static bool GetFieldValue(const ScopedFastNativeObjectAccess& soa, mirror::Objec value->SetS(f->GetShort(o)); return true; case Primitive::kPrimNot: - if (allow_references) { + if (kAllowReferences) { value->SetL(f->GetObject(o)); return true; } @@ -89,29 +92,29 @@ static bool GetFieldValue(const ScopedFastNativeObjectAccess& soa, mirror::Objec return false; } -static bool CheckReceiver(const ScopedFastNativeObjectAccess& soa, jobject j_rcvr, - mirror::ArtField** f, mirror::Object** class_or_rcvr) +ALWAYS_INLINE inline static bool CheckReceiver(const ScopedFastNativeObjectAccess& soa, + jobject j_rcvr, mirror::ArtField** f, + mirror::Object** class_or_rcvr) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { soa.Self()->AssertThreadSuspensionIsAllowable(); + mirror::Class* declaringClass = (*f)->GetDeclaringClass(); if ((*f)->IsStatic()) { - StackHandleScope<2> hs(soa.Self()); - HandleWrapper h_f(hs.NewHandleWrapper(f)); - Handle h_klass(hs.NewHandle((*f)->GetDeclaringClass())); - if (UNLIKELY(!Runtime::Current()->GetClassLinker()->EnsureInitialized(soa.Self(), h_klass, true, - true))) { - DCHECK(soa.Self()->IsExceptionPending()); - *class_or_rcvr = nullptr; - return false; + if (UNLIKELY(!declaringClass->IsInitialized())) { + ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); + StackHandleScope<2> hs(soa.Self()); + HandleWrapper h_f(hs.NewHandleWrapper(f)); + HandleWrapper h_klass(hs.NewHandleWrapper(&declaringClass)); + if (UNLIKELY(!class_linker->EnsureInitialized(soa.Self(), h_klass, true, true))) { + DCHECK(soa.Self()->IsExceptionPending()); + return false; + } } - *class_or_rcvr = h_klass.Get(); + *class_or_rcvr = declaringClass; return true; } - *class_or_rcvr = soa.Decode(j_rcvr); - mirror::Class* declaringClass = (*f)->GetDeclaringClass(); if (!VerifyObjectIsClass(*class_or_rcvr, declaringClass)) { DCHECK(soa.Self()->IsExceptionPending()); - *class_or_rcvr = nullptr; return false; } return true; @@ -126,7 +129,7 @@ static jobject Field_get(JNIEnv* env, jobject javaField, jobject javaObj, jboole return nullptr; } // If field is not set to be accessible, verify it can be accessed by the caller. - if ((accessible == JNI_FALSE) && !VerifyFieldAccess(f, o, false)) { + if ((accessible == JNI_FALSE) && !VerifyFieldAccess(soa.Self(), f, o)) { DCHECK(soa.Self()->IsExceptionPending()); return nullptr; } @@ -134,15 +137,16 @@ static jobject Field_get(JNIEnv* env, jobject javaField, jobject javaObj, jboole // Get the field's value, boxing if necessary. Primitive::Type field_type = f->GetTypeAsPrimitiveType(); JValue value; - if (!GetFieldValue(soa, o, f, field_type, true, &value)) { + if (!GetFieldValue(soa, o, f, field_type, &value)) { DCHECK(soa.Self()->IsExceptionPending()); return nullptr; } return soa.AddLocalReference(BoxPrimitive(field_type, value)); } -static JValue GetPrimitiveField(JNIEnv* env, jobject javaField, jobject javaObj, - char dst_descriptor, jboolean accessible) { +template +ALWAYS_INLINE inline static JValue GetPrimitiveField(JNIEnv* env, jobject javaField, + jobject javaObj, jboolean accessible) { ScopedFastNativeObjectAccess soa(env); mirror::ArtField* f = mirror::ArtField::FromReflectedField(soa, javaField); mirror::Object* o = nullptr; @@ -152,7 +156,7 @@ static JValue GetPrimitiveField(JNIEnv* env, jobject javaField, jobject javaObj, } // If field is not set to be accessible, verify it can be accessed by the caller. - if ((accessible == JNI_FALSE) && !VerifyFieldAccess(f, o, false)) { + if (accessible == JNI_FALSE && !VerifyFieldAccess(soa.Self(), f, o)) { DCHECK(soa.Self()->IsExceptionPending()); return JValue(); } @@ -161,15 +165,22 @@ static JValue GetPrimitiveField(JNIEnv* env, jobject javaField, jobject javaObj, // Read the value. Primitive::Type field_type = f->GetTypeAsPrimitiveType(); JValue field_value; - if (!GetFieldValue(soa, o, f, field_type, false, &field_value)) { + if (field_type == kPrimitiveType) { + // This if statement should get optimized out since we only pass in valid primitive types. + if (UNLIKELY(!GetFieldValue(soa, o, f, kPrimitiveType, &field_value))) { + DCHECK(soa.Self()->IsExceptionPending()); + return JValue(); + } + return field_value; + } + if (!GetFieldValue(soa, o, f, field_type, &field_value)) { DCHECK(soa.Self()->IsExceptionPending()); return JValue(); } - // Widen it if necessary (and possible). JValue wide_value; - if (!ConvertPrimitiveValue(NULL, false, field_type, Primitive::GetType(dst_descriptor), - field_value, &wide_value)) { + if (!ConvertPrimitiveValue(nullptr, false, field_type, kPrimitiveType, field_value, + &wide_value)) { DCHECK(soa.Self()->IsExceptionPending()); return JValue(); } @@ -178,36 +189,36 @@ static JValue GetPrimitiveField(JNIEnv* env, jobject javaField, jobject javaObj, static jboolean Field_getBoolean(JNIEnv* env, jobject javaField, jobject javaObj, jboolean accessible) { - return GetPrimitiveField(env, javaField, javaObj, 'Z', accessible).GetZ(); + return GetPrimitiveField(env, javaField, javaObj, accessible).GetZ(); } static jbyte Field_getByte(JNIEnv* env, jobject javaField, jobject javaObj, jboolean accessible) { - return GetPrimitiveField(env, javaField, javaObj, 'B', accessible).GetB(); + return GetPrimitiveField(env, javaField, javaObj, accessible).GetB(); } static jchar Field_getChar(JNIEnv* env, jobject javaField, jobject javaObj, jboolean accessible) { - return GetPrimitiveField(env, javaField, javaObj, 'C', accessible).GetC(); + return GetPrimitiveField(env, javaField, javaObj, accessible).GetC(); } static jdouble Field_getDouble(JNIEnv* env, jobject javaField, jobject javaObj, jboolean accessible) { - return GetPrimitiveField(env, javaField, javaObj, 'D', accessible).GetD(); + return GetPrimitiveField(env, javaField, javaObj, accessible).GetD(); } static jfloat Field_getFloat(JNIEnv* env, jobject javaField, jobject javaObj, jboolean accessible) { - return GetPrimitiveField(env, javaField, javaObj, 'F', accessible).GetF(); + return GetPrimitiveField(env, javaField, javaObj, accessible).GetF(); } static jint Field_getInt(JNIEnv* env, jobject javaField, jobject javaObj, jboolean accessible) { - return GetPrimitiveField(env, javaField, javaObj, 'I', accessible).GetI(); + return GetPrimitiveField(env, javaField, javaObj, accessible).GetI(); } static jlong Field_getLong(JNIEnv* env, jobject javaField, jobject javaObj, jboolean accessible) { - return GetPrimitiveField(env, javaField, javaObj, 'J', accessible).GetJ(); + return GetPrimitiveField(env, javaField, javaObj, accessible).GetJ(); } static jshort Field_getShort(JNIEnv* env, jobject javaField, jobject javaObj, jboolean accessible) { - return GetPrimitiveField(env, javaField, javaObj, 'S', accessible).GetS(); + return GetPrimitiveField(env, javaField, javaObj, accessible).GetS(); } static void SetFieldValue(ScopedFastNativeObjectAccess& soa, mirror::Object* o, @@ -290,14 +301,15 @@ static void Field_set(JNIEnv* env, jobject javaField, jobject javaObj, jobject j return; } // If field is not set to be accessible, verify it can be accessed by the caller. - if ((accessible == JNI_FALSE) && !VerifyFieldAccess(f, o, true)) { + if ((accessible == JNI_FALSE) && !VerifyFieldAccess(soa.Self(), f, o)) { DCHECK(soa.Self()->IsExceptionPending()); return; } SetFieldValue(soa, o, f, field_prim_type, true, unboxed_value); } -static void SetPrimitiveField(JNIEnv* env, jobject javaField, jobject javaObj, char src_descriptor, +template +static void SetPrimitiveField(JNIEnv* env, jobject javaField, jobject javaObj, const JValue& new_value, jboolean accessible) { ScopedFastNativeObjectAccess soa(env); mirror::ArtField* f = mirror::ArtField::FromReflectedField(soa, javaField); @@ -314,14 +326,13 @@ static void SetPrimitiveField(JNIEnv* env, jobject javaField, jobject javaObj, c // Widen the value if necessary (and possible). JValue wide_value; - if (!ConvertPrimitiveValue(nullptr, false, Primitive::GetType(src_descriptor), - field_type, new_value, &wide_value)) { + if (!ConvertPrimitiveValue(nullptr, false, kPrimitiveType, field_type, new_value, &wide_value)) { DCHECK(soa.Self()->IsExceptionPending()); return; } // If field is not set to be accessible, verify it can be accessed by the caller. - if ((accessible == JNI_FALSE) && !VerifyFieldAccess(f, o, true)) { + if ((accessible == JNI_FALSE) && !VerifyFieldAccess(soa.Self(), f, o)) { DCHECK(soa.Self()->IsExceptionPending()); return; } @@ -334,56 +345,56 @@ static void Field_setBoolean(JNIEnv* env, jobject javaField, jobject javaObj, jb jboolean accessible) { JValue value; value.SetZ(z); - SetPrimitiveField(env, javaField, javaObj, 'Z', value, accessible); + SetPrimitiveField(env, javaField, javaObj, value, accessible); } static void Field_setByte(JNIEnv* env, jobject javaField, jobject javaObj, jbyte b, jboolean accessible) { JValue value; value.SetB(b); - SetPrimitiveField(env, javaField, javaObj, 'B', value, accessible); + SetPrimitiveField(env, javaField, javaObj, value, accessible); } static void Field_setChar(JNIEnv* env, jobject javaField, jobject javaObj, jchar c, jboolean accessible) { JValue value; value.SetC(c); - SetPrimitiveField(env, javaField, javaObj, 'C', value, accessible); + SetPrimitiveField(env, javaField, javaObj, value, accessible); } static void Field_setDouble(JNIEnv* env, jobject javaField, jobject javaObj, jdouble d, jboolean accessible) { JValue value; value.SetD(d); - SetPrimitiveField(env, javaField, javaObj, 'D', value, accessible); + SetPrimitiveField(env, javaField, javaObj, value, accessible); } static void Field_setFloat(JNIEnv* env, jobject javaField, jobject javaObj, jfloat f, jboolean accessible) { JValue value; value.SetF(f); - SetPrimitiveField(env, javaField, javaObj, 'F', value, accessible); + SetPrimitiveField(env, javaField, javaObj, value, accessible); } static void Field_setInt(JNIEnv* env, jobject javaField, jobject javaObj, jint i, jboolean accessible) { JValue value; value.SetI(i); - SetPrimitiveField(env, javaField, javaObj, 'I', value, accessible); + SetPrimitiveField(env, javaField, javaObj, value, accessible); } static void Field_setLong(JNIEnv* env, jobject javaField, jobject javaObj, jlong j, jboolean accessible) { JValue value; value.SetJ(j); - SetPrimitiveField(env, javaField, javaObj, 'J', value, accessible); + SetPrimitiveField(env, javaField, javaObj, value, accessible); } static void Field_setShort(JNIEnv* env, jobject javaField, jobject javaObj, jshort s, jboolean accessible) { JValue value; value.SetS(s); - SetPrimitiveField(env, javaField, javaObj, 'S', value, accessible); + SetPrimitiveField(env, javaField, javaObj, value, accessible); } static JNINativeMethod gMethods[] = { diff --git a/runtime/reflection-inl.h b/runtime/reflection-inl.h new file mode 100644 index 000000000..be4d5603f --- /dev/null +++ b/runtime/reflection-inl.h @@ -0,0 +1,106 @@ +/* + * Copyright (C) 2014 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ART_RUNTIME_REFLECTION_INL_H_ +#define ART_RUNTIME_REFLECTION_INL_H_ + +#include "reflection.h" + +#include "base/stringprintf.h" +#include "common_throws.h" +#include "jvalue.h" +#include "primitive.h" +#include "utils.h" + +namespace art { + +inline bool ConvertPrimitiveValue(const ThrowLocation* throw_location, bool unbox_for_result, + Primitive::Type srcType, Primitive::Type dstType, + const JValue& src, JValue* dst) { + DCHECK(srcType != Primitive::kPrimNot && dstType != Primitive::kPrimNot); + if (LIKELY(srcType == dstType)) { + dst->SetJ(src.GetJ()); + return true; + } + switch (dstType) { + case Primitive::kPrimBoolean: // Fall-through. + case Primitive::kPrimChar: // Fall-through. + case Primitive::kPrimByte: + // Only expect assignment with source and destination of identical type. + break; + case Primitive::kPrimShort: + if (srcType == Primitive::kPrimByte) { + dst->SetS(src.GetI()); + return true; + } + break; + case Primitive::kPrimInt: + if (srcType == Primitive::kPrimByte || srcType == Primitive::kPrimChar || + srcType == Primitive::kPrimShort) { + dst->SetI(src.GetI()); + return true; + } + break; + case Primitive::kPrimLong: + if (srcType == Primitive::kPrimByte || srcType == Primitive::kPrimChar || + srcType == Primitive::kPrimShort || srcType == Primitive::kPrimInt) { + dst->SetJ(src.GetI()); + return true; + } + break; + case Primitive::kPrimFloat: + if (srcType == Primitive::kPrimByte || srcType == Primitive::kPrimChar || + srcType == Primitive::kPrimShort || srcType == Primitive::kPrimInt) { + dst->SetF(src.GetI()); + return true; + } else if (srcType == Primitive::kPrimLong) { + dst->SetF(src.GetJ()); + return true; + } + break; + case Primitive::kPrimDouble: + if (srcType == Primitive::kPrimByte || srcType == Primitive::kPrimChar || + srcType == Primitive::kPrimShort || srcType == Primitive::kPrimInt) { + dst->SetD(src.GetI()); + return true; + } else if (srcType == Primitive::kPrimLong) { + dst->SetD(src.GetJ()); + return true; + } else if (srcType == Primitive::kPrimFloat) { + dst->SetD(src.GetF()); + return true; + } + break; + default: + break; + } + if (!unbox_for_result) { + ThrowIllegalArgumentException(throw_location, + StringPrintf("Invalid primitive conversion from %s to %s", + PrettyDescriptor(srcType).c_str(), + PrettyDescriptor(dstType).c_str()).c_str()); + } else { + ThrowClassCastException(throw_location, + StringPrintf("Couldn't convert result of type %s to %s", + PrettyDescriptor(srcType).c_str(), + PrettyDescriptor(dstType).c_str()).c_str()); + } + return false; +} + +} // namespace art + +#endif // ART_RUNTIME_REFLECTION_INL_H_ diff --git a/runtime/reflection.cc b/runtime/reflection.cc index 9fe296a6b..0705d402b 100644 --- a/runtime/reflection.cc +++ b/runtime/reflection.cc @@ -14,7 +14,7 @@ * limitations under the License. */ -#include "reflection.h" +#include "reflection-inl.h" #include "class_linker.h" #include "common_throws.h" @@ -592,7 +592,7 @@ jobject InvokeMethod(const ScopedObjectAccessAlreadyRunnable& soa, jobject javaM } // If method is not set to be accessible, verify it can be accessed by the caller. - if (!accessible && !VerifyAccess(receiver, declaring_class, m->GetAccessFlags())) { + if (!accessible && !VerifyAccess(soa.Self(), receiver, declaring_class, m->GetAccessFlags())) { ThrowIllegalAccessException(nullptr, StringPrintf("Cannot access method: %s", PrettyMethod(m).c_str()).c_str()); return nullptr; @@ -644,80 +644,6 @@ bool VerifyObjectIsClass(mirror::Object* o, mirror::Class* c) { return true; } -bool ConvertPrimitiveValue(const ThrowLocation* throw_location, bool unbox_for_result, - Primitive::Type srcType, Primitive::Type dstType, - const JValue& src, JValue* dst) { - DCHECK(srcType != Primitive::kPrimNot && dstType != Primitive::kPrimNot); - if (LIKELY(srcType == dstType)) { - dst->SetJ(src.GetJ()); - return true; - } - switch (dstType) { - case Primitive::kPrimBoolean: // Fall-through. - case Primitive::kPrimChar: // Fall-through. - case Primitive::kPrimByte: - // Only expect assignment with source and destination of identical type. - break; - case Primitive::kPrimShort: - if (srcType == Primitive::kPrimByte) { - dst->SetS(src.GetI()); - return true; - } - break; - case Primitive::kPrimInt: - if (srcType == Primitive::kPrimByte || srcType == Primitive::kPrimChar || - srcType == Primitive::kPrimShort) { - dst->SetI(src.GetI()); - return true; - } - break; - case Primitive::kPrimLong: - if (srcType == Primitive::kPrimByte || srcType == Primitive::kPrimChar || - srcType == Primitive::kPrimShort || srcType == Primitive::kPrimInt) { - dst->SetJ(src.GetI()); - return true; - } - break; - case Primitive::kPrimFloat: - if (srcType == Primitive::kPrimByte || srcType == Primitive::kPrimChar || - srcType == Primitive::kPrimShort || srcType == Primitive::kPrimInt) { - dst->SetF(src.GetI()); - return true; - } else if (srcType == Primitive::kPrimLong) { - dst->SetF(src.GetJ()); - return true; - } - break; - case Primitive::kPrimDouble: - if (srcType == Primitive::kPrimByte || srcType == Primitive::kPrimChar || - srcType == Primitive::kPrimShort || srcType == Primitive::kPrimInt) { - dst->SetD(src.GetI()); - return true; - } else if (srcType == Primitive::kPrimLong) { - dst->SetD(src.GetJ()); - return true; - } else if (srcType == Primitive::kPrimFloat) { - dst->SetD(src.GetF()); - return true; - } - break; - default: - break; - } - if (!unbox_for_result) { - ThrowIllegalArgumentException(throw_location, - StringPrintf("Invalid primitive conversion from %s to %s", - PrettyDescriptor(srcType).c_str(), - PrettyDescriptor(dstType).c_str()).c_str()); - } else { - ThrowClassCastException(throw_location, - StringPrintf("Couldn't convert result of type %s to %s", - PrettyDescriptor(srcType).c_str(), - PrettyDescriptor(dstType).c_str()).c_str()); - } - return false; -} - mirror::Object* BoxPrimitive(Primitive::Type src_class, const JValue& value) { if (src_class == Primitive::kPrimNot) { return value.GetL(); @@ -889,16 +815,18 @@ bool UnboxPrimitiveForResult(const ThrowLocation& throw_location, mirror::Object return UnboxPrimitive(&throw_location, o, dst_class, nullptr, unboxed_value); } -bool VerifyAccess(mirror::Object* obj, mirror::Class* declaring_class, uint32_t access_flags) { - NthCallerVisitor visitor(Thread::Current(), 2); +bool VerifyAccess(Thread* self, mirror::Object* obj, mirror::Class* declaring_class, uint32_t access_flags) { + if ((access_flags & kAccPublic) != 0) { + return true; + } + NthCallerVisitor visitor(self, 2); visitor.WalkStack(); if (UNLIKELY(visitor.caller == nullptr)) { // The caller is an attached native thread. - return (access_flags & kAccPublic) != 0; + return false; } mirror::Class* caller_class = visitor.caller->GetDeclaringClass(); - - if (((access_flags & kAccPublic) != 0) || (caller_class == declaring_class)) { + if (caller_class == declaring_class) { return true; } if ((access_flags & kAccPrivate) != 0) { @@ -912,10 +840,7 @@ bool VerifyAccess(mirror::Object* obj, mirror::Class* declaring_class, uint32_t return true; } } - if (!declaring_class->IsInSamePackage(caller_class)) { - return false; - } - return true; + return declaring_class->IsInSamePackage(caller_class); } } // namespace art diff --git a/runtime/reflection.h b/runtime/reflection.h index 61370c650..00f9d0912 100644 --- a/runtime/reflection.h +++ b/runtime/reflection.h @@ -43,9 +43,9 @@ bool UnboxPrimitiveForResult(const ThrowLocation& throw_location, mirror::Object mirror::Class* dst_class, JValue* unboxed_value) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); -bool ConvertPrimitiveValue(const ThrowLocation* throw_location, bool unbox_for_result, - Primitive::Type src_class, Primitive::Type dst_class, - const JValue& src, JValue* dst) +ALWAYS_INLINE bool ConvertPrimitiveValue(const ThrowLocation* throw_location, bool unbox_for_result, + Primitive::Type src_class, Primitive::Type dst_class, + const JValue& src, JValue* dst) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); JValue InvokeWithVarArgs(const ScopedObjectAccessAlreadyRunnable& soa, jobject obj, jmethodID mid, @@ -75,7 +75,8 @@ jobject InvokeMethod(const ScopedObjectAccessAlreadyRunnable& soa, jobject metho bool VerifyObjectIsClass(mirror::Object* o, mirror::Class* c) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); -bool VerifyAccess(mirror::Object* obj, mirror::Class* declaring_class, uint32_t access_flags) +bool VerifyAccess(Thread* self, mirror::Object* obj, mirror::Class* declaring_class, + uint32_t access_flags) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); } // namespace art diff --git a/runtime/thread-inl.h b/runtime/thread-inl.h index 6698634dc..170cec68d 100644 --- a/runtime/thread-inl.h +++ b/runtime/thread-inl.h @@ -23,7 +23,6 @@ #include "base/casts.h" #include "base/mutex-inl.h" -#include "entrypoints/entrypoint_utils-inl.h" #include "gc/heap.h" #include "jni_env_ext.h" diff --git a/runtime/utils.cc b/runtime/utils.cc index 9157f6c9b..dbd22139e 100644 --- a/runtime/utils.cc +++ b/runtime/utils.cc @@ -315,10 +315,6 @@ std::string PrettyDescriptor(const char* descriptor) { return result; } -std::string PrettyDescriptor(Primitive::Type type) { - return PrettyDescriptor(Primitive::Descriptor(type)); -} - std::string PrettyField(mirror::ArtField* f, bool with_type) { if (f == NULL) { return "null"; @@ -1421,4 +1417,8 @@ void PushWord(std::vector* buf, int data) { buf->push_back((data >> 24) & 0xff); } +std::string PrettyDescriptor(Primitive::Type type) { + return PrettyDescriptor(Primitive::Descriptor(type)); +} + } // namespace art diff --git a/runtime/utils_test.cc b/runtime/utils_test.cc index d6c90e1d4..1b2c3eec0 100644 --- a/runtime/utils_test.cc +++ b/runtime/utils_test.cc @@ -44,6 +44,18 @@ TEST_F(UtilsTest, PrettyDescriptor_ScalarReferences) { EXPECT_EQ("java.lang.String", PrettyDescriptor("Ljava/lang/String;")); } +TEST_F(UtilsTest, PrettyDescriptor_Primitive) { + EXPECT_EQ("boolean", PrettyDescriptor(Primitive::kPrimBoolean)); + EXPECT_EQ("byte", PrettyDescriptor(Primitive::kPrimByte)); + EXPECT_EQ("char", PrettyDescriptor(Primitive::kPrimChar)); + EXPECT_EQ("short", PrettyDescriptor(Primitive::kPrimShort)); + EXPECT_EQ("int", PrettyDescriptor(Primitive::kPrimInt)); + EXPECT_EQ("float", PrettyDescriptor(Primitive::kPrimFloat)); + EXPECT_EQ("long", PrettyDescriptor(Primitive::kPrimLong)); + EXPECT_EQ("double", PrettyDescriptor(Primitive::kPrimDouble)); + EXPECT_EQ("void", PrettyDescriptor(Primitive::kPrimVoid)); +} + TEST_F(UtilsTest, PrettyDescriptor_PrimitiveArrays) { EXPECT_EQ("boolean[]", PrettyDescriptor("[Z")); EXPECT_EQ("boolean[][]", PrettyDescriptor("[[Z")); -- 2.11.0