diff options
22 files changed, 848 insertions, 24 deletions
diff --git a/openjdkjvmti/fixed_up_dex_file.cc b/openjdkjvmti/fixed_up_dex_file.cc index 16bbee4037..ee83c4a4e8 100644 --- a/openjdkjvmti/fixed_up_dex_file.cc +++ b/openjdkjvmti/fixed_up_dex_file.cc @@ -115,6 +115,9 @@ std::unique_ptr<FixedUpDexFile> FixedUpDexFile::Create(const art::DexFile& origi // this before unquickening. art::Options options; options.compact_dex_level_ = art::CompactDexLevel::kCompactDexLevelNone; + // Never verify the output since hidden API flags may cause the dex file verifier to fail. + // See b/74063493 + options.verify_output_ = false; // Add a filter to only include the class that has the matching descriptor. static constexpr bool kFilterByDescriptor = true; if (kFilterByDescriptor) { diff --git a/openjdkjvmti/ti_redefine.cc b/openjdkjvmti/ti_redefine.cc index 46734548a8..5d430d2073 100644 --- a/openjdkjvmti/ti_redefine.cc +++ b/openjdkjvmti/ti_redefine.cc @@ -1402,13 +1402,6 @@ void Redefiner::ClassRedefinition::UpdateMethods(art::ObjPtr<art::mirror::Class> method.SetCodeItemOffset(dex_file_->FindCodeItemOffset(class_def, dex_method_idx)); // Clear all the intrinsics related flags. method.SetNotIntrinsic(); - // Notify the jit that this method is redefined. - art::jit::Jit* jit = driver_->runtime_->GetJit(); - // Non-invokable methods don't have any JIT data associated with them so we don't need to tell - // the jit about them. - if (jit != nullptr && method.IsInvokable()) { - jit->GetCodeCache()->NotifyMethodRedefined(&method); - } } } @@ -1450,6 +1443,23 @@ void Redefiner::ClassRedefinition::UpdateClass( art::ObjPtr<art::mirror::ClassExt> ext(mclass->GetExtData()); CHECK(!ext.IsNull()); ext->SetOriginalDexFile(original_dex_file); + + // Notify the jit that all the methods in this class were redefined. Need to do this last since + // the jit relies on the dex_file_ being correct (for native methods at least) to find the method + // meta-data. + art::jit::Jit* jit = driver_->runtime_->GetJit(); + if (jit != nullptr) { + art::PointerSize image_pointer_size = + driver_->runtime_->GetClassLinker()->GetImagePointerSize(); + auto code_cache = jit->GetCodeCache(); + // Non-invokable methods don't have any JIT data associated with them so we don't need to tell + // the jit about them. + for (art::ArtMethod& method : mclass->GetDeclaredMethods(image_pointer_size)) { + if (method.IsInvokable()) { + code_cache->NotifyMethodRedefined(&method); + } + } + } } // Restores the old obsolete methods maps if it turns out they weren't needed (ie there were no new diff --git a/runtime/art_method-inl.h b/runtime/art_method-inl.h index 145eb67aa9..92769942c0 100644 --- a/runtime/art_method-inl.h +++ b/runtime/art_method-inl.h @@ -295,6 +295,11 @@ inline const DexFile::ClassDef& ArtMethod::GetClassDef() { return GetDexFile()->GetClassDef(GetClassDefIndex()); } +inline size_t ArtMethod::GetNumberOfParameters() { + constexpr size_t return_type_count = 1u; + return strlen(GetShorty()) - return_type_count; +} + inline const char* ArtMethod::GetReturnTypeDescriptor() { DCHECK(!IsProxyMethod()); const DexFile* dex_file = GetDexFile(); diff --git a/runtime/art_method.h b/runtime/art_method.h index 013856f3fe..5d9b729847 100644 --- a/runtime/art_method.h +++ b/runtime/art_method.h @@ -602,6 +602,8 @@ class ArtMethod FINAL { const DexFile::ClassDef& GetClassDef() REQUIRES_SHARED(Locks::mutator_lock_); + ALWAYS_INLINE size_t GetNumberOfParameters() REQUIRES_SHARED(Locks::mutator_lock_); + const char* GetReturnTypeDescriptor() REQUIRES_SHARED(Locks::mutator_lock_); ALWAYS_INLINE Primitive::Type GetReturnTypePrimitive() REQUIRES_SHARED(Locks::mutator_lock_); diff --git a/runtime/dex/dex_file_annotations.cc b/runtime/dex/dex_file_annotations.cc index 3431bb7efb..6f3354b724 100644 --- a/runtime/dex/dex_file_annotations.cc +++ b/runtime/dex/dex_file_annotations.cc @@ -1121,6 +1121,21 @@ mirror::ObjectArray<mirror::Object>* GetParameterAnnotations(ArtMethod* method) return ProcessAnnotationSetRefList(ClassData(method), set_ref_list, size); } +uint32_t GetNumberOfAnnotatedMethodParameters(ArtMethod* method) { + const DexFile* dex_file = method->GetDexFile(); + const DexFile::ParameterAnnotationsItem* parameter_annotations = + FindAnnotationsItemForMethod(method); + if (parameter_annotations == nullptr) { + return 0u; + } + const DexFile::AnnotationSetRefList* set_ref_list = + dex_file->GetParameterAnnotationSetRefList(parameter_annotations); + if (set_ref_list == nullptr) { + return 0u; + } + return set_ref_list->size_; +} + mirror::Object* GetAnnotationForMethodParameter(ArtMethod* method, uint32_t parameter_idx, Handle<mirror::Class> annotation_class) { @@ -1141,7 +1156,9 @@ mirror::Object* GetAnnotationForMethodParameter(ArtMethod* method, const DexFile::AnnotationSetRefItem* annotation_set_ref = &set_ref_list->list_[parameter_idx]; const DexFile::AnnotationSetItem* annotation_set = dex_file->GetSetRefItemItem(annotation_set_ref); - + if (annotation_set == nullptr) { + return nullptr; + } return GetAnnotationObjectFromAnnotationSet(ClassData(method), annotation_set, DexFile::kDexVisibilityRuntime, diff --git a/runtime/dex/dex_file_annotations.h b/runtime/dex/dex_file_annotations.h index d7ebf84b1c..4bb0d75a57 100644 --- a/runtime/dex/dex_file_annotations.h +++ b/runtime/dex/dex_file_annotations.h @@ -55,6 +55,8 @@ mirror::ObjectArray<mirror::Class>* GetExceptionTypesForMethod(ArtMethod* method REQUIRES_SHARED(Locks::mutator_lock_); mirror::ObjectArray<mirror::Object>* GetParameterAnnotations(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_); +uint32_t GetNumberOfAnnotatedMethodParameters(ArtMethod* method) + REQUIRES_SHARED(Locks::mutator_lock_); mirror::Object* GetAnnotationForMethodParameter(ArtMethod* method, uint32_t parameter_idx, Handle<mirror::Class> annotation_class) diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h index a1d0ff7374..6000317c85 100644 --- a/runtime/mirror/class.h +++ b/runtime/mirror/class.h @@ -186,6 +186,11 @@ class MANAGED Class FINAL : public Object { void SetAccessFlags(uint32_t new_access_flags) REQUIRES_SHARED(Locks::mutator_lock_); + // Returns true if the class is an enum. + ALWAYS_INLINE bool IsEnum() REQUIRES_SHARED(Locks::mutator_lock_) { + return (GetAccessFlags() & kAccEnum) != 0; + } + // Returns true if the class is an interface. ALWAYS_INLINE bool IsInterface() REQUIRES_SHARED(Locks::mutator_lock_) { return (GetAccessFlags() & kAccInterface) != 0; diff --git a/runtime/native/java_lang_reflect_Executable.cc b/runtime/native/java_lang_reflect_Executable.cc index a5e70affa5..b129c66759 100644 --- a/runtime/native/java_lang_reflect_Executable.cc +++ b/runtime/native/java_lang_reflect_Executable.cc @@ -70,7 +70,6 @@ static jobjectArray Executable_getSignatureAnnotation(JNIEnv* env, jobject javaM if (method->GetDeclaringClass()->IsProxyClass()) { return nullptr; } - StackHandleScope<1> hs(soa.Self()); return soa.AddLocalReference<jobjectArray>(annotations::GetSignatureAnnotationForMethod(method)); } @@ -80,9 +79,76 @@ static jobjectArray Executable_getParameterAnnotationsNative(JNIEnv* env, jobjec ArtMethod* method = ArtMethod::FromReflectedMethod(soa, javaMethod); if (method->IsProxyMethod()) { return nullptr; + } + + StackHandleScope<4> hs(soa.Self()); + Handle<mirror::ObjectArray<mirror::Object>> annotations = + hs.NewHandle(annotations::GetParameterAnnotations(method)); + if (annotations.IsNull()) { + return nullptr; + } + + // If the method is not a constructor, or has parameter annotations + // for each parameter, then we can return those annotations + // unmodified. Otherwise, we need to look at whether the + // constructor has implicit parameters as these may need padding + // with empty parameter annotations. + if (!method->IsConstructor() || + annotations->GetLength() == static_cast<int>(method->GetNumberOfParameters())) { + return soa.AddLocalReference<jobjectArray>(annotations.Get()); + } + + // If declaring class is a local or an enum, do not pad parameter + // annotations, as the implicit constructor parameters are an implementation + // detail rather than required by JLS. + Handle<mirror::Class> declaring_class = hs.NewHandle(method->GetDeclaringClass()); + if (annotations::GetEnclosingMethod(declaring_class) != nullptr || + declaring_class->IsEnum()) { + return soa.AddLocalReference<jobjectArray>(annotations.Get()); + } + + // Prepare to resize the annotations so there is 1:1 correspondence + // with the constructor parameters. + Handle<mirror::ObjectArray<mirror::Object>> resized_annotations = hs.NewHandle( + mirror::ObjectArray<mirror::Object>::Alloc( + soa.Self(), + annotations->GetClass(), + static_cast<int>(method->GetNumberOfParameters()))); + if (resized_annotations.IsNull()) { + DCHECK(soa.Self()->IsExceptionPending()); + return nullptr; + } + + static constexpr bool kTransactionActive = false; + const int32_t offset = resized_annotations->GetLength() - annotations->GetLength(); + if (offset > 0) { + // Workaround for dexers (d8/dx) that do not insert annotations + // for implicit parameters (b/68033708). + ObjPtr<mirror::Class> annotation_array_class = + soa.Decode<mirror::Class>(WellKnownClasses::java_lang_annotation_Annotation__array); + Handle<mirror::ObjectArray<mirror::Object>> empty_annotations = hs.NewHandle( + mirror::ObjectArray<mirror::Object>::Alloc(soa.Self(), annotation_array_class, 0)); + if (empty_annotations.IsNull()) { + DCHECK(soa.Self()->IsExceptionPending()); + return nullptr; + } + for (int i = 0; i < offset; ++i) { + resized_annotations->SetWithoutChecks<kTransactionActive>(i, empty_annotations.Get()); + } + for (int i = 0; i < annotations->GetLength(); ++i) { + ObjPtr<mirror::Object> annotation = annotations->GetWithoutChecks(i); + resized_annotations->SetWithoutChecks<kTransactionActive>(i + offset, annotation); + } } else { - return soa.AddLocalReference<jobjectArray>(annotations::GetParameterAnnotations(method)); + // Workaround for Jack (defunct) erroneously inserting annotations + // for local classes (b/68033708). + DCHECK_LT(offset, 0); + for (int i = 0; i < resized_annotations->GetLength(); ++i) { + ObjPtr<mirror::Object> annotation = annotations->GetWithoutChecks(i - offset); + resized_annotations->SetWithoutChecks<kTransactionActive>(i, annotation); + } } + return soa.AddLocalReference<jobjectArray>(resized_annotations.Get()); } static jobjectArray Executable_getParameters0(JNIEnv* env, jobject javaMethod) { diff --git a/runtime/native/java_lang_reflect_Parameter.cc b/runtime/native/java_lang_reflect_Parameter.cc index 0b3015bda8..1ab91098d7 100644 --- a/runtime/native/java_lang_reflect_Parameter.cc +++ b/runtime/native/java_lang_reflect_Parameter.cc @@ -58,6 +58,40 @@ static jobject Parameter_getAnnotationNative(JNIEnv* env, return nullptr; } + uint32_t annotated_parameter_count = annotations::GetNumberOfAnnotatedMethodParameters(method); + if (annotated_parameter_count == 0u) { + return nullptr; + } + + // For constructors with implicit arguments, we may need to adjust + // annotation positions based on whether the implicit parameters are + // expected to known and not just a compiler implementation detail. + if (method->IsConstructor()) { + StackHandleScope<1> hs(soa.Self()); + // If declaring class is a local or an enum, do not pad parameter + // annotations, as the implicit constructor parameters are an + // implementation detail rather than required by JLS. + Handle<mirror::Class> declaring_class = hs.NewHandle(method->GetDeclaringClass()); + if (annotations::GetEnclosingMethod(declaring_class) == nullptr && !declaring_class->IsEnum()) { + // Adjust the parameter index if the number of annotations does + // not match the number of parameters. + if (annotated_parameter_count <= parameter_count) { + // Workaround for dexer not inserting annotation state for implicit parameters (b/68033708). + uint32_t skip_count = parameter_count - annotated_parameter_count; + DCHECK_GE(2u, skip_count); + if (parameterIndex < static_cast<jint>(skip_count)) { + return nullptr; + } + parameterIndex -= skip_count; + } else { + // Workaround for Jack erroneously inserting implicit parameter for local classes + // (b/68033708). + DCHECK_EQ(1u, annotated_parameter_count - parameter_count); + parameterIndex += static_cast<jint>(annotated_parameter_count - parameter_count); + } + } + } + StackHandleScope<1> hs(soa.Self()); Handle<mirror::Class> klass(hs.NewHandle(soa.Decode<mirror::Class>(annotationType))); return soa.AddLocalReference<jobject>( @@ -65,9 +99,10 @@ static jobject Parameter_getAnnotationNative(JNIEnv* env, } static JNINativeMethod gMethods[] = { - FAST_NATIVE_METHOD(Parameter, - getAnnotationNative, - "(Ljava/lang/reflect/Executable;ILjava/lang/Class;)Ljava/lang/annotation/Annotation;"), + FAST_NATIVE_METHOD( + Parameter, + getAnnotationNative, + "(Ljava/lang/reflect/Executable;ILjava/lang/Class;)Ljava/lang/annotation/Annotation;"), }; void register_java_lang_reflect_Parameter(JNIEnv* env) { diff --git a/runtime/oat_file_assistant.cc b/runtime/oat_file_assistant.cc index 6bf05b77a8..c5569ff909 100644 --- a/runtime/oat_file_assistant.cc +++ b/runtime/oat_file_assistant.cc @@ -1261,18 +1261,33 @@ std::unique_ptr<OatFile> OatFileAssistant::OatFileInfo::ReleaseFileForUse() { return ReleaseFile(); } - if (Status() == kOatRelocationOutOfDate) { - // We are loading an oat file for runtime use that needs relocation. - // Reload the file non-executable to ensure that we interpret out of the - // dex code in the oat file rather than trying to execute the unrelocated - // compiled code. - oat_file_assistant_->load_executable_ = false; - Reset(); - if (IsUseable()) { - CHECK(!IsExecutable()); - return ReleaseFile(); - } + switch (Status()) { + case kOatBootImageOutOfDate: + if (oat_file_assistant_->HasOriginalDexFiles()) { + // If there are original dex files, it is better to use them. + break; + } + FALLTHROUGH_INTENDED; + + case kOatRelocationOutOfDate: + // We are loading an oat file for runtime use that needs relocation. + // Reload the file non-executable to ensure that we interpret out of the + // dex code in the oat file rather than trying to execute the unrelocated + // compiled code. + oat_file_assistant_->load_executable_ = false; + Reset(); + if (IsUseable()) { + CHECK(!IsExecutable()); + return ReleaseFile(); + } + break; + + case kOatUpToDate: + case kOatCannotOpen: + case kOatDexOutOfDate: + break; } + return std::unique_ptr<OatFile>(); } diff --git a/runtime/oat_file_manager.cc b/runtime/oat_file_manager.cc index e4194442d3..f6fb9ded87 100644 --- a/runtime/oat_file_manager.cc +++ b/runtime/oat_file_manager.cc @@ -469,6 +469,9 @@ std::vector<std::unique_ptr<const DexFile>> OatFileManager::OpenDexFilesFromOat( // Get the oat file on disk. std::unique_ptr<const OatFile> oat_file(oat_file_assistant.GetBestOatFile().release()); + VLOG(oat) << "OatFileAssistant(" << dex_location << ").GetBestOatFile()=" + << reinterpret_cast<uintptr_t>(oat_file.get()) + << " (executable=" << (oat_file != nullptr ? oat_file->IsExecutable() : false) << ")"; if ((class_loader != nullptr || dex_elements != nullptr) && oat_file != nullptr) { // Prevent oat files from being loaded if no class_loader or dex_elements are provided. diff --git a/test/1949-short-dex-file/expected.txt b/test/1949-short-dex-file/expected.txt new file mode 100644 index 0000000000..863339fb8c --- /dev/null +++ b/test/1949-short-dex-file/expected.txt @@ -0,0 +1 @@ +Passed diff --git a/test/1949-short-dex-file/info.txt b/test/1949-short-dex-file/info.txt new file mode 100644 index 0000000000..e924086e23 --- /dev/null +++ b/test/1949-short-dex-file/info.txt @@ -0,0 +1,30 @@ +Tests the fix for b/74116990 + +The JIT was reading into incorrect dex files during class redefinition if a +native method was present. + +The transformed dex file is specifically crafted to have exactly 4 methodIDs in +it. They are (in order): + (0) Ljava/lang/Object;-><init>()V + (1) Lxyz/Transform;-><init>()V + (2) Lxyz/Transform;->bar()V + (3) Lxyz/Transform;->foo()V + +In the transformed version of the dex file there is a new method. The new list of methodIDs is: + (0) Lart/Test1949;->doNothing()V + (1) Ljava/lang/Object;-><init>()V + (2) Lxyz/Transform;-><init>()V + (3) Lxyz/Transform;->bar()V + (4) Lxyz/Transform;->foo()V + +This test tries to get the JIT to read out-of-bounds on the initial dex file by getting it to +read the 5th method id of the new file (Lxyz/Transform;->foo()V) from the old dex file (which +only has 4 method ids). + +To do this we need to make sure that the class being transformed is near the end of the +alphabet (package xyz, method foo). If it is further forward than the other method-ids then the +JIT will read an incorrect (but valid) method-id from the old-dex file. This is why the error +wasn't caught in our other tests (package art is always at the front). + +The final method that causes the OOB read needs to be a native method because that is the only +method-type the jit uses dex-file information to keep track of. diff --git a/test/1949-short-dex-file/run b/test/1949-short-dex-file/run new file mode 100755 index 0000000000..c6e62ae6cd --- /dev/null +++ b/test/1949-short-dex-file/run @@ -0,0 +1,17 @@ +#!/bin/bash +# +# Copyright 2016 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. + +./default-run "$@" --jvmti diff --git a/test/1949-short-dex-file/src/Main.java b/test/1949-short-dex-file/src/Main.java new file mode 100644 index 0000000000..dbbaa861c9 --- /dev/null +++ b/test/1949-short-dex-file/src/Main.java @@ -0,0 +1,21 @@ +/* + * Copyright (C) 2017 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. + */ + +public class Main { + public static void main(String[] args) throws Exception { + art.Test1949.run(); + } +} diff --git a/test/1949-short-dex-file/src/art/Redefinition.java b/test/1949-short-dex-file/src/art/Redefinition.java new file mode 100644 index 0000000000..56d2938a01 --- /dev/null +++ b/test/1949-short-dex-file/src/art/Redefinition.java @@ -0,0 +1,91 @@ +/* + * Copyright (C) 2017 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. + */ + +package art; + +import java.util.ArrayList; +// Common Redefinition functions. Placed here for use by CTS +public class Redefinition { + public static final class CommonClassDefinition { + public final Class<?> target; + public final byte[] class_file_bytes; + public final byte[] dex_file_bytes; + + public CommonClassDefinition(Class<?> target, byte[] class_file_bytes, byte[] dex_file_bytes) { + this.target = target; + this.class_file_bytes = class_file_bytes; + this.dex_file_bytes = dex_file_bytes; + } + } + + // A set of possible test configurations. Test should set this if they need to. + // This must be kept in sync with the defines in ti-agent/common_helper.cc + public static enum Config { + COMMON_REDEFINE(0), + COMMON_RETRANSFORM(1), + COMMON_TRANSFORM(2); + + private final int val; + private Config(int val) { + this.val = val; + } + } + + public static void setTestConfiguration(Config type) { + nativeSetTestConfiguration(type.val); + } + + private static native void nativeSetTestConfiguration(int type); + + // Transforms the class + public static native void doCommonClassRedefinition(Class<?> target, + byte[] classfile, + byte[] dexfile); + + public static void doMultiClassRedefinition(CommonClassDefinition... defs) { + ArrayList<Class<?>> classes = new ArrayList<>(); + ArrayList<byte[]> class_files = new ArrayList<>(); + ArrayList<byte[]> dex_files = new ArrayList<>(); + + for (CommonClassDefinition d : defs) { + classes.add(d.target); + class_files.add(d.class_file_bytes); + dex_files.add(d.dex_file_bytes); + } + doCommonMultiClassRedefinition(classes.toArray(new Class<?>[0]), + class_files.toArray(new byte[0][]), + dex_files.toArray(new byte[0][])); + } + + public static void addMultiTransformationResults(CommonClassDefinition... defs) { + for (CommonClassDefinition d : defs) { + addCommonTransformationResult(d.target.getCanonicalName(), + d.class_file_bytes, + d.dex_file_bytes); + } + } + + public static native void doCommonMultiClassRedefinition(Class<?>[] targets, + byte[][] classfiles, + byte[][] dexfiles); + public static native void doCommonClassRetransformation(Class<?>... target); + public static native void setPopRetransformations(boolean pop); + public static native void popTransformationFor(String name); + public static native void enableCommonRetransformation(boolean enable); + public static native void addCommonTransformationResult(String target_name, + byte[] class_bytes, + byte[] dex_bytes); +} diff --git a/test/1949-short-dex-file/src/art/Test1949.java b/test/1949-short-dex-file/src/art/Test1949.java new file mode 100644 index 0000000000..98fa7fc2c1 --- /dev/null +++ b/test/1949-short-dex-file/src/art/Test1949.java @@ -0,0 +1,142 @@ +/* + * Copyright (C) 2016 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. + */ + +package art; + +import java.lang.reflect.*; +import java.util.Base64; +import java.nio.ByteBuffer; + +public class Test1949 { + private final static boolean isDalvik = System.getProperty("java.vm.name").equals("Dalvik"); + + // This dex file is specifically crafted to have exactly 4 methodIDs in it. They are (in order): + // (0) Ljava/lang/Object;-><init>()V + // (1) Lxyz/Transform;-><init>()V + // (2) Lxyz/Transform;->bar()V + // (3) Lxyz/Transform;->foo()V + // + // In the transformed version of the dex file there is a new method. The new list of methodIDs is: + // (0) Lart/Test1949;->doNothing()V + // (1) Ljava/lang/Object;-><init>()V + // (2) Lxyz/Transform;-><init>()V + // (3) Lxyz/Transform;->bar()V + // (4) Lxyz/Transform;->foo()V + // + // This test tries to get the JIT to read out-of-bounds on the initial dex file by getting it to + // read the 5th method id of the new file (Lxyz/Transform;->foo()V) from the old dex file (which + // only has 4 method ids). + // + // To do this we need to make sure that the class being transformed is near the end of the + // alphabet (package xyz, method foo). If it is further forward than the other method-ids then the + // JIT will read an incorrect (but valid) method-id from the old-dex file. This is why the error + // wasn't caught in our other tests (package art is always at the front). + // + // The final method that causes the OOB read needs to be a native method because that is the only + // method-type the jit uses dex-file information to keep track of. + + /** + * base64 encoded class/dex file for + * package xyz; + * public class Transform { + * public native void foo(); + * public void bar() {} + * } + */ + private static final byte[] CLASS_BYTES_INIT = Base64.getDecoder().decode( + "yv66vgAAADUADwoAAwAMBwANBwAOAQAGPGluaXQ+AQADKClWAQAEQ29kZQEAD0xpbmVOdW1iZXJU" + + "YWJsZQEAA2ZvbwEAA2JhcgEAClNvdXJjZUZpbGUBAA5UcmFuc2Zvcm0uamF2YQwABAAFAQANeHl6" + + "L1RyYW5zZm9ybQEAEGphdmEvbGFuZy9PYmplY3QAIQACAAMAAAAAAAMAAQAEAAUAAQAGAAAAHQAB" + + "AAEAAAAFKrcAAbEAAAABAAcAAAAGAAEAAAACAQEACAAFAAAAAQAJAAUAAQAGAAAAGQAAAAEAAAAB" + + "sQAAAAEABwAAAAYAAQAAAAQAAQAKAAAAAgAL"); + private static final byte[] DEX_BYTES_INIT = Base64.getDecoder().decode( + "ZGV4CjAzNQBDUutFJpeT+okk+aXah8NQ61q2XRtkmChwAgAAcAAAAHhWNBIAAAAAAAAAANwBAAAI" + + "AAAAcAAAAAMAAACQAAAAAQAAAJwAAAAAAAAAAAAAAAQAAACoAAAAAQAAAMgAAACIAQAA6AAAABwB" + + "AAAkAQAAOAEAAEkBAABZAQAAXAEAAGEBAABmAQAAAQAAAAIAAAAEAAAABAAAAAIAAAAAAAAAAAAA" + + "AAAAAAABAAAAAAAAAAEAAAAFAAAAAQAAAAYAAAABAAAAAQAAAAAAAAAAAAAAAwAAAAAAAADDAQAA" + + "AAAAAAEAAQABAAAAEgEAAAQAAABwEAAAAAAOAAEAAQAAAAAAFgEAAAEAAAAOAAIADgAEAA4AAAAG" + + "PGluaXQ+ABJMamF2YS9sYW5nL09iamVjdDsAD0x4eXovVHJhbnNmb3JtOwAOVHJhbnNmb3JtLmph" + + "dmEAAVYAA2JhcgADZm9vAFt+fkQ4eyJtaW4tYXBpIjoxLCJzaGEtMSI6IjkwZWYyMjkwNWMzZmVj" + + "Y2FiMjMwMzBhNmJkYzU2NTcwYTMzNWVmMDUiLCJ2ZXJzaW9uIjoidjEuMS44LWRldiJ9AAAAAQIB" + + "gYAE6AECAYACAYECAAAAAAAAAAAMAAAAAAAAAAEAAAAAAAAAAQAAAAgAAABwAAAAAgAAAAMAAACQ" + + "AAAAAwAAAAEAAACcAAAABQAAAAQAAACoAAAABgAAAAEAAADIAAAAASAAAAIAAADoAAAAAyAAAAIA" + + "AAASAQAAAiAAAAgAAAAcAQAAACAAAAEAAADDAQAAAxAAAAEAAADYAQAAABAAAAEAAADcAQAA"); + + /** + * base64 encoded class/dex file for + * package xyz; + * public class Transform { + * public native void foo(); + * public void bar() { + * // Make sure the methodID is before any of the ones in Transform + * art.Test1949.doNothing(); + * } + * } + */ + private static final byte[] CLASS_BYTES_FINAL = Base64.getDecoder().decode( + "yv66vgAAADUAFAoABAANCgAOAA8HABAHABEBAAY8aW5pdD4BAAMoKVYBAARDb2RlAQAPTGluZU51" + + "bWJlclRhYmxlAQADZm9vAQADYmFyAQAKU291cmNlRmlsZQEADlRyYW5zZm9ybS5qYXZhDAAFAAYH" + + "ABIMABMABgEADXh5ei9UcmFuc2Zvcm0BABBqYXZhL2xhbmcvT2JqZWN0AQAMYXJ0L1Rlc3QxOTQ5" + + "AQAJZG9Ob3RoaW5nACEAAwAEAAAAAAADAAEABQAGAAEABwAAAB0AAQABAAAABSq3AAGxAAAAAQAI" + + "AAAABgABAAAAAgEBAAkABgAAAAEACgAGAAEABwAAABwAAAABAAAABLgAArEAAAABAAgAAAAGAAEA" + + "AAAEAAEACwAAAAIADA=="); + private static final byte[] DEX_BYTES_FINAL = Base64.getDecoder().decode( + "ZGV4CjAzNQBHXBiw7Hso1vnmaXE1VCV41f4+0aECixOgAgAAcAAAAHhWNBIAAAAAAAAAAAwCAAAK" + + "AAAAcAAAAAQAAACYAAAAAQAAAKgAAAAAAAAAAAAAAAUAAAC0AAAAAQAAANwAAACkAQAA/AAAADQB" + + "AAA8AQAATAEAAGABAABxAQAAgQEAAIQBAACJAQAAlAEAAJkBAAABAAAAAgAAAAMAAAAFAAAABQAA" + + "AAMAAAAAAAAAAAAAAAcAAAABAAAAAAAAAAIAAAAAAAAAAgAAAAYAAAACAAAACAAAAAIAAAABAAAA" + + "AQAAAAAAAAAEAAAAAAAAAPYBAAAAAAAAAQABAAEAAAAsAQAABAAAAHAQAQAAAA4AAQABAAAAAAAw" + + "AQAABAAAAHEAAAAAAA4AAgAOAAQADgAGPGluaXQ+AA5MYXJ0L1Rlc3QxOTQ5OwASTGphdmEvbGFu" + + "Zy9PYmplY3Q7AA9MeHl6L1RyYW5zZm9ybTsADlRyYW5zZm9ybS5qYXZhAAFWAANiYXIACWRvTm90" + + "aGluZwADZm9vAFt+fkQ4eyJtaW4tYXBpIjoxLCJzaGEtMSI6IjkwZWYyMjkwNWMzZmVjY2FiMjMw" + + "MzBhNmJkYzU2NTcwYTMzNWVmMDUiLCJ2ZXJzaW9uIjoidjEuMS44LWRldiJ9AAAAAQICgYAE/AED" + + "AZQCAYECAAAAAAAMAAAAAAAAAAEAAAAAAAAAAQAAAAoAAABwAAAAAgAAAAQAAACYAAAAAwAAAAEA" + + "AACoAAAABQAAAAUAAAC0AAAABgAAAAEAAADcAAAAASAAAAIAAAD8AAAAAyAAAAIAAAAsAQAAAiAA" + + "AAoAAAA0AQAAACAAAAEAAAD2AQAAAxAAAAEAAAAIAgAAABAAAAEAAAAMAgAA"); + + public static void run() throws Exception { + Redefinition.setTestConfiguration(Redefinition.Config.COMMON_REDEFINE); + doTest(); + } + + // A method with a methodID before anything in Transform. + public static void doNothing() {} + + private static ClassLoader CreateClassLoader(byte[] clz, byte[] dex) throws Exception { + if (isDalvik) { + Class<?> class_loader_class = Class.forName("dalvik.system.InMemoryDexClassLoader"); + Constructor<?> ctor = class_loader_class.getConstructor(ByteBuffer.class, ClassLoader.class); + /* on Dalvik, this is a DexFile; otherwise, it's null */ + return (ClassLoader)ctor.newInstance(ByteBuffer.wrap(dex), Test1949.class.getClassLoader()); + } else { + return new ClassLoader() { + public Class<?> findClass(String name) throws ClassNotFoundException { + if (name.equals("xyz.Transform")) { + return defineClass(name, clz, 0, clz.length); + } else { + throw new ClassNotFoundException("Couldn't find class: " + name); + } + } + }; + } + } + + public static void doTest() throws Exception { + Class c = CreateClassLoader(CLASS_BYTES_INIT, DEX_BYTES_INIT).loadClass("xyz.Transform"); + Redefinition.doCommonClassRedefinition(c, CLASS_BYTES_FINAL, DEX_BYTES_FINAL); + System.out.println("Passed"); + } +} diff --git a/test/715-clinit-implicit-parameter-annotations/build b/test/715-clinit-implicit-parameter-annotations/build new file mode 100644 index 0000000000..4753c8c7dc --- /dev/null +++ b/test/715-clinit-implicit-parameter-annotations/build @@ -0,0 +1,24 @@ +#!/bin/bash +# +# Copyright 2018 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. + +# Make us exit on a failure +set -e + +# Always use D8 as DX does not support propagating parameter name and +# access_flag information. +export USE_D8=true + +./default-build "$@" --experimental parameter-annotations diff --git a/test/715-clinit-implicit-parameter-annotations/expected.txt b/test/715-clinit-implicit-parameter-annotations/expected.txt new file mode 100644 index 0000000000..357eb6253e --- /dev/null +++ b/test/715-clinit-implicit-parameter-annotations/expected.txt @@ -0,0 +1,113 @@ +Main + public Main() +Main$1LocalClassStaticContext + Main$1LocalClassStaticContext(int) + Parameter [0]: Indexed : @Main$AnnotationA() + Array : @Main$AnnotationA() + Main$AnnotationA Yes + @Main$AnnotationA() + Main$AnnotationB No +Main$1LocalClassStaticContextWithCapture + Main$1LocalClassStaticContextWithCapture(java.lang.String,long) + Parameter [0]: Indexed : @Main$AnnotationA() + Array : @Main$AnnotationA() + Main$AnnotationA Yes + @Main$AnnotationA() + Main$AnnotationB No +Main$1LocalClassStaticContextWithCaptureAlternateOrdering + Main$1LocalClassStaticContextWithCaptureAlternateOrdering(java.lang.String,long) + Parameter [0]: Indexed : @Main$AnnotationA() + Array : @Main$AnnotationA() + Main$AnnotationA Yes + @Main$AnnotationA() + Main$AnnotationB No +Main$1LocalClass + Main$1LocalClass(Main,int) + Parameter [0]: Indexed : @Main$AnnotationA() + Array : @Main$AnnotationA() + Main$AnnotationA Yes + @Main$AnnotationA() + Main$AnnotationB No +Main$1LocalClassWithCapture + Main$1LocalClassWithCapture(Main,java.lang.String,long) + Parameter [0]: Indexed : @Main$AnnotationA() + Array : @Main$AnnotationA() + Main$AnnotationA Yes + @Main$AnnotationA() + Main$AnnotationB No +Main$Inner + Main$Inner(Main,int,java.lang.String) + Parameter [0]: Main$AnnotationA No + Main$AnnotationB No + Parameter [1]: Indexed : @Main$AnnotationA() + Array : @Main$AnnotationA() + Main$AnnotationA Yes + @Main$AnnotationA() + Main$AnnotationB No + Parameter [2]: Main$AnnotationA No + Main$AnnotationB No + Main$Inner(Main,int,java.lang.String,boolean) + Parameter [0]: Main$AnnotationA No + Main$AnnotationB No + Parameter [1]: Indexed : @Main$AnnotationA() + Array : @Main$AnnotationA() + Main$AnnotationA Yes + @Main$AnnotationA() + Main$AnnotationB No + Parameter [2]: Main$AnnotationA No + Main$AnnotationB No + Parameter [3]: Indexed : @Main$AnnotationB(value=x) + Array : @Main$AnnotationB(value=x) + Main$AnnotationA No + Main$AnnotationB Yes + @Main$AnnotationB(value=x) +Main$StaticInner + Main$StaticInner(int,java.lang.String) + Parameter [0]: Indexed : @Main$AnnotationA() + Array : @Main$AnnotationA() + Main$AnnotationA Yes + @Main$AnnotationA() + Main$AnnotationB No + Parameter [1]: Main$AnnotationA No + Main$AnnotationB No + Main$StaticInner(int,java.lang.String,boolean) + Parameter [0]: Indexed : @Main$AnnotationB(value=foo) + Array : @Main$AnnotationB(value=foo) + Main$AnnotationA No + Main$AnnotationB Yes + @Main$AnnotationB(value=foo) + Parameter [1]: Main$AnnotationA No + Main$AnnotationB No + Parameter [2]: Indexed : @Main$AnnotationA() + Array : @Main$AnnotationA() + Main$AnnotationA Yes + @Main$AnnotationA() + Main$AnnotationB No +Main$ImportantNumber + private Main$ImportantNumber(java.lang.String,int,double) + Parameter [0]: Indexed : @Main$AnnotationA() + Array : @Main$AnnotationA() + Main$AnnotationA Yes + @Main$AnnotationA() + Main$AnnotationB No + private Main$ImportantNumber(java.lang.String,int,double,boolean) + Parameter [0]: Indexed : @Main$AnnotationB(value=x) + Array : @Main$AnnotationB(value=x) + Main$AnnotationA No + Main$AnnotationB Yes + @Main$AnnotationB(value=x) + Parameter [1]: Indexed : @Main$AnnotationB(value=y) + Array : @Main$AnnotationB(value=y) + Main$AnnotationA No + Main$AnnotationB Yes + @Main$AnnotationB(value=y) +Main$BinaryNumber + private Main$BinaryNumber(java.lang.String,int) + Parameter [0]: Main$AnnotationA No + Main$AnnotationB No + Parameter [1]: Main$AnnotationA No + Main$AnnotationB No +Main$1 + Main$1(java.lang.String) + Parameter [0]: Main$AnnotationA No + Main$AnnotationB No diff --git a/test/715-clinit-implicit-parameter-annotations/info.txt b/test/715-clinit-implicit-parameter-annotations/info.txt new file mode 100644 index 0000000000..31afd62f27 --- /dev/null +++ b/test/715-clinit-implicit-parameter-annotations/info.txt @@ -0,0 +1,5 @@ +Tests ART synthesizes parameter annotations for implicit parameters on +constructors. Inner class and enum constructors may have implicit +parameters. If the constructor has parameter annotations, the implicit +parameters may not have annotations in the DEX file, but code that +looks at these annotations will expect them to. diff --git a/test/715-clinit-implicit-parameter-annotations/src/Main.java b/test/715-clinit-implicit-parameter-annotations/src/Main.java new file mode 100644 index 0000000000..351e3a94b3 --- /dev/null +++ b/test/715-clinit-implicit-parameter-annotations/src/Main.java @@ -0,0 +1,215 @@ +/* + * Copyright (C) 2018 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. + */ + +import java.lang.annotation.Annotation; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.reflect.Constructor; +import java.lang.reflect.Parameter; + +public class Main { + // A simple parameter annotation + @Retention(RetentionPolicy.RUNTIME) + public @interface AnnotationA {} + + // A parameter annotation with additional state + @Retention(RetentionPolicy.RUNTIME) + public @interface AnnotationB { + String value() default "default-value"; + } + + // An inner class whose constructors with have an implicit + // argument for the enclosing instance. + public class Inner { + private final int number; + private final String text; + boolean flag; + + Inner(@AnnotationA int number, String text) { + this.number = number; + this.text = text; + this.flag = false; + } + + Inner(@AnnotationA int number, String text, @AnnotationB("x") boolean flag) { + this.number = number; + this.text = text; + this.flag = flag; + } + } + + // An inner class whose constructors with have no implicit + // arguments for the enclosing instance. + public static class StaticInner { + private final int number; + private final String text; + boolean flag; + + StaticInner(@AnnotationA int number, String text) { + this.number = number; + this.text = text; + this.flag = false; + } + + StaticInner(@AnnotationB("foo") int number, String text, @AnnotationA boolean flag) { + this.number = number; + this.text = text; + this.flag = flag; + } + } + + public enum ImportantNumber { + ONE(1.0), + TWO(2.0), + MANY(3.0, true); + + private double doubleValue; + private boolean isLarge; + + ImportantNumber(@AnnotationA double doubleValue) { + this.doubleValue = doubleValue; + this.isLarge = false; + } + + ImportantNumber(@AnnotationB("x") double doubleValue, @AnnotationB("y") boolean isLarge) { + this.doubleValue = doubleValue; + this.isLarge = isLarge; + } + } + + public enum BinaryNumber { + ZERO, + ONE; + } + + private abstract static class AnonymousBase { + public AnonymousBase(@AnnotationA String s) {} + } + + private static String annotationToNormalizedString(Annotation annotation) { + // String.replace() to accomodate different representation across VMs. + return annotation.toString().replace("\"", ""); + } + + private static void DumpConstructorParameterAnnotations(Class<?> cls) throws Throwable { + System.out.println(cls.getName()); + for (Constructor c : cls.getDeclaredConstructors()) { + System.out.println(" " + c); + Annotation[][] annotations = c.getParameterAnnotations(); + Parameter[] parameters = c.getParameters(); + for (int i = 0; i < annotations.length; ++i) { + // Exercise java.lang.reflect.Executable.getParameterAnnotationsNative() + // which retrieves all annotations for the parameters. + System.out.print(" Parameter [" + i + "]:"); + for (Annotation annotation : parameters[i].getAnnotations()) { + System.out.println(" Indexed : " + annotationToNormalizedString(annotation)); + } + for (Annotation annotation : annotations[i]) { + System.out.println(" Array : " + annotationToNormalizedString(annotation)); + } + + // Exercise Parameter.getAnnotationNative() with + // retrieves a single parameter annotation according to type. + Object[] opaqueClasses = new Object[] {AnnotationA.class, AnnotationB.class}; + for (Object opaqueClass : opaqueClasses) { + @SuppressWarnings("unchecked") + Class<? extends Annotation> annotationClass = + (Class<? extends Annotation>) opaqueClass; + Annotation annotation = parameters[i].getDeclaredAnnotation(annotationClass); + String hasAnnotation = (annotation != null ? "Yes" : "No"); + System.out.println(" " + annotationClass.getName() + " " + hasAnnotation); + + Annotation[] parameterAnnotations = parameters[i].getDeclaredAnnotationsByType(annotationClass); + for (Annotation parameterAnnotation : parameterAnnotations) { + System.out.println(" " + annotationToNormalizedString(parameterAnnotation)); + } + } + } + } + } + + private Class<?> getLocalClassWithEnclosingInstanceCapture() { + class LocalClass { + private final int integerValue; + + LocalClass(@AnnotationA int integerValue) { + this.integerValue = integerValue; + } + } + return LocalClass.class; + } + + private Class<?> getLocalClassWithEnclosingInstanceAndLocalCapture() { + final long CAPTURED_VALUE = System.currentTimeMillis(); + class LocalClassWithCapture { + private final String value; + private final long capturedValue; + + LocalClassWithCapture(@AnnotationA String p1) { + this.value = p1; + this.capturedValue = CAPTURED_VALUE; + } + } + return LocalClassWithCapture.class; + } + + public static void main(String[] args) throws Throwable { + // A local class declared in a static context (0 implicit parameters). + class LocalClassStaticContext { + private final int value; + + LocalClassStaticContext(@AnnotationA int p0) { + this.value = p0; + } + } + + final long CAPTURED_VALUE = System.currentTimeMillis(); + // A local class declared in a static context with a capture (1 implicit parameters). + class LocalClassStaticContextWithCapture { + private final long capturedValue; + private final String argumentValue; + + LocalClassStaticContextWithCapture(@AnnotationA String p1) { + this.capturedValue = CAPTURED_VALUE; + this.argumentValue = p1; + } + } + + // Another local class declared in a static context with a capture (1 implicit parameters). + class LocalClassStaticContextWithCaptureAlternateOrdering { + private final String argumentValue; + private final long capturedValue; + + LocalClassStaticContextWithCaptureAlternateOrdering(@AnnotationA String p1) { + this.argumentValue = p1; + this.capturedValue = CAPTURED_VALUE; + } + } + + DumpConstructorParameterAnnotations(Main.class); + DumpConstructorParameterAnnotations(LocalClassStaticContext.class); + DumpConstructorParameterAnnotations(LocalClassStaticContextWithCapture.class); + DumpConstructorParameterAnnotations(LocalClassStaticContextWithCaptureAlternateOrdering.class); + Main m = new Main(); + DumpConstructorParameterAnnotations(m.getLocalClassWithEnclosingInstanceCapture()); + DumpConstructorParameterAnnotations(m.getLocalClassWithEnclosingInstanceAndLocalCapture()); + DumpConstructorParameterAnnotations(Inner.class); + DumpConstructorParameterAnnotations(StaticInner.class); + DumpConstructorParameterAnnotations(ImportantNumber.class); + DumpConstructorParameterAnnotations(BinaryNumber.class); + DumpConstructorParameterAnnotations(new AnonymousBase("") {}.getClass()); + } +} diff --git a/test/etc/default-build b/test/etc/default-build index 4ed2af6ac6..3e6577cfda 100755 --- a/test/etc/default-build +++ b/test/etc/default-build @@ -137,12 +137,14 @@ declare -A JAVAC_EXPERIMENTAL_ARGS JAVAC_EXPERIMENTAL_ARGS["default-methods"]="-source 1.8 -target 1.8" JAVAC_EXPERIMENTAL_ARGS["lambdas"]="-source 1.8 -target 1.8" JAVAC_EXPERIMENTAL_ARGS["method-handles"]="-source 1.8 -target 1.8" +JAVAC_EXPERIMENTAL_ARGS["parameter-annotations"]="-source 1.8 -target 1.8" JAVAC_EXPERIMENTAL_ARGS["var-handles"]="-source 1.8 -target 1.8" JAVAC_EXPERIMENTAL_ARGS[${DEFAULT_EXPERIMENT}]="-source 1.8 -target 1.8" JAVAC_EXPERIMENTAL_ARGS["agents"]="-source 1.8 -target 1.8" declare -A DX_EXPERIMENTAL_ARGS DX_EXPERIMENTAL_ARGS["method-handles"]="--min-sdk-version=26" +DX_EXPERIMENTAL_ARGS["parameter-annotations"]="--min-sdk-version=25" DX_EXPERIMENTAL_ARGS["var-handles"]="--min-sdk-version=26" while true; do |