diff options
36 files changed, 1099 insertions, 98 deletions
diff --git a/build/Android.gtest.mk b/build/Android.gtest.mk index ed34a8df5f..f27db0eed9 100644 --- a/build/Android.gtest.mk +++ b/build/Android.gtest.mk @@ -170,6 +170,12 @@ ART_GTEST_dex2oat_test_TARGET_DEPS := \ # TODO: document why this is needed. ART_GTEST_proxy_test_HOST_DEPS := $(HOST_CORE_IMAGE_DEFAULT_64) $(HOST_CORE_IMAGE_DEFAULT_32) +# The dexdiag test requires the dexdiag utility. +ART_GTEST_dexdiag_test_HOST_DEPS := \ + $(HOST_OUT_EXECUTABLES)/dexdiag +ART_GTEST_dexdiag_test_TARGET_DEPS := \ + dexdiag + # The dexdump test requires an image and the dexdump utility. # TODO: rename into dexdump when migration completes ART_GTEST_dexdump_test_HOST_DEPS := \ @@ -241,6 +247,7 @@ ART_TEST_MODULES := \ art_compiler_tests \ art_compiler_host_tests \ art_dex2oat_tests \ + art_dexdiag_tests \ art_dexdump_tests \ art_dexlayout_tests \ art_dexlist_tests \ diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc index e823f67d3c..1a4452429e 100644 --- a/compiler/driver/compiler_driver.cc +++ b/compiler/driver/compiler_driver.cc @@ -2181,6 +2181,10 @@ class VerifyClassVisitor : public CompilationVisitor { CHECK(klass->ShouldVerifyAtRuntime() || klass->IsVerified() || klass->IsErroneous()) << klass->PrettyDescriptor() << ": state=" << klass->GetStatus(); + // Class has a meaningful status for the compiler now, record it. + ClassReference ref(manager_->GetDexFile(), class_def_index); + manager_->GetCompiler()->RecordClassStatus(ref, klass->GetStatus()); + // It is *very* problematic if there are verification errors in the boot classpath. For example, // we rely on things working OK without verification when the decryption dialog is brought up. // So abort in a debug build if we find this violated. diff --git a/compiler/driver/compiler_driver_test.cc b/compiler/driver/compiler_driver_test.cc index fa1b3a304a..42ff1e748a 100644 --- a/compiler/driver/compiler_driver_test.cc +++ b/compiler/driver/compiler_driver_test.cc @@ -23,6 +23,7 @@ #include "art_method-inl.h" #include "class_linker-inl.h" #include "common_compiler_test.h" +#include "compiled_class.h" #include "dex_file.h" #include "dex_file_types.h" #include "gc/heap.h" @@ -319,6 +320,47 @@ TEST_F(CompilerDriverProfileTest, ProfileGuidedCompilation) { CheckCompiledMethods(class_loader, "LSecond;", s); } +// Test that a verify only compiler filter updates the CompiledClass map, +// which will be used for OatClass. +class CompilerDriverVerifyTest : public CompilerDriverTest { + protected: + CompilerFilter::Filter GetCompilerFilter() const OVERRIDE { + return CompilerFilter::kVerifyProfile; + } + + void CheckVerifiedClass(jobject class_loader, const std::string& clazz) const { + ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); + Thread* self = Thread::Current(); + ScopedObjectAccess soa(self); + StackHandleScope<1> hs(self); + Handle<mirror::ClassLoader> h_loader( + hs.NewHandle(soa.Decode<mirror::ClassLoader>(class_loader))); + mirror::Class* klass = class_linker->FindClass(self, clazz.c_str(), h_loader); + ASSERT_NE(klass, nullptr); + EXPECT_TRUE(klass->IsVerified()); + + CompiledClass* compiled_class = compiler_driver_->GetCompiledClass( + ClassReference(&klass->GetDexFile(), klass->GetDexTypeIndex().index_)); + ASSERT_NE(compiled_class, nullptr); + EXPECT_EQ(compiled_class->GetStatus(), mirror::Class::kStatusVerified); + } +}; + +TEST_F(CompilerDriverVerifyTest, VerifyCompilation) { + Thread* self = Thread::Current(); + jobject class_loader; + { + ScopedObjectAccess soa(self); + class_loader = LoadDex("ProfileTestMultiDex"); + } + ASSERT_NE(class_loader, nullptr); + + CompileAll(class_loader); + + CheckVerifiedClass(class_loader, "LMain;"); + CheckVerifiedClass(class_loader, "LSecond;"); +} + // TODO: need check-cast test (when stub complete & we can throw/catch } // namespace art diff --git a/compiler/linker/arm64/relative_patcher_arm64.cc b/compiler/linker/arm64/relative_patcher_arm64.cc index 53797d280a..551c73b2a4 100644 --- a/compiler/linker/arm64/relative_patcher_arm64.cc +++ b/compiler/linker/arm64/relative_patcher_arm64.cc @@ -383,9 +383,14 @@ static void EmitGrayCheckAndFastPath(arm64::Arm64Assembler& assembler, static_assert(ReadBarrier::WhiteState() == 0, "Expecting white to have value 0"); static_assert(ReadBarrier::GrayState() == 1, "Expecting gray to have value 1"); __ Tbnz(ip0.W(), LockWord::kReadBarrierStateShift, slow_path); - static_assert(BAKER_MARK_INTROSPECTION_FIELD_LDR_OFFSET == -4, "Check field LDR offset"); - static_assert(BAKER_MARK_INTROSPECTION_ARRAY_LDR_OFFSET == -4, "Check array LDR offset"); - __ Sub(lr, lr, 4); // Adjust the return address one instruction back to the LDR. + static_assert( + BAKER_MARK_INTROSPECTION_ARRAY_LDR_OFFSET == BAKER_MARK_INTROSPECTION_FIELD_LDR_OFFSET, + "Field and array LDR offsets must be the same to reuse the same code."); + // Adjust the return address back to the LDR (1 instruction; 2 for heap poisoning). + static_assert(BAKER_MARK_INTROSPECTION_FIELD_LDR_OFFSET == (kPoisonHeapReferences ? -8 : -4), + "Field LDR must be 1 instruction (4B) before the return address label; " + " 2 instructions (8B) for heap poisoning."); + __ Add(lr, lr, BAKER_MARK_INTROSPECTION_FIELD_LDR_OFFSET); // Introduce a dependency on the lock_word including rb_state, // to prevent load-load reordering, and without using // a memory barrier (which would be more expensive). @@ -431,8 +436,9 @@ std::vector<uint8_t> Arm64RelativePatcher::CompileThunk(const ThunkKey& key) { __ Bind(&slow_path); MemOperand ldr_address(lr, BAKER_MARK_INTROSPECTION_FIELD_LDR_OFFSET); __ Ldr(ip0.W(), ldr_address); // Load the LDR (immediate) unsigned offset. - __ Ubfx(ip0, ip0, 10, 12); // Extract the offset. + __ Ubfx(ip0.W(), ip0.W(), 10, 12); // Extract the offset. __ Ldr(ip0.W(), MemOperand(base_reg, ip0, LSL, 2)); // Load the reference. + // Do not unpoison. With heap poisoning enabled, the entrypoint expects a poisoned reference. __ Br(ip1); // Jump to the entrypoint. if (holder_reg.Is(base_reg)) { // Add null check slow path. The stack map is at the address pointed to by LR. diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc index 4955562f82..4629c54a17 100644 --- a/compiler/optimizing/code_generator_arm64.cc +++ b/compiler/optimizing/code_generator_arm64.cc @@ -90,9 +90,8 @@ static constexpr uint32_t kPackedSwitchCompareJumpThreshold = 7; constexpr uint32_t kReferenceLoadMinFarOffset = 16 * KB; // Flags controlling the use of link-time generated thunks for Baker read barriers. -// Not yet implemented for heap poisoning. -constexpr bool kBakerReadBarrierLinkTimeThunksEnableForFields = !kPoisonHeapReferences; -constexpr bool kBakerReadBarrierLinkTimeThunksEnableForGcRoots = !kPoisonHeapReferences; +constexpr bool kBakerReadBarrierLinkTimeThunksEnableForFields = true; +constexpr bool kBakerReadBarrierLinkTimeThunksEnableForGcRoots = true; // Some instructions have special requirements for a temporary, for example // LoadClass/kBssEntry and LoadString/kBssEntry for Baker read barrier require @@ -3053,6 +3052,11 @@ void InstructionCodeGeneratorARM64::VisitArraySet(HArraySet* instruction) { if (!index.IsConstant()) { __ Add(temp, array, offset); + } else { + // We no longer need the `temp` here so release it as the store below may + // need a scratch register (if the constant index makes the offset too large) + // and the poisoned `source` could be using the other scratch register. + temps.Release(temp); } { // Ensure that between store and MaybeRecordImplicitNullCheck there are no pools emitted. @@ -6093,17 +6097,21 @@ void CodeGeneratorARM64::GenerateFieldLoadWithBakerReadBarrier(HInstruction* ins const int32_t entry_point_offset = CodeGenerator::GetReadBarrierMarkEntryPointsOffset<kArm64PointerSize>(ip0.GetCode()); __ Ldr(ip1, MemOperand(tr, entry_point_offset)); - EmissionCheckScope guard(GetVIXLAssembler(), 3 * vixl::aarch64::kInstructionSize); + EmissionCheckScope guard(GetVIXLAssembler(), + (kPoisonHeapReferences ? 4u : 3u) * vixl::aarch64::kInstructionSize); vixl::aarch64::Label return_address; __ adr(lr, &return_address); __ Bind(cbnz_label); __ cbnz(ip1, static_cast<int64_t>(0)); // Placeholder, patched at link-time. - static_assert(BAKER_MARK_INTROSPECTION_FIELD_LDR_OFFSET == -4, - "Field LDR must be 1 instruction (4B) before the return address label."); - __ ldr(RegisterFrom(ref, Primitive::kPrimNot), MemOperand(base.X(), offset)); + static_assert(BAKER_MARK_INTROSPECTION_FIELD_LDR_OFFSET == (kPoisonHeapReferences ? -8 : -4), + "Field LDR must be 1 instruction (4B) before the return address label; " + " 2 instructions (8B) for heap poisoning."); + Register ref_reg = RegisterFrom(ref, Primitive::kPrimNot); + __ ldr(ref_reg, MemOperand(base.X(), offset)); if (needs_null_check) { MaybeRecordImplicitNullCheck(instruction); } + GetAssembler()->MaybeUnpoisonHeapReference(ref_reg); __ Bind(&return_address); return; } diff --git a/compiler/optimizing/intrinsics_mips64.cc b/compiler/optimizing/intrinsics_mips64.cc index 82d0567ef9..b57b41f686 100644 --- a/compiler/optimizing/intrinsics_mips64.cc +++ b/compiler/optimizing/intrinsics_mips64.cc @@ -2093,6 +2093,199 @@ void IntrinsicCodeGeneratorMIPS64::VisitStringGetCharsNoCheck(HInvoke* invoke) { __ Bind(&done); } +// static void java.lang.System.arraycopy(Object src, int srcPos, +// Object dest, int destPos, +// int length) +void IntrinsicLocationsBuilderMIPS64::VisitSystemArrayCopyChar(HInvoke* invoke) { + HIntConstant* src_pos = invoke->InputAt(1)->AsIntConstant(); + HIntConstant* dest_pos = invoke->InputAt(3)->AsIntConstant(); + HIntConstant* length = invoke->InputAt(4)->AsIntConstant(); + + // As long as we are checking, we might as well check to see if the src and dest + // positions are >= 0. + if ((src_pos != nullptr && src_pos->GetValue() < 0) || + (dest_pos != nullptr && dest_pos->GetValue() < 0)) { + // We will have to fail anyways. + return; + } + + // And since we are already checking, check the length too. + if (length != nullptr) { + int32_t len = length->GetValue(); + if (len < 0) { + // Just call as normal. + return; + } + } + + // Okay, it is safe to generate inline code. + LocationSummary* locations = + new (arena_) LocationSummary(invoke, LocationSummary::kCallOnSlowPath, kIntrinsified); + // arraycopy(Object src, int srcPos, Object dest, int destPos, int length). + locations->SetInAt(0, Location::RequiresRegister()); + locations->SetInAt(1, Location::RegisterOrConstant(invoke->InputAt(1))); + locations->SetInAt(2, Location::RequiresRegister()); + locations->SetInAt(3, Location::RegisterOrConstant(invoke->InputAt(3))); + locations->SetInAt(4, Location::RegisterOrConstant(invoke->InputAt(4))); + + locations->AddTemp(Location::RequiresRegister()); + locations->AddTemp(Location::RequiresRegister()); + locations->AddTemp(Location::RequiresRegister()); +} + +// Utility routine to verify that "length(input) - pos >= length" +static void EnoughItems(Mips64Assembler* assembler, + GpuRegister length_input_minus_pos, + Location length, + SlowPathCodeMIPS64* slow_path) { + if (length.IsConstant()) { + int32_t length_constant = length.GetConstant()->AsIntConstant()->GetValue(); + + if (IsInt<16>(length_constant)) { + __ Slti(TMP, length_input_minus_pos, length_constant); + __ Bnezc(TMP, slow_path->GetEntryLabel()); + } else { + __ LoadConst32(TMP, length_constant); + __ Bltc(length_input_minus_pos, TMP, slow_path->GetEntryLabel()); + } + } else { + __ Bltc(length_input_minus_pos, length.AsRegister<GpuRegister>(), slow_path->GetEntryLabel()); + } +} + +static void CheckPosition(Mips64Assembler* assembler, + Location pos, + GpuRegister input, + Location length, + SlowPathCodeMIPS64* slow_path, + bool length_is_input_length = false) { + // Where is the length in the Array? + const uint32_t length_offset = mirror::Array::LengthOffset().Uint32Value(); + + // Calculate length(input) - pos. + if (pos.IsConstant()) { + int32_t pos_const = pos.GetConstant()->AsIntConstant()->GetValue(); + if (pos_const == 0) { + if (!length_is_input_length) { + // Check that length(input) >= length. + __ LoadFromOffset(kLoadWord, AT, input, length_offset); + EnoughItems(assembler, AT, length, slow_path); + } + } else { + // Check that (length(input) - pos) >= zero. + __ LoadFromOffset(kLoadWord, AT, input, length_offset); + DCHECK_GT(pos_const, 0); + __ Addiu32(AT, AT, -pos_const); + __ Bltzc(AT, slow_path->GetEntryLabel()); + + // Verify that (length(input) - pos) >= length. + EnoughItems(assembler, AT, length, slow_path); + } + } else if (length_is_input_length) { + // The only way the copy can succeed is if pos is zero. + GpuRegister pos_reg = pos.AsRegister<GpuRegister>(); + __ Bnezc(pos_reg, slow_path->GetEntryLabel()); + } else { + // Verify that pos >= 0. + GpuRegister pos_reg = pos.AsRegister<GpuRegister>(); + __ Bltzc(pos_reg, slow_path->GetEntryLabel()); + + // Check that (length(input) - pos) >= zero. + __ LoadFromOffset(kLoadWord, AT, input, length_offset); + __ Subu(AT, AT, pos_reg); + __ Bltzc(AT, slow_path->GetEntryLabel()); + + // Verify that (length(input) - pos) >= length. + EnoughItems(assembler, AT, length, slow_path); + } +} + +void IntrinsicCodeGeneratorMIPS64::VisitSystemArrayCopyChar(HInvoke* invoke) { + Mips64Assembler* assembler = GetAssembler(); + LocationSummary* locations = invoke->GetLocations(); + + GpuRegister src = locations->InAt(0).AsRegister<GpuRegister>(); + Location src_pos = locations->InAt(1); + GpuRegister dest = locations->InAt(2).AsRegister<GpuRegister>(); + Location dest_pos = locations->InAt(3); + Location length = locations->InAt(4); + + Mips64Label loop; + + GpuRegister dest_base = locations->GetTemp(0).AsRegister<GpuRegister>(); + GpuRegister src_base = locations->GetTemp(1).AsRegister<GpuRegister>(); + GpuRegister count = locations->GetTemp(2).AsRegister<GpuRegister>(); + + SlowPathCodeMIPS64* slow_path = new (GetAllocator()) IntrinsicSlowPathMIPS64(invoke); + codegen_->AddSlowPath(slow_path); + + // Bail out if the source and destination are the same (to handle overlap). + __ Beqc(src, dest, slow_path->GetEntryLabel()); + + // Bail out if the source is null. + __ Beqzc(src, slow_path->GetEntryLabel()); + + // Bail out if the destination is null. + __ Beqzc(dest, slow_path->GetEntryLabel()); + + // Load length into register for count. + if (length.IsConstant()) { + __ LoadConst32(count, length.GetConstant()->AsIntConstant()->GetValue()); + } else { + // If the length is negative, bail out. + // We have already checked in the LocationsBuilder for the constant case. + __ Bltzc(length.AsRegister<GpuRegister>(), slow_path->GetEntryLabel()); + + __ Move(count, length.AsRegister<GpuRegister>()); + } + + // Validity checks: source. + CheckPosition(assembler, src_pos, src, Location::RegisterLocation(count), slow_path); + + // Validity checks: dest. + CheckPosition(assembler, dest_pos, dest, Location::RegisterLocation(count), slow_path); + + // If count is zero, we're done. + __ Beqzc(count, slow_path->GetExitLabel()); + + // Okay, everything checks out. Finally time to do the copy. + // Check assumption that sizeof(Char) is 2 (used in scaling below). + const size_t char_size = Primitive::ComponentSize(Primitive::kPrimChar); + DCHECK_EQ(char_size, 2u); + + const size_t char_shift = Primitive::ComponentSizeShift(Primitive::kPrimChar); + + const uint32_t data_offset = mirror::Array::DataOffset(char_size).Uint32Value(); + + // Calculate source and destination addresses. + if (src_pos.IsConstant()) { + int32_t src_pos_const = src_pos.GetConstant()->AsIntConstant()->GetValue(); + + __ Daddiu64(src_base, src, data_offset + char_size * src_pos_const, TMP); + } else { + __ Daddiu64(src_base, src, data_offset, TMP); + __ Dlsa(src_base, src_pos.AsRegister<GpuRegister>(), src_base, char_shift); + } + if (dest_pos.IsConstant()) { + int32_t dest_pos_const = dest_pos.GetConstant()->AsIntConstant()->GetValue(); + + __ Daddiu64(dest_base, dest, data_offset + char_size * dest_pos_const, TMP); + } else { + __ Daddiu64(dest_base, dest, data_offset, TMP); + __ Dlsa(dest_base, dest_pos.AsRegister<GpuRegister>(), dest_base, char_shift); + } + + __ Bind(&loop); + __ Lh(TMP, src_base, 0); + __ Daddiu(src_base, src_base, char_size); + __ Daddiu(count, count, -1); + __ Sh(TMP, dest_base, 0); + __ Daddiu(dest_base, dest_base, char_size); + __ Bnezc(count, &loop); + + __ Bind(slow_path->GetExitLabel()); +} + static void GenHighestOneBit(LocationSummary* locations, Primitive::Type type, Mips64Assembler* assembler) { @@ -2372,7 +2565,6 @@ void IntrinsicCodeGeneratorMIPS64::VisitMathTanh(HInvoke* invoke) { } UNIMPLEMENTED_INTRINSIC(MIPS64, ReferenceGetReferent) -UNIMPLEMENTED_INTRINSIC(MIPS64, SystemArrayCopyChar) UNIMPLEMENTED_INTRINSIC(MIPS64, SystemArrayCopy) UNIMPLEMENTED_INTRINSIC(MIPS64, StringStringIndexOf); diff --git a/compiler/utils/mips64/assembler_mips64.cc b/compiler/utils/mips64/assembler_mips64.cc index 0cff44d830..57223b52a3 100644 --- a/compiler/utils/mips64/assembler_mips64.cc +++ b/compiler/utils/mips64/assembler_mips64.cc @@ -1703,6 +1703,7 @@ void Mips64Assembler::Addiu32(GpuRegister rt, GpuRegister rs, int32_t value) { // TODO: don't use rtmp, use daui, dahi, dati. void Mips64Assembler::Daddiu64(GpuRegister rt, GpuRegister rs, int64_t value, GpuRegister rtmp) { + CHECK_NE(rs, rtmp); if (IsInt<16>(value)) { Daddiu(rt, rs, value); } else { diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc index 81566c40e9..a9108e0eb0 100644 --- a/dex2oat/dex2oat.cc +++ b/dex2oat/dex2oat.cc @@ -385,6 +385,8 @@ NO_RETURN static void Usage(const char* fmt, ...) { UsageError(" This option is incompatible with read barriers (e.g., if dex2oat has been"); UsageError(" built with the environment variable `ART_USE_READ_BARRIER` set to `true`)."); UsageError(""); + UsageError(" --classpath-dir=<directory-path>: directory used to resolve relative class paths."); + UsageError(""); std::cerr << "See log for usage error information\n"; exit(EXIT_FAILURE); } @@ -1234,6 +1236,8 @@ class Dex2Oat FINAL { Usage("Cannot use --force-determinism with read barriers or non-CMS garbage collector"); } force_determinism_ = true; + } else if (option.starts_with("--classpath-dir=")) { + classpath_dir_ = option.substr(strlen("--classpath-dir=")).data(); } else if (!compiler_options_->ParseCompilerOption(option, Usage)) { Usage("Unknown argument %s", option.data()); } @@ -1486,12 +1490,13 @@ class Dex2Oat FINAL { } // Open dex files for class path. - const std::vector<std::string> class_path_locations = + std::vector<std::string> class_path_locations = GetClassPathLocations(runtime_->GetClassPathString()); OpenClassPathFiles(class_path_locations, &class_path_files_, &opened_oat_files_, - runtime_->GetInstructionSet()); + runtime_->GetInstructionSet(), + classpath_dir_); // Store the classpath we have right now. std::vector<const DexFile*> class_path_files = MakeNonOwningPointerVector(class_path_files_); @@ -1501,7 +1506,7 @@ class Dex2Oat FINAL { // When passing the special shared library as the classpath, it is the only path. encoded_class_path = OatFile::kSpecialSharedLibrary; } else { - encoded_class_path = OatFile::EncodeDexFileDependencies(class_path_files); + encoded_class_path = OatFile::EncodeDexFileDependencies(class_path_files, classpath_dir_); } key_value_store_->Put(OatHeader::kClassPathKey, encoded_class_path); } @@ -2180,18 +2185,23 @@ class Dex2Oat FINAL { // Opens requested class path files and appends them to opened_dex_files. If the dex files have // been stripped, this opens them from their oat files and appends them to opened_oat_files. - static void OpenClassPathFiles(const std::vector<std::string>& class_path_locations, + static void OpenClassPathFiles(std::vector<std::string>& class_path_locations, std::vector<std::unique_ptr<const DexFile>>* opened_dex_files, std::vector<std::unique_ptr<OatFile>>* opened_oat_files, - InstructionSet isa) { + InstructionSet isa, + std::string& classpath_dir) { DCHECK(opened_dex_files != nullptr) << "OpenClassPathFiles dex out-param is nullptr"; DCHECK(opened_oat_files != nullptr) << "OpenClassPathFiles oat out-param is nullptr"; - for (const std::string& location : class_path_locations) { + for (std::string& location : class_path_locations) { // Stop early if we detect the special shared library, which may be passed as the classpath // for dex2oat when we want to skip the shared libraries check. if (location == OatFile::kSpecialSharedLibrary) { break; } + // If path is relative, append it to the provided base directory. + if (!classpath_dir.empty() && location[0] != '/') { + location = classpath_dir + '/' + location; + } static constexpr bool kVerifyChecksum = true; std::string error_msg; if (!DexFile::Open( @@ -2743,6 +2753,9 @@ class Dex2Oat FINAL { // See CompilerOptions.force_determinism_. bool force_determinism_; + // Directory of relative classpaths. + std::string classpath_dir_; + // Whether the given input vdex is also the output. bool update_input_vdex_ = false; diff --git a/dexlayout/Android.bp b/dexlayout/Android.bp index 4b65c5299a..16e6809f4a 100644 --- a/dexlayout/Android.bp +++ b/dexlayout/Android.bp @@ -20,7 +20,7 @@ art_cc_defaults { "dexlayout.cc", "dex_ir.cc", "dex_ir_builder.cc", - "dex_verify.cc", + "dex_verify.cc", "dex_visualize.cc", "dex_writer.cc", ], @@ -43,6 +43,7 @@ art_cc_library { art_cc_binary { name: "dexlayout", + defaults: ["art_defaults"], host_supported: true, srcs: ["dexlayout_main.cc"], cflags: ["-Wall"], @@ -60,13 +61,28 @@ art_cc_test { art_cc_binary { name: "dexdiag", - host_supported: false, + defaults: ["art_defaults"], + host_supported: true, srcs: ["dexdiag.cc"], cflags: ["-Wall"], shared_libs: [ "libart", "libart-dexlayout", - "libpagemap", ], + target: { + android: { + shared_libs: [ + "libpagemap", + ] + }, + } } +art_cc_test { + name: "art_dexdiag_tests", + host_supported: true, + defaults: [ + "art_gtest_defaults", + ], + srcs: ["dexdiag_test.cc"], +} diff --git a/dexlayout/dexdiag.cc b/dexlayout/dexdiag.cc index 688201b6b8..bc56bbfa81 100644 --- a/dexlayout/dexdiag.cc +++ b/dexlayout/dexdiag.cc @@ -15,6 +15,7 @@ */ #include <errno.h> +#include <inttypes.h> #include <stdint.h> #include <stdlib.h> #include <string.h> @@ -30,7 +31,9 @@ #include "dex_file.h" #include "dex_ir.h" #include "dex_ir_builder.h" +#ifdef ART_TARGET_ANDROID #include "pagemap/pagemap.h" +#endif #include "runtime.h" #include "vdex_file.h" @@ -38,8 +41,6 @@ namespace art { using android::base::StringPrintf; -static constexpr size_t kLineLength = 32; - static bool g_verbose = false; // The width needed to print a file page offset (32-bit). @@ -164,6 +165,7 @@ static void PrintLetterKey() { std::cout << ". (Mapped page not resident)" << std::endl; } +#ifdef ART_TARGET_ANDROID static char PageTypeChar(uint16_t type) { if (kDexSectionInfoMap.find(type) == kDexSectionInfoMap.end()) { return '-'; @@ -194,6 +196,7 @@ static void ProcessPageMap(uint64_t* pagemap, size_t end, const std::vector<dex_ir::DexFileSection>& sections, PageCount* page_counts) { + static constexpr size_t kLineLength = 32; for (size_t page = start; page < end; ++page) { char type_char = '.'; if (PM_PAGEMAP_PRESENT(pagemap[page])) { @@ -268,7 +271,7 @@ static void ProcessOneDexMapping(uint64_t* pagemap, std::cerr << "Dex file start offset for " << dex_file->GetLocation().c_str() << " is incorrect: map start " - << StringPrintf("%zx > dex start %zx\n", map_start, dex_file_start) + << StringPrintf("%" PRIx64 " > dex start %" PRIx64 "\n", map_start, dex_file_start) << std::endl; return; } @@ -277,7 +280,7 @@ static void ProcessOneDexMapping(uint64_t* pagemap, uint64_t end_page = RoundUp(start_address + dex_file_size, kPageSize) / kPageSize; std::cout << "DEX " << dex_file->GetLocation().c_str() - << StringPrintf(": %zx-%zx", + << StringPrintf(": %" PRIx64 "-%" PRIx64, map_start + start_page * kPageSize, map_start + end_page * kPageSize) << std::endl; @@ -341,7 +344,7 @@ static bool DisplayMappingIfFromVdexFile(pm_map_t* map, Printer* printer) { // Process the dex files. std::cout << "MAPPING " << pm_map_name(map) - << StringPrintf(": %zx-%zx", pm_map_start(map), pm_map_end(map)) + << StringPrintf(": %" PRIx64 "-%" PRIx64, pm_map_start(map), pm_map_end(map)) << std::endl; for (const auto& dex_file : dex_files) { ProcessOneDexMapping(pagemap, @@ -355,6 +358,7 @@ static bool DisplayMappingIfFromVdexFile(pm_map_t* map, Printer* printer) { } static void ProcessOneOatMapping(uint64_t* pagemap, size_t size, Printer* printer) { + static constexpr size_t kLineLength = 32; size_t resident_page_count = 0; for (size_t page = 0; page < size; ++page) { char type_char = '.'; @@ -405,7 +409,7 @@ static bool DisplayMappingIfFromOatFile(pm_map_t* map, Printer* printer) { // Process the dex files. std::cout << "MAPPING " << pm_map_name(map) - << StringPrintf(": %zx-%zx", pm_map_start(map), pm_map_end(map)) + << StringPrintf(": %" PRIx64 "-%" PRIx64, pm_map_start(map), pm_map_end(map)) << std::endl; ProcessOneOatMapping(pagemap, len, printer); free(pagemap); @@ -425,9 +429,10 @@ static bool FilterByNameContains(const std::string& mapped_file_name, } return false; } +#endif static void Usage(const char* cmd) { - std::cerr << "Usage: " << cmd << " [options] pid" << std::endl + std::cout << "Usage: " << cmd << " [options] pid" << std::endl << " --contains=<string>: Display sections containing string." << std::endl << " --help: Shows this message." << std::endl << " --verbose: Makes displays verbose." << std::endl; @@ -462,6 +467,7 @@ static int DexDiagMain(int argc, char* argv[]) { InitLogging(argv, Runtime::Aborter); MemMap::Init(); +#ifdef ART_TARGET_ANDROID pid_t pid; char* endptr; pid = (pid_t)strtol(argv[argc - 1], &endptr, 10); @@ -495,7 +501,7 @@ static int DexDiagMain(int argc, char* argv[]) { return EXIT_FAILURE; } - // Process the mappings that are due to DEX files. + // Process the mappings that are due to vdex or oat files. Printer printer; for (size_t i = 0; i < num_maps; ++i) { std::string mapped_file_name = pm_map_name(maps[i]); @@ -509,6 +515,7 @@ static int DexDiagMain(int argc, char* argv[]) { return EXIT_FAILURE; } } +#endif return EXIT_SUCCESS; } diff --git a/dexlayout/dexdiag_test.cc b/dexlayout/dexdiag_test.cc new file mode 100644 index 0000000000..dfe42b20fe --- /dev/null +++ b/dexlayout/dexdiag_test.cc @@ -0,0 +1,155 @@ +/* + * 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. + */ + +#include <string> +#include <vector> + +#include "common_runtime_test.h" + +#include "runtime/exec_utils.h" +#include "runtime/oat_file.h" +#include "runtime/os.h" + +namespace art { + +static const char* kDexDiagContains = "--contains=boot.vdex"; +static const char* kDexDiagContainsFails = "--contains=anything_other_than_boot.vdex"; +static const char* kDexDiagHelp = "--help"; +static const char* kDexDiagVerbose = "--verbose"; +static const char* kDexDiagBinaryName = "dexdiag"; + +class DexDiagTest : public CommonRuntimeTest { + protected: + virtual void SetUp() { + CommonRuntimeTest::SetUp(); + } + + // Path to the dexdiag(d?)[32|64] binary. + std::string GetDexDiagFilePath() { + std::string root = GetTestAndroidRoot(); + + root += "/bin/"; + root += kDexDiagBinaryName; + + std::string root32 = root + "32"; + // If we have both a 32-bit and a 64-bit build, the 32-bit file will have a 32 suffix. + if (OS::FileExists(root32.c_str()) && !Is64BitInstructionSet(kRuntimeISA)) { + return root32; + } else { + // This is a 64-bit build or only a single build exists. + return root; + } + } + + void OpenOatAndVdexFiles() { + // Open the boot.oat file. + // This is a little convoluted because we have to + // get the location of the default boot image (/system/framework/boot.art), + // find it in the right architecture subdirectory (/system/framework/arm/boot.art), + // find the oat file next to the image (/system/framework/arm/boot.oat). + // Then, opening the oat file has the side-effect of opening the corresponding + // vdex file (/system/framework/arm/boot.vdex). + std::string error_msg; + const std::string default_location = GetDefaultBootImageLocation(&error_msg); + EXPECT_TRUE(!default_location.empty()) << error_msg; + std::string oat_location = GetSystemImageFilename(default_location.c_str(), kRuntimeISA); + EXPECT_TRUE(!oat_location.empty()); + const std::string kImageFileSuffix = ".art"; + size_t suffix_pos = oat_location.rfind(kImageFileSuffix); + EXPECT_TRUE(suffix_pos != std::string::npos); + const std::string kOatFileSuffix = ".oat"; + oat_location = oat_location.replace(suffix_pos, kImageFileSuffix.length(), kOatFileSuffix); + std::unique_ptr<OatFile> oat(OatFile::Open(oat_location.c_str(), + oat_location.c_str(), + nullptr, + nullptr, + false, + /*low_4gb*/false, + nullptr, + &error_msg)); + EXPECT_TRUE(oat != nullptr) << error_msg; + } + + // Run dexdiag with a custom boot image location. + bool Exec(pid_t this_pid, const std::vector<std::string>& args, std::string* error_msg) { + // Invoke 'dexdiag' against the current process. + // This should succeed because we have a runtime and so it should + // be able to map in the boot.art and do a diff for it. + std::vector<std::string> exec_argv; + + // Build the command line "dexdiag <args> this_pid". + std::string executable_path = GetDexDiagFilePath(); + EXPECT_TRUE(OS::FileExists(executable_path.c_str())) << executable_path + << " should be a valid file path"; + exec_argv.push_back(executable_path); + for (const auto& arg : args) { + exec_argv.push_back(arg); + } + exec_argv.push_back(std::to_string(this_pid)); + + return ::art::Exec(exec_argv, error_msg); + } +}; + +// We can't run these tests on the host, as they will fail when trying to open +// /proc/pid/pagemap. +// On the target, we invoke 'dexdiag' against the current process. +// This should succeed because we have a runtime and so dexdiag should +// be able to find the map for, e.g., boot.vdex and friends. +TEST_F(DexDiagTest, DexDiagHelpTest) { + // TODO: test the resulting output. + std::string error_msg; + ASSERT_TRUE(Exec(getpid(), { kDexDiagHelp }, &error_msg)) << "Failed to execute -- because: " + << error_msg; +} + +#if defined (ART_TARGET) +TEST_F(DexDiagTest, DexDiagContainsTest) { +#else +TEST_F(DexDiagTest, DISABLED_DexDiagContainsTest) { +#endif + OpenOatAndVdexFiles(); + // TODO: test the resulting output. + std::string error_msg; + ASSERT_TRUE(Exec(getpid(), { kDexDiagContains }, &error_msg)) << "Failed to execute -- because: " + << error_msg; +} + +#if defined (ART_TARGET) +TEST_F(DexDiagTest, DexDiagContainsFailsTest) { +#else +TEST_F(DexDiagTest, DISABLED_DexDiagContainsFailsTest) { +#endif + OpenOatAndVdexFiles(); + // TODO: test the resulting output. + std::string error_msg; + ASSERT_TRUE(Exec(getpid(), { kDexDiagContainsFails }, &error_msg)) + << "Failed to execute -- because: " + << error_msg; +} + +#if defined (ART_TARGET) +TEST_F(DexDiagTest, DexDiagVerboseTest) { +#else +TEST_F(DexDiagTest, DISABLED_DexDiagVerboseTest) { +#endif + // TODO: test the resulting output. + std::string error_msg; + ASSERT_TRUE(Exec(getpid(), { kDexDiagVerbose }, &error_msg)) << "Failed to execute -- because: " + << error_msg; +} + +} // namespace art diff --git a/runtime/arch/arm64/asm_support_arm64.h b/runtime/arch/arm64/asm_support_arm64.h index cfcd6a7e00..6b7720023e 100644 --- a/runtime/arch/arm64/asm_support_arm64.h +++ b/runtime/arch/arm64/asm_support_arm64.h @@ -32,9 +32,17 @@ #define BAKER_MARK_INTROSPECTION_GC_ROOT_ENTRYPOINT_OFFSET 0x300 // The offset of the reference load LDR from the return address in LR for field loads. +#ifdef USE_HEAP_POISONING +#define BAKER_MARK_INTROSPECTION_FIELD_LDR_OFFSET -8 +#else #define BAKER_MARK_INTROSPECTION_FIELD_LDR_OFFSET -4 +#endif // The offset of the reference load LDR from the return address in LR for array loads. +#ifdef USE_HEAP_POISONING +#define BAKER_MARK_INTROSPECTION_ARRAY_LDR_OFFSET -8 +#else #define BAKER_MARK_INTROSPECTION_ARRAY_LDR_OFFSET -4 +#endif // The offset of the reference load LDR from the return address in LR for GC root loads. #define BAKER_MARK_INTROSPECTION_GC_ROOT_LDR_OFFSET -8 diff --git a/runtime/arch/arm64/quick_entrypoints_arm64.S b/runtime/arch/arm64/quick_entrypoints_arm64.S index c7fa7f5d2b..d043962b96 100644 --- a/runtime/arch/arm64/quick_entrypoints_arm64.S +++ b/runtime/arch/arm64/quick_entrypoints_arm64.S @@ -2649,7 +2649,8 @@ READ_BARRIER_MARK_REG art_quick_read_barrier_mark_reg29, w29, x29 * * For field accesses and array loads with a constant index the thunk loads * the reference into IP0 using introspection and calls the main entrypoint, - * art_quick_read_barrier_mark_introspection. + * art_quick_read_barrier_mark_introspection. With heap poisoning enabled, + * the passed reference is poisoned. * * For array accesses with non-constant index, the thunk inserts the bits * 16-21 of the LDR instruction to the entrypoint address, effectively @@ -2663,6 +2664,7 @@ READ_BARRIER_MARK_REG art_quick_read_barrier_mark_reg29, w29, x29 * * For GC root accesses we cannot use the main entrypoint because of the * different offset where the LDR instruction in generated code is located. + * (And even with heap poisoning enabled, GC roots are not poisoned.) * To re-use the same entrypoint pointer in generated code, we make sure * that the gc root entrypoint (a copy of the entrypoint with a different * offset for introspection loads) is located at a known offset (768 bytes, @@ -2686,6 +2688,8 @@ READ_BARRIER_MARK_REG art_quick_read_barrier_mark_reg29, w29, x29 .balign 512 ENTRY art_quick_read_barrier_mark_introspection // At this point, IP0 contains the reference, IP1 can be freely used. + // For heap poisoning, the reference is poisoned, so unpoison it first. + UNPOISON_HEAP_REF wIP0 // If reference is null, just return it in the right register. cbz wIP0, .Lmark_introspection_return // Use wIP1 as temp and check the mark bit of the reference. diff --git a/runtime/debugger.cc b/runtime/debugger.cc index 039b60a26d..63794bff6f 100644 --- a/runtime/debugger.cc +++ b/runtime/debugger.cc @@ -2425,7 +2425,9 @@ JDWP::JdwpError Dbg::SuspendThread(JDWP::ObjectId thread_id, bool request_suspen // Suspend thread to build stack trace. bool timed_out; ThreadList* thread_list = Runtime::Current()->GetThreadList(); - Thread* thread = thread_list->SuspendThreadByPeer(peer.get(), request_suspension, true, + Thread* thread = thread_list->SuspendThreadByPeer(peer.get(), + request_suspension, + /* debug_suspension */ true, &timed_out); if (thread != nullptr) { return JDWP::ERR_NONE; @@ -3669,7 +3671,10 @@ class ScopedDebuggerThreadSuspension { jobject thread_peer = Dbg::GetObjectRegistry()->GetJObject(thread_id); bool timed_out; ThreadList* const thread_list = Runtime::Current()->GetThreadList(); - suspended_thread = thread_list->SuspendThreadByPeer(thread_peer, true, true, &timed_out); + suspended_thread = thread_list->SuspendThreadByPeer(thread_peer, + /* request_suspension */ true, + /* debug_suspension */ true, + &timed_out); } if (suspended_thread == nullptr) { // Thread terminated from under us while suspending. diff --git a/runtime/oat_file.cc b/runtime/oat_file.cc index 493da271d1..a00674a9fe 100644 --- a/runtime/oat_file.cc +++ b/runtime/oat_file.cc @@ -1497,11 +1497,18 @@ CompilerFilter::Filter OatFile::GetCompilerFilter() const { static constexpr char kDexClassPathEncodingSeparator = '*'; -std::string OatFile::EncodeDexFileDependencies(const std::vector<const DexFile*>& dex_files) { +std::string OatFile::EncodeDexFileDependencies(const std::vector<const DexFile*>& dex_files, + std::string& base_dir) { std::ostringstream out; for (const DexFile* dex_file : dex_files) { - out << dex_file->GetLocation().c_str(); + const std::string& location = dex_file->GetLocation(); + // Find paths that were relative and convert them back from absolute. + if (!base_dir.empty() && location.substr(0, base_dir.length()) == base_dir) { + out << location.substr(base_dir.length() + 1).c_str(); + } else { + out << dex_file->GetLocation().c_str(); + } out << kDexClassPathEncodingSeparator; out << dex_file->GetLocationChecksum(); out << kDexClassPathEncodingSeparator; diff --git a/runtime/oat_file.h b/runtime/oat_file.h index d24283afee..06c76b5464 100644 --- a/runtime/oat_file.h +++ b/runtime/oat_file.h @@ -288,7 +288,9 @@ class OatFile { const char* abs_dex_location, const std::string& rel_dex_location); // Create a dependency list (dex locations and checksums) for the given dex files. - static std::string EncodeDexFileDependencies(const std::vector<const DexFile*>& dex_files); + // Removes dex file paths prefixed with base_dir to convert them back to relative paths. + static std::string EncodeDexFileDependencies(const std::vector<const DexFile*>& dex_files, + std::string& base_dir); // Finds the associated oat class for a dex_file and descriptor. Returns an invalid OatClass on // error and sets found to false. diff --git a/runtime/oat_file_manager.cc b/runtime/oat_file_manager.cc index a950980e6b..139022210c 100644 --- a/runtime/oat_file_manager.cc +++ b/runtime/oat_file_manager.cc @@ -440,8 +440,12 @@ static bool AreSharedLibrariesOk(const std::string& shared_libraries, return false; } + // Check that the loaded dex files have the same order and checksums as the shared libraries. for (size_t i = 0; i < dex_files.size(); ++i) { - if (dex_files[i]->GetLocation() != shared_libraries_split[i * 2]) { + std::string absolute_library_path = + OatFile::ResolveRelativeEncodedDexLocation(dex_files[i]->GetLocation().c_str(), + shared_libraries_split[i * 2]); + if (dex_files[i]->GetLocation() != absolute_library_path) { return false; } char* end; diff --git a/runtime/openjdkjvmti/ti_redefine.cc b/runtime/openjdkjvmti/ti_redefine.cc index 06550791c1..358bb0f70e 100644 --- a/runtime/openjdkjvmti/ti_redefine.cc +++ b/runtime/openjdkjvmti/ti_redefine.cc @@ -1418,14 +1418,18 @@ void Redefiner::ClassRedefinition::RestoreObsoleteMethodMapsIfUnneeded( art::mirror::Class* klass = GetMirrorClass(); art::mirror::ClassExt* ext = klass->GetExtData(); art::mirror::PointerArray* methods = ext->GetObsoleteMethods(); - int32_t old_length = - cur_data->GetOldDexCaches() == nullptr ? 0 : cur_data->GetOldDexCaches()->GetLength(); + art::mirror::PointerArray* old_methods = cur_data->GetOldObsoleteMethods(); + int32_t old_length = old_methods == nullptr ? 0 : old_methods->GetLength(); int32_t expected_length = old_length + klass->NumDirectMethods() + klass->NumDeclaredVirtualMethods(); // Check to make sure we are only undoing this one. if (expected_length == methods->GetLength()) { - for (int32_t i = old_length; i < expected_length; i++) { - if (methods->GetElementPtrSize<art::ArtMethod*>(i, art::kRuntimePointerSize) != nullptr) { + for (int32_t i = 0; i < expected_length; i++) { + art::ArtMethod* expected = nullptr; + if (i < old_length) { + expected = old_methods->GetElementPtrSize<art::ArtMethod*>(i, art::kRuntimePointerSize); + } + if (methods->GetElementPtrSize<art::ArtMethod*>(i, art::kRuntimePointerSize) != expected) { // We actually have some new obsolete methods. Just abort since we cannot safely shrink the // obsolete methods array. return; diff --git a/runtime/runtime.cc b/runtime/runtime.cc index e563027243..a48a58d235 100644 --- a/runtime/runtime.cc +++ b/runtime/runtime.cc @@ -1722,6 +1722,7 @@ void Runtime::VisitConstantRoots(RootVisitor* visitor) { mirror::MethodHandlesLookup::VisitRoots(visitor); mirror::EmulatedStackFrame::VisitRoots(visitor); mirror::ClassExt::VisitRoots(visitor); + mirror::CallSite::VisitRoots(visitor); // Visit all the primitive array types classes. mirror::PrimitiveArray<uint8_t>::VisitRoots(visitor); // BooleanArray mirror::PrimitiveArray<int8_t>::VisitRoots(visitor); // ByteArray diff --git a/runtime/thread.cc b/runtime/thread.cc index abe65c1de1..a8a03c7e86 100644 --- a/runtime/thread.cc +++ b/runtime/thread.cc @@ -1451,7 +1451,8 @@ void Thread::RequestSynchronousCheckpoint(Closure* function) { MutexLock mu2(self, *Locks::thread_suspend_count_lock_); DCHECK_NE(GetState(), ThreadState::kRunnable); - CHECK(ModifySuspendCount(self, -1, nullptr, false)); + bool updated = ModifySuspendCount(self, -1, nullptr, false); + DCHECK(updated); } return; // We're done, break out of the loop. diff --git a/runtime/thread.h b/runtime/thread.h index de0b892f5f..4d5d644f1c 100644 --- a/runtime/thread.h +++ b/runtime/thread.h @@ -244,6 +244,7 @@ class Thread { int delta, AtomicInteger* suspend_barrier, bool for_debugger) + WARN_UNUSED REQUIRES(Locks::thread_suspend_count_lock_); bool RequestCheckpoint(Closure* function) @@ -1276,6 +1277,7 @@ class Thread { int delta, AtomicInteger* suspend_barrier, bool for_debugger) + WARN_UNUSED REQUIRES(Locks::thread_suspend_count_lock_); void RunCheckpointFunction(); diff --git a/runtime/thread_list.cc b/runtime/thread_list.cc index 8d72fe80c2..2e0d866c21 100644 --- a/runtime/thread_list.cc +++ b/runtime/thread_list.cc @@ -323,7 +323,8 @@ size_t ThreadList::RunCheckpoint(Closure* checkpoint_function, Closure* callback // Spurious fail, try again. continue; } - thread->ModifySuspendCount(self, +1, nullptr, false); + bool updated = thread->ModifySuspendCount(self, +1, nullptr, false); + DCHECK(updated); suspended_count_modified_threads.push_back(thread); break; } @@ -365,7 +366,8 @@ size_t ThreadList::RunCheckpoint(Closure* checkpoint_function, Closure* callback checkpoint_function->Run(thread); { MutexLock mu2(self, *Locks::thread_suspend_count_lock_); - thread->ModifySuspendCount(self, -1, nullptr, false); + bool updated = thread->ModifySuspendCount(self, -1, nullptr, false); + DCHECK(updated); } } @@ -565,7 +567,8 @@ size_t ThreadList::FlipThreadRoots(Closure* thread_flip_visitor, if ((state == kWaitingForGcThreadFlip || thread->IsTransitioningToRunnable()) && thread->GetSuspendCount() == 1) { // The thread will resume right after the broadcast. - thread->ModifySuspendCount(self, -1, nullptr, false); + bool updated = thread->ModifySuspendCount(self, -1, nullptr, false); + DCHECK(updated); ++runnable_thread_count; } else { other_threads.push_back(thread); @@ -598,7 +601,8 @@ size_t ThreadList::FlipThreadRoots(Closure* thread_flip_visitor, TimingLogger::ScopedTiming split4("ResumeOtherThreads", collector->GetTimings()); MutexLock mu2(self, *Locks::thread_suspend_count_lock_); for (const auto& thread : other_threads) { - thread->ModifySuspendCount(self, -1, nullptr, false); + bool updated = thread->ModifySuspendCount(self, -1, nullptr, false); + DCHECK(updated); } Thread::resume_cond_->Broadcast(self); } @@ -708,7 +712,8 @@ void ThreadList::SuspendAllInternal(Thread* self, continue; } VLOG(threads) << "requesting thread suspend: " << *thread; - thread->ModifySuspendCount(self, +1, &pending_threads, debug_suspend); + bool updated = thread->ModifySuspendCount(self, +1, &pending_threads, debug_suspend); + DCHECK(updated); // Must install the pending_threads counter first, then check thread->IsSuspend() and clear // the counter. Otherwise there's a race with Thread::TransitionFromRunnableToSuspended() @@ -786,7 +791,8 @@ void ThreadList::ResumeAll() { if (thread == self) { continue; } - thread->ModifySuspendCount(self, -1, nullptr, false); + bool updated = thread->ModifySuspendCount(self, -1, nullptr, false); + DCHECK(updated); } // Broadcast a notification to all suspended threads, some or all of @@ -828,7 +834,8 @@ void ThreadList::Resume(Thread* thread, bool for_debugger) { << ") thread not within thread list"; return; } - thread->ModifySuspendCount(self, -1, nullptr, for_debugger); + bool updated = thread->ModifySuspendCount(self, -1, nullptr, for_debugger); + DCHECK(updated); } { @@ -884,7 +891,11 @@ Thread* ThreadList::SuspendThreadByPeer(jobject peer, // If we incremented the suspend count but the thread reset its peer, we need to // re-decrement it since it is shutting down and may deadlock the runtime in // ThreadList::WaitForOtherNonDaemonThreadsToExit. - suspended_thread->ModifySuspendCount(soa.Self(), -1, nullptr, debug_suspension); + bool updated = suspended_thread->ModifySuspendCount(soa.Self(), + -1, + nullptr, + debug_suspension); + DCHECK(updated); } ThreadSuspendByPeerWarning(self, ::android::base::WARNING, @@ -910,7 +921,8 @@ Thread* ThreadList::SuspendThreadByPeer(jobject peer, } CHECK(suspended_thread == nullptr); suspended_thread = thread; - suspended_thread->ModifySuspendCount(self, +1, nullptr, debug_suspension); + bool updated = suspended_thread->ModifySuspendCount(self, +1, nullptr, debug_suspension); + DCHECK(updated); request_suspension = false; } else { // If the caller isn't requesting suspension, a suspension should have already occurred. @@ -942,7 +954,11 @@ Thread* ThreadList::SuspendThreadByPeer(jobject peer, peer); if (suspended_thread != nullptr) { CHECK_EQ(suspended_thread, thread); - suspended_thread->ModifySuspendCount(soa.Self(), -1, nullptr, debug_suspension); + bool updated = suspended_thread->ModifySuspendCount(soa.Self(), + -1, + nullptr, + debug_suspension); + DCHECK(updated); } *timed_out = true; return nullptr; @@ -1015,7 +1031,8 @@ Thread* ThreadList::SuspendThreadByThreadId(uint32_t thread_id, // which will allow this thread to be suspended. continue; } - thread->ModifySuspendCount(self, +1, nullptr, debug_suspension); + bool updated = thread->ModifySuspendCount(self, +1, nullptr, debug_suspension); + DCHECK(updated); suspended_thread = thread; } else { CHECK_EQ(suspended_thread, thread); @@ -1046,7 +1063,8 @@ Thread* ThreadList::SuspendThreadByThreadId(uint32_t thread_id, "Thread suspension timed out", thread_id); if (suspended_thread != nullptr) { - thread->ModifySuspendCount(soa.Self(), -1, nullptr, debug_suspension); + bool updated = thread->ModifySuspendCount(soa.Self(), -1, nullptr, debug_suspension); + DCHECK(updated); } *timed_out = true; return nullptr; @@ -1123,7 +1141,8 @@ void ThreadList::SuspendSelfForDebugger() { // to ensure that we're the only one fiddling with the suspend count // though. MutexLock mu(self, *Locks::thread_suspend_count_lock_); - self->ModifySuspendCount(self, +1, nullptr, true); + bool updated = self->ModifySuspendCount(self, +1, nullptr, true); + DCHECK(updated); CHECK_GT(self->GetSuspendCount(), 0); VLOG(threads) << *self << " self-suspending (debugger)"; @@ -1207,7 +1226,8 @@ void ThreadList::ResumeAllForDebugger() { continue; } VLOG(threads) << "requesting thread resume: " << *thread; - thread->ModifySuspendCount(self, -1, nullptr, true); + bool updated = thread->ModifySuspendCount(self, -1, nullptr, true); + DCHECK(updated); } } } @@ -1236,7 +1256,11 @@ void ThreadList::UndoDebuggerSuspensions() { if (thread == self || thread->GetDebugSuspendCount() == 0) { continue; } - thread->ModifySuspendCount(self, -thread->GetDebugSuspendCount(), nullptr, true); + bool suspended = thread->ModifySuspendCount(self, + -thread->GetDebugSuspendCount(), + nullptr, + true); + DCHECK(suspended); } } @@ -1293,7 +1317,8 @@ void ThreadList::SuspendAllDaemonThreadsForShutdown() { // daemons. CHECK(thread->IsDaemon()) << *thread; if (thread != self) { - thread->ModifySuspendCount(self, +1, nullptr, false); + bool updated = thread->ModifySuspendCount(self, +1, nullptr, false); + DCHECK(updated); ++daemons_left; } // We are shutting down the runtime, set the JNI functions of all the JNIEnvs to be @@ -1352,10 +1377,12 @@ void ThreadList::Register(Thread* self) { // Modify suspend count in increments of 1 to maintain invariants in ModifySuspendCount. While // this isn't particularly efficient the suspend counts are most commonly 0 or 1. for (int delta = debug_suspend_all_count_; delta > 0; delta--) { - self->ModifySuspendCount(self, +1, nullptr, true); + bool updated = self->ModifySuspendCount(self, +1, nullptr, true); + DCHECK(updated); } for (int delta = suspend_all_count_ - debug_suspend_all_count_; delta > 0; delta--) { - self->ModifySuspendCount(self, +1, nullptr, false); + bool updated = self->ModifySuspendCount(self, +1, nullptr, false); + DCHECK(updated); } CHECK(!Contains(self)); list_.push_back(self); @@ -1450,11 +1477,13 @@ void ThreadList::VisitRootsForSuspendedThreads(RootVisitor* visitor) { MutexLock mu(self, *Locks::thread_list_lock_); MutexLock mu2(self, *Locks::thread_suspend_count_lock_); for (Thread* thread : list_) { - thread->ModifySuspendCount(self, +1, nullptr, false); + bool suspended = thread->ModifySuspendCount(self, +1, nullptr, false); + DCHECK(suspended); if (thread == self || thread->IsSuspended()) { threads_to_visit.push_back(thread); } else { - thread->ModifySuspendCount(self, -1, nullptr, false); + bool resumed = thread->ModifySuspendCount(self, -1, nullptr, false); + DCHECK(resumed); } } } @@ -1469,7 +1498,8 @@ void ThreadList::VisitRootsForSuspendedThreads(RootVisitor* visitor) { { MutexLock mu2(self, *Locks::thread_suspend_count_lock_); for (Thread* thread : threads_to_visit) { - thread->ModifySuspendCount(self, -1, nullptr, false); + bool updated = thread->ModifySuspendCount(self, -1, nullptr, false); + DCHECK(updated); } } } diff --git a/test/913-heaps/heaps.cc b/test/913-heaps/heaps.cc index 19e12ae731..e319f7d98c 100644 --- a/test/913-heaps/heaps.cc +++ b/test/913-heaps/heaps.cc @@ -137,9 +137,9 @@ extern "C" JNIEXPORT jobjectArray JNICALL Java_art_Test913_followReferences( if (reference_kind == JVMTI_HEAP_REFERENCE_JNI_GLOBAL && class_tag == 0) { return 0; } - // Ignore classes (1000-1002@0) for thread objects. These can be held by the JIT. + // Ignore classes (1000 <= tag < 3000) for thread objects. These can be held by the JIT. if (reference_kind == JVMTI_HEAP_REFERENCE_THREAD && class_tag == 0 && - (1000 <= *tag_ptr && *tag_ptr <= 1002)) { + (1000 <= *tag_ptr && *tag_ptr < 3000)) { return 0; } // Ignore stack-locals of untagged threads. That is the environment. diff --git a/test/924-threads/expected.txt b/test/924-threads/expected.txt index 4c0f4eaa5b..1eb2e1bd52 100644 --- a/test/924-threads/expected.txt +++ b/test/924-threads/expected.txt @@ -1,10 +1,10 @@ currentThread OK -main +TestThread 5 false java.lang.ThreadGroup[name=main,maxpri=10] class dalvik.system.PathClassLoader -main +TestThread 5 false java.lang.ThreadGroup[name=main,maxpri=10] @@ -33,10 +33,11 @@ class dalvik.system.PathClassLoader e1 = ALIVE|WAITING_WITH_TIMEOUT|SLEEPING|WAITING 5 = ALIVE|RUNNABLE 2 = TERMINATED -[Thread[FinalizerDaemon,5,system], Thread[FinalizerWatchdogDaemon,5,system], Thread[HeapTaskDaemon,5,system], Thread[ReferenceQueueDaemon,5,system], Thread[Signal Catcher,5,system], Thread[main,5,main]] +[Thread[FinalizerDaemon,5,system], Thread[FinalizerWatchdogDaemon,5,system], Thread[HeapTaskDaemon,5,system], Thread[ReferenceQueueDaemon,5,system], Thread[TestThread,5,main], Thread[main,5,main]] JVMTI_ERROR_THREAD_NOT_ALIVE JVMTI_ERROR_THREAD_NOT_ALIVE Constructed thread -Thread(EventTestThread): start -Thread(EventTestThread): end +[] +[Thread(EventTestThread): start] +[Thread(EventTestThread): end] Thread joined diff --git a/test/924-threads/src/art/Test924.java b/test/924-threads/src/art/Test924.java index 160bf8ea67..5445939cbc 100644 --- a/test/924-threads/src/art/Test924.java +++ b/test/924-threads/src/art/Test924.java @@ -25,17 +25,35 @@ import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Set; public class Test924 { public static void run() throws Exception { Main.bindAgentJNIForClass(Test924.class); - doTest(); + + // Run the test on its own thread, so we have a known state for the "current" thread. + Thread t = new Thread("TestThread") { + @Override + public void run() { + try { + doTest(); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + }; + t.start(); + t.join(); } private static void doTest() throws Exception { Thread t1 = Thread.currentThread(); Thread t2 = getCurrentThread(); + // Need to adjust priority, as on-device this may be unexpected (and we prefer not + // to special-case this.) + t1.setPriority(5); + if (t1 != t2) { throw new RuntimeException("Expected " + t1 + " but got " + t2); } @@ -188,7 +206,32 @@ public class Test924 { } Collections.sort(threadList, THREAD_COMP); - System.out.println(threadList); + + List<Thread> expectedList = new ArrayList<>(); + Set<Thread> threadsFromTraces = Thread.getAllStackTraces().keySet(); + + expectedList.add(findThreadByName(threadsFromTraces, "FinalizerDaemon")); + expectedList.add(findThreadByName(threadsFromTraces, "FinalizerWatchdogDaemon")); + expectedList.add(findThreadByName(threadsFromTraces, "HeapTaskDaemon")); + expectedList.add(findThreadByName(threadsFromTraces, "ReferenceQueueDaemon")); + // We can't get the signal catcher through getAllStackTraces. So ignore it. + // expectedList.add(findThreadByName(threadsFromTraces, "Signal Catcher")); + expectedList.add(findThreadByName(threadsFromTraces, "TestThread")); + expectedList.add(findThreadByName(threadsFromTraces, "main")); + + if (!threadList.containsAll(expectedList)) { + throw new RuntimeException("Expected " + expectedList + " as subset, got " + threadList); + } + System.out.println(expectedList); + } + + private static Thread findThreadByName(Set<Thread> threads, String name) { + for (Thread t : threads) { + if (t.getName().equals(name)) { + return t; + } + } + throw new RuntimeException("Did not find thread " + name + ": " + threads); } private static void doTLSTests() throws Exception { @@ -256,13 +299,35 @@ public class Test924 { private static void doTestEvents() throws Exception { enableThreadEvents(true); - Thread t = new Thread("EventTestThread"); + final CountDownLatch cdl1 = new CountDownLatch(1); + final CountDownLatch cdl2 = new CountDownLatch(1); + + Runnable r = new Runnable() { + @Override + public void run() { + try { + cdl1.countDown(); + cdl2.await(); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + }; + Thread t = new Thread(r, "EventTestThread"); System.out.println("Constructed thread"); Thread.yield(); + Thread.sleep(100); + System.out.println(Arrays.toString(getThreadEventMessages())); t.start(); + cdl1.await(); + + System.out.println(Arrays.toString(getThreadEventMessages())); + + cdl2.countDown(); t.join(); + System.out.println(Arrays.toString(getThreadEventMessages())); System.out.println("Thread joined"); @@ -337,4 +402,5 @@ public class Test924 { private static native void setTLS(Thread t, long l); private static native long getTLS(Thread t); private static native void enableThreadEvents(boolean b); + private static native String[] getThreadEventMessages(); } diff --git a/test/924-threads/threads.cc b/test/924-threads/threads.cc index 701ab1def3..e21dcc240e 100644 --- a/test/924-threads/threads.cc +++ b/test/924-threads/threads.cc @@ -16,6 +16,10 @@ #include <stdio.h> +#include <mutex> +#include <string> +#include <vector> + #include "android-base/logging.h" #include "android-base/stringprintf.h" #include "jni.h" @@ -139,17 +143,27 @@ extern "C" JNIEXPORT void JNICALL Java_art_Test924_setTLS( JvmtiErrorToException(env, jvmti_env, result); } +static std::mutex gEventsMutex; +static std::vector<std::string> gEvents; + static void JNICALL ThreadEvent(jvmtiEnv* jvmti_env, JNIEnv* jni_env, jthread thread, bool is_start) { jvmtiThreadInfo info; - jvmtiError result = jvmti_env->GetThreadInfo(thread, &info); - if (result != JVMTI_ERROR_NONE) { - printf("Error getting thread info"); - return; + { + std::lock_guard<std::mutex> guard(gEventsMutex); + + jvmtiError result = jvmti_env->GetThreadInfo(thread, &info); + if (result != JVMTI_ERROR_NONE) { + gEvents.push_back("Error getting thread info"); + return; + } + + gEvents.push_back(android::base::StringPrintf("Thread(%s): %s", + info.name, + is_start ? "start" : "end")); } - printf("Thread(%s): %s\n", info.name, is_start ? "start" : "end"); jvmti_env->Deallocate(reinterpret_cast<unsigned char*>(info.name)); jni_env->DeleteLocalRef(info.thread_group); @@ -205,5 +219,18 @@ extern "C" JNIEXPORT void JNICALL Java_art_Test924_enableThreadEvents( JvmtiErrorToException(env, jvmti_env, ret); } +extern "C" JNIEXPORT jobjectArray JNICALL Java_art_Test924_getThreadEventMessages( + JNIEnv* env, jclass Main_klass ATTRIBUTE_UNUSED) { + std::lock_guard<std::mutex> guard(gEventsMutex); + jobjectArray ret = CreateObjectArray(env, + static_cast<jint>(gEvents.size()), + "java/lang/String", + [&](jint i) { + return env->NewStringUTF(gEvents[i].c_str()); + }); + gEvents.clear(); + return ret; +} + } // namespace Test924Threads } // namespace art diff --git a/test/985-re-obsolete/expected.txt b/test/985-re-obsolete/expected.txt new file mode 100644 index 0000000000..5159a00f43 --- /dev/null +++ b/test/985-re-obsolete/expected.txt @@ -0,0 +1,35 @@ +Pre Start private method call +hello - private +Post Start private method call +Not doing anything here +Pre Finish private method call +goodbye - private +Post Finish private method call +Pre Start private method call +hello - private +Post Start private method call +transforming calling function +Pre Finish private method call +Goodbye - private - Transformed +Post Finish private method call +Pre Start private method call - Transformed +Hello - private - Transformed +Post Start private method call - Transformed +Not doing anything here +Pre Finish private method call - Transformed +Goodbye - private - Transformed +Post Finish private method call - Transformed +Pre Start private method call - Transformed +Hello - private - Transformed +Post Start private method call - Transformed +transforming calling function +Pre Finish private method call - Transformed +second - Goodbye - private - Transformed +Post Finish private method call - Transformed +second - Pre Start private method call - Transformed +second - Hello - private - Transformed +second - Post Start private method call - Transformed +Not doing anything here +second - Pre Finish private method call - Transformed +second - Goodbye - private - Transformed +second - Post Finish private method call - Transformed diff --git a/test/985-re-obsolete/info.txt b/test/985-re-obsolete/info.txt new file mode 100644 index 0000000000..c8eafdc723 --- /dev/null +++ b/test/985-re-obsolete/info.txt @@ -0,0 +1,4 @@ +Tests basic obsolete method support + +Regression test for b/37475600 which was caused by incorrectly checking for +differences in the obsolete methods map. diff --git a/test/985-re-obsolete/run b/test/985-re-obsolete/run new file mode 100755 index 0000000000..e92b873956 --- /dev/null +++ b/test/985-re-obsolete/run @@ -0,0 +1,17 @@ +#!/bin/bash +# +# Copyright 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. + +./default-run "$@" --jvmti diff --git a/test/985-re-obsolete/src/Main.java b/test/985-re-obsolete/src/Main.java new file mode 100644 index 0000000000..d78d591f47 --- /dev/null +++ b/test/985-re-obsolete/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.Test985.run(); + } +} diff --git a/test/985-re-obsolete/src/art/Main.java b/test/985-re-obsolete/src/art/Main.java new file mode 100644 index 0000000000..8b01920638 --- /dev/null +++ b/test/985-re-obsolete/src/art/Main.java @@ -0,0 +1,28 @@ +/* + * 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; + +// Binder class so the agent's C code has something that can be bound and exposed to tests. +// In a package to separate cleanly and work around CTS reference issues (though this class +// should be replaced in the CTS version). +public class Main { + // Load the given class with the given classloader, and bind all native methods to corresponding + // C methods in the agent. Will abort if any of the steps fail. + public static native void bindAgentJNI(String className, ClassLoader classLoader); + // Same as above, giving the class directly. + public static native void bindAgentJNIForClass(Class<?> klass); +} diff --git a/test/985-re-obsolete/src/art/Redefinition.java b/test/985-re-obsolete/src/art/Redefinition.java new file mode 100644 index 0000000000..0350ab42ad --- /dev/null +++ b/test/985-re-obsolete/src/art/Redefinition.java @@ -0,0 +1,96 @@ +/* + * 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 { + // Bind native functions. + static { + Main.bindAgentJNIForClass(Redefinition.class); + } + + 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/985-re-obsolete/src/art/Test985.java b/test/985-re-obsolete/src/art/Test985.java new file mode 100644 index 0000000000..405abd5d14 --- /dev/null +++ b/test/985-re-obsolete/src/art/Test985.java @@ -0,0 +1,197 @@ +/* + * 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.Base64; + +public class Test985 { + + static class Transform { + private void Start() { + System.out.println("hello - private"); + } + + private void Finish() { + System.out.println("goodbye - private"); + } + + public void sayHi(Runnable r) { + System.out.println("Pre Start private method call"); + Start(); + System.out.println("Post Start private method call"); + r.run(); + System.out.println("Pre Finish private method call"); + Finish(); + System.out.println("Post Finish private method call"); + } + } + + // static class Transform { + // private void Start() { + // System.out.println("Hello - private - Transformed"); + // } + // + // private void Finish() { + // System.out.println("Goodbye - private - Transformed"); + // } + // + // public void sayHi(Runnable r) { + // System.out.println("Pre Start private method call - Transformed"); + // Start(); + // System.out.println("Post Start private method call - Transformed"); + // r.run(); + // System.out.println("Pre Finish private method call - Transformed"); + // Finish(); + // System.out.println("Post Finish private method call - Transformed"); + // } + // } + private static final byte[] CLASS_BYTES_1 = Base64.getDecoder().decode( + "yv66vgAAADQANgoADgAZCQAaABsIABwKAB0AHggAHwgAIAoADQAhCAAiCwAjACQIACUKAA0AJggA" + + "JwcAKQcALAEABjxpbml0PgEAAygpVgEABENvZGUBAA9MaW5lTnVtYmVyVGFibGUBAAVTdGFydAEA" + + "BkZpbmlzaAEABXNheUhpAQAXKExqYXZhL2xhbmcvUnVubmFibGU7KVYBAApTb3VyY2VGaWxlAQAM" + + "VGVzdDk4NS5qYXZhDAAPABAHAC0MAC4ALwEAHUhlbGxvIC0gcHJpdmF0ZSAtIFRyYW5zZm9ybWVk" + + "BwAwDAAxADIBAB9Hb29kYnllIC0gcHJpdmF0ZSAtIFRyYW5zZm9ybWVkAQArUHJlIFN0YXJ0IHBy" + + "aXZhdGUgbWV0aG9kIGNhbGwgLSBUcmFuc2Zvcm1lZAwAEwAQAQAsUG9zdCBTdGFydCBwcml2YXRl" + + "IG1ldGhvZCBjYWxsIC0gVHJhbnNmb3JtZWQHADMMADQAEAEALFByZSBGaW5pc2ggcHJpdmF0ZSBt" + + "ZXRob2QgY2FsbCAtIFRyYW5zZm9ybWVkDAAUABABAC1Qb3N0IEZpbmlzaCBwcml2YXRlIG1ldGhv" + + "ZCBjYWxsIC0gVHJhbnNmb3JtZWQHADUBABVhcnQvVGVzdDk4NSRUcmFuc2Zvcm0BAAlUcmFuc2Zv" + + "cm0BAAxJbm5lckNsYXNzZXMBABBqYXZhL2xhbmcvT2JqZWN0AQAQamF2YS9sYW5nL1N5c3RlbQEA" + + "A291dAEAFUxqYXZhL2lvL1ByaW50U3RyZWFtOwEAE2phdmEvaW8vUHJpbnRTdHJlYW0BAAdwcmlu" + + "dGxuAQAVKExqYXZhL2xhbmcvU3RyaW5nOylWAQASamF2YS9sYW5nL1J1bm5hYmxlAQADcnVuAQAL" + + "YXJ0L1Rlc3Q5ODUAIAANAA4AAAAAAAQAAAAPABAAAQARAAAAHQABAAEAAAAFKrcAAbEAAAABABIA" + + "AAAGAAEAAAAEAAIAEwAQAAEAEQAAACUAAgABAAAACbIAAhIDtgAEsQAAAAEAEgAAAAoAAgAAAAYA" + + "CAAHAAIAFAAQAAEAEQAAACUAAgABAAAACbIAAhIFtgAEsQAAAAEAEgAAAAoAAgAAAAkACAAKAAEA" + + "FQAWAAEAEQAAAGMAAgACAAAAL7IAAhIGtgAEKrcAB7IAAhIItgAEK7kACQEAsgACEgq2AAQqtwAL" + + "sgACEgy2AASxAAAAAQASAAAAIgAIAAAADAAIAA0ADAAOABQADwAaABAAIgARACYAEgAuABMAAgAX" + + "AAAAAgAYACsAAAAKAAEADQAoACoACA=="); + private static final byte[] DEX_BYTES_1 = Base64.getDecoder().decode( + "ZGV4CjAzNQAh+CJbAAAAAAAAAAAAAAAAAAAAAAAAAADUBQAAcAAAAHhWNBIAAAAAAAAAABAFAAAd" + + "AAAAcAAAAAoAAADkAAAAAwAAAAwBAAABAAAAMAEAAAcAAAA4AQAAAQAAAHABAABEBAAAkAEAAJAB" + + "AACYAQAAoAEAAMEBAADgAQAA+QEAAAgCAAAsAgAATAIAAGMCAAB3AgAAjQIAAKECAAC1AgAA5AIA" + + "ABIDAABAAwAAbQMAAHQDAACCAwAAjQMAAJADAACUAwAAoQMAAKcDAACsAwAAtQMAALoDAADBAwAA" + + "BAAAAAUAAAAGAAAABwAAAAgAAAAJAAAACgAAAAsAAAAMAAAAFAAAABQAAAAJAAAAAAAAABUAAAAJ" + + "AAAA0AMAABUAAAAJAAAAyAMAAAgABAAYAAAAAAAAAAAAAAAAAAAAAQAAAAAAAAARAAAAAAABABsA" + + "AAAEAAIAGQAAAAUAAAAAAAAABgAAABoAAAAAAAAAAAAAAAUAAAAAAAAAEgAAAAAFAADMBAAAAAAA" + + "AAY8aW5pdD4ABkZpbmlzaAAfR29vZGJ5ZSAtIHByaXZhdGUgLSBUcmFuc2Zvcm1lZAAdSGVsbG8g" + + "LSBwcml2YXRlIC0gVHJhbnNmb3JtZWQAF0xhcnQvVGVzdDk4NSRUcmFuc2Zvcm07AA1MYXJ0L1Rl" + + "c3Q5ODU7ACJMZGFsdmlrL2Fubm90YXRpb24vRW5jbG9zaW5nQ2xhc3M7AB5MZGFsdmlrL2Fubm90" + + "YXRpb24vSW5uZXJDbGFzczsAFUxqYXZhL2lvL1ByaW50U3RyZWFtOwASTGphdmEvbGFuZy9PYmpl" + + "Y3Q7ABRMamF2YS9sYW5nL1J1bm5hYmxlOwASTGphdmEvbGFuZy9TdHJpbmc7ABJMamF2YS9sYW5n" + + "L1N5c3RlbTsALVBvc3QgRmluaXNoIHByaXZhdGUgbWV0aG9kIGNhbGwgLSBUcmFuc2Zvcm1lZAAs" + + "UG9zdCBTdGFydCBwcml2YXRlIG1ldGhvZCBjYWxsIC0gVHJhbnNmb3JtZWQALFByZSBGaW5pc2gg" + + "cHJpdmF0ZSBtZXRob2QgY2FsbCAtIFRyYW5zZm9ybWVkACtQcmUgU3RhcnQgcHJpdmF0ZSBtZXRo" + + "b2QgY2FsbCAtIFRyYW5zZm9ybWVkAAVTdGFydAAMVGVzdDk4NS5qYXZhAAlUcmFuc2Zvcm0AAVYA" + + "AlZMAAthY2Nlc3NGbGFncwAEbmFtZQADb3V0AAdwcmludGxuAANydW4ABXNheUhpAAV2YWx1ZQAB" + + "AAAABwAAAAEAAAAGAAAABAAHDgAJAAcOAQgPAAYABw4BCA8ADAEABw4BCA8BAw8BCA8BAw8BCA8B" + + "Aw8BCA8AAQABAAEAAADYAwAABAAAAHAQBQAAAA4AAwABAAIAAADdAwAACQAAAGIAAAAbAQIAAABu" + + "IAQAEAAOAAAAAwABAAIAAADlAwAACQAAAGIAAAAbAQMAAABuIAQAEAAOAAAABAACAAIAAADtAwAA" + + "KgAAAGIAAAAbARAAAABuIAQAEABwEAIAAgBiAAAAGwEOAAAAbiAEABAAchAGAAMAYgAAABsBDwAA" + + "AG4gBAAQAHAQAQACAGIAAAAbAQ0AAABuIAQAEAAOAAAAAwEAgIAEiAgBAqAIAQLECAMB6AgAAAIC" + + "ARwYAQIDAhYECBcXEwACAAAA5AQAAOoEAAD0BAAAAAAAAAAAAAAAAAAAEAAAAAAAAAABAAAAAAAA" + + "AAEAAAAdAAAAcAAAAAIAAAAKAAAA5AAAAAMAAAADAAAADAEAAAQAAAABAAAAMAEAAAUAAAAHAAAA" + + "OAEAAAYAAAABAAAAcAEAAAIgAAAdAAAAkAEAAAEQAAACAAAAyAMAAAMgAAAEAAAA2AMAAAEgAAAE" + + "AAAACAQAAAAgAAABAAAAzAQAAAQgAAACAAAA5AQAAAMQAAABAAAA9AQAAAYgAAABAAAAAAUAAAAQ" + + "AAABAAAAEAUAAA=="); + + // static class Transform { + // private void Start() { + // System.out.println("second - Hello - private - Transformed"); + // } + // + // private void Finish() { + // System.out.println("second - Goodbye - private - Transformed"); + // } + // + // public void sayHi(Runnable r) { + // System.out.println("second - Pre Start private method call - Transformed"); + // Start(); + // System.out.println("second - Post Start private method call - Transformed"); + // r.run(); + // System.out.println("second - Pre Finish private method call - Transformed"); + // Finish(); + // System.out.println("second - Post Finish private method call - Transformed"); + // } + // } + private static final byte[] CLASS_BYTES_2 = Base64.getDecoder().decode( + "yv66vgAAADQANgoADgAZCQAaABsIABwKAB0AHggAHwgAIAoADQAhCAAiCwAjACQIACUKAA0AJggA" + + "JwcAKQcALAEABjxpbml0PgEAAygpVgEABENvZGUBAA9MaW5lTnVtYmVyVGFibGUBAAVTdGFydAEA" + + "BkZpbmlzaAEABXNheUhpAQAXKExqYXZhL2xhbmcvUnVubmFibGU7KVYBAApTb3VyY2VGaWxlAQAM" + + "VGVzdDk4NS5qYXZhDAAPABAHAC0MAC4ALwEAJnNlY29uZCAtIEhlbGxvIC0gcHJpdmF0ZSAtIFRy" + + "YW5zZm9ybWVkBwAwDAAxADIBAChzZWNvbmQgLSBHb29kYnllIC0gcHJpdmF0ZSAtIFRyYW5zZm9y" + + "bWVkAQA0c2Vjb25kIC0gUHJlIFN0YXJ0IHByaXZhdGUgbWV0aG9kIGNhbGwgLSBUcmFuc2Zvcm1l" + + "ZAwAEwAQAQA1c2Vjb25kIC0gUG9zdCBTdGFydCBwcml2YXRlIG1ldGhvZCBjYWxsIC0gVHJhbnNm" + + "b3JtZWQHADMMADQAEAEANXNlY29uZCAtIFByZSBGaW5pc2ggcHJpdmF0ZSBtZXRob2QgY2FsbCAt" + + "IFRyYW5zZm9ybWVkDAAUABABADZzZWNvbmQgLSBQb3N0IEZpbmlzaCBwcml2YXRlIG1ldGhvZCBj" + + "YWxsIC0gVHJhbnNmb3JtZWQHADUBABVhcnQvVGVzdDk4NSRUcmFuc2Zvcm0BAAlUcmFuc2Zvcm0B" + + "AAxJbm5lckNsYXNzZXMBABBqYXZhL2xhbmcvT2JqZWN0AQAQamF2YS9sYW5nL1N5c3RlbQEAA291" + + "dAEAFUxqYXZhL2lvL1ByaW50U3RyZWFtOwEAE2phdmEvaW8vUHJpbnRTdHJlYW0BAAdwcmludGxu" + + "AQAVKExqYXZhL2xhbmcvU3RyaW5nOylWAQASamF2YS9sYW5nL1J1bm5hYmxlAQADcnVuAQALYXJ0" + + "L1Rlc3Q5ODUAIAANAA4AAAAAAAQAAAAPABAAAQARAAAAHQABAAEAAAAFKrcAAbEAAAABABIAAAAG" + + "AAEAAAAEAAIAEwAQAAEAEQAAACUAAgABAAAACbIAAhIDtgAEsQAAAAEAEgAAAAoAAgAAAAYACAAH" + + "AAIAFAAQAAEAEQAAACUAAgABAAAACbIAAhIFtgAEsQAAAAEAEgAAAAoAAgAAAAkACAAKAAEAFQAW" + + "AAEAEQAAAGMAAgACAAAAL7IAAhIGtgAEKrcAB7IAAhIItgAEK7kACQEAsgACEgq2AAQqtwALsgAC" + + "Egy2AASxAAAAAQASAAAAIgAIAAAADAAIAA0ADAAOABQADwAaABAAIgARACYAEgAuABMAAgAXAAAA" + + "AgAYACsAAAAKAAEADQAoACoACA=="); + private static final byte[] DEX_BYTES_2 = Base64.getDecoder().decode( + "ZGV4CjAzNQBw/x+UAAAAAAAAAAAAAAAAAAAAAAAAAAAMBgAAcAAAAHhWNBIAAAAAAAAAAEgFAAAd" + + "AAAAcAAAAAoAAADkAAAAAwAAAAwBAAABAAAAMAEAAAcAAAA4AQAAAQAAAHABAAB8BAAAkAEAAJAB" + + "AACYAQAAoAEAALkBAADIAQAA7AEAAAwCAAAjAgAANwIAAE0CAABhAgAAdQIAAHwCAACKAgAAlQIA" + + "AJgCAACcAgAAqQIAAK8CAAC0AgAAvQIAAMICAADJAgAA8wIAABsDAABTAwAAigMAAMEDAAD3AwAA" + + "AgAAAAMAAAAEAAAABQAAAAYAAAAHAAAACAAAAAkAAAAKAAAADgAAAA4AAAAJAAAAAAAAAA8AAAAJ" + + "AAAACAQAAA8AAAAJAAAAAAQAAAgABAASAAAAAAAAAAAAAAAAAAAAAQAAAAAAAAALAAAAAAABABUA" + + "AAAEAAIAEwAAAAUAAAAAAAAABgAAABQAAAAAAAAAAAAAAAUAAAAAAAAADAAAADgFAAAEBQAAAAAA" + + "AAY8aW5pdD4ABkZpbmlzaAAXTGFydC9UZXN0OTg1JFRyYW5zZm9ybTsADUxhcnQvVGVzdDk4NTsA" + + "IkxkYWx2aWsvYW5ub3RhdGlvbi9FbmNsb3NpbmdDbGFzczsAHkxkYWx2aWsvYW5ub3RhdGlvbi9J" + + "bm5lckNsYXNzOwAVTGphdmEvaW8vUHJpbnRTdHJlYW07ABJMamF2YS9sYW5nL09iamVjdDsAFExq" + + "YXZhL2xhbmcvUnVubmFibGU7ABJMamF2YS9sYW5nL1N0cmluZzsAEkxqYXZhL2xhbmcvU3lzdGVt" + + "OwAFU3RhcnQADFRlc3Q5ODUuamF2YQAJVHJhbnNmb3JtAAFWAAJWTAALYWNjZXNzRmxhZ3MABG5h" + + "bWUAA291dAAHcHJpbnRsbgADcnVuAAVzYXlIaQAoc2Vjb25kIC0gR29vZGJ5ZSAtIHByaXZhdGUg" + + "LSBUcmFuc2Zvcm1lZAAmc2Vjb25kIC0gSGVsbG8gLSBwcml2YXRlIC0gVHJhbnNmb3JtZWQANnNl" + + "Y29uZCAtIFBvc3QgRmluaXNoIHByaXZhdGUgbWV0aG9kIGNhbGwgLSBUcmFuc2Zvcm1lZAA1c2Vj" + + "b25kIC0gUG9zdCBTdGFydCBwcml2YXRlIG1ldGhvZCBjYWxsIC0gVHJhbnNmb3JtZWQANXNlY29u" + + "ZCAtIFByZSBGaW5pc2ggcHJpdmF0ZSBtZXRob2QgY2FsbCAtIFRyYW5zZm9ybWVkADRzZWNvbmQg" + + "LSBQcmUgU3RhcnQgcHJpdmF0ZSBtZXRob2QgY2FsbCAtIFRyYW5zZm9ybWVkAAV2YWx1ZQAAAAEA" + + "AAAHAAAAAQAAAAYAAAAEAAcOAAkABw4BCA8ABgAHDgEIDwAMAQAHDgEIDwEDDwEIDwEDDwEIDwED" + + "DwEIDwABAAEAAQAAABAEAAAEAAAAcBAFAAAADgADAAEAAgAAABUEAAAJAAAAYgAAABsBFgAAAG4g" + + "BAAQAA4AAAADAAEAAgAAAB0EAAAJAAAAYgAAABsBFwAAAG4gBAAQAA4AAAAEAAIAAgAAACUEAAAq" + + "AAAAYgAAABsBGwAAAG4gBAAQAHAQAgACAGIAAAAbARkAAABuIAQAEAByEAYAAwBiAAAAGwEaAAAA" + + "biAEABAAcBABAAIAYgAAABsBGAAAAG4gBAAQAA4AAAADAQCAgATACAEC2AgBAvwIAwGgCQAAAgIB" + + "HBgBAgMCEAQIERcNAAIAAAAcBQAAIgUAACwFAAAAAAAAAAAAAAAAAAAQAAAAAAAAAAEAAAAAAAAA" + + "AQAAAB0AAABwAAAAAgAAAAoAAADkAAAAAwAAAAMAAAAMAQAABAAAAAEAAAAwAQAABQAAAAcAAAA4" + + "AQAABgAAAAEAAABwAQAAAiAAAB0AAACQAQAAARAAAAIAAAAABAAAAyAAAAQAAAAQBAAAASAAAAQA" + + "AABABAAAACAAAAEAAAAEBQAABCAAAAIAAAAcBQAAAxAAAAEAAAAsBQAABiAAAAEAAAA4BQAAABAA" + + "AAEAAABIBQAA"); + + public static void run() { + Redefinition.setTestConfiguration(Redefinition.Config.COMMON_REDEFINE); + doTest(new Transform()); + } + + public static void doTest(Transform t) { + t.sayHi(() -> { System.out.println("Not doing anything here"); }); + t.sayHi(() -> { + System.out.println("transforming calling function"); + Redefinition.doCommonClassRedefinition(Transform.class, CLASS_BYTES_1, DEX_BYTES_1); + }); + t.sayHi(() -> { System.out.println("Not doing anything here"); }); + t.sayHi(() -> { + System.out.println("transforming calling function"); + Redefinition.doCommonClassRedefinition(Transform.class, CLASS_BYTES_2, DEX_BYTES_2); + }); + t.sayHi(() -> { System.out.println("Not doing anything here"); }); + } +} diff --git a/test/common/stack_inspect.cc b/test/common/stack_inspect.cc index df7fa20226..ceb4ba241b 100644 --- a/test/common/stack_inspect.cc +++ b/test/common/stack_inspect.cc @@ -144,22 +144,11 @@ extern "C" JNIEXPORT void JNICALL Java_Main_assertIsInterpreted(JNIEnv* env, jcl } } -static jboolean IsManaged(JNIEnv* env, jclass cls, size_t level) { +static jboolean IsManaged(JNIEnv* env, jclass, size_t level) { ScopedObjectAccess soa(env); - - ObjPtr<mirror::Class> klass = soa.Decode<mirror::Class>(cls); - const DexFile& dex_file = klass->GetDexFile(); - const OatFile::OatDexFile* oat_dex_file = dex_file.GetOatDexFile(); - if (oat_dex_file == nullptr) { - // No oat file, this must be a test configuration that doesn't compile at all. Ignore that the - // result will be that we're running the interpreter. - return JNI_FALSE; - } - NthCallerVisitor caller(soa.Self(), level, false); caller.WalkStack(); CHECK(caller.caller != nullptr); - return caller.GetCurrentShadowFrame() != nullptr ? JNI_FALSE : JNI_TRUE; } diff --git a/test/knownfailures.json b/test/knownfailures.json index e7343a0fd5..0e42a29bf6 100644 --- a/test/knownfailures.json +++ b/test/knownfailures.json @@ -220,7 +220,6 @@ "tests": ["604-hot-static-interface", "612-jit-dex-cache", "613-inlining-dex-cache", - "616-cha", "626-set-resolved-string"], "variant": "trace | stream", "description": ["These tests expect JIT compilation, which is", @@ -330,14 +329,8 @@ "variant": "interpreter | optimizing | regalloc_gc | jit" }, { - "tests": ["912-classes", - "616-cha", - "616-cha-abstract", - "616-cha-interface", - "616-cha-interface-default", - "616-cha-miranda", - "616-cha-proxy-method-inline"], - "bug": "http://b/36344364 http://b/36344221", + "tests": ["912-classes"], + "bug": "http://b/36344364", "variant": "no-dex2oat | relocate-npatchoat" }, { diff --git a/tools/run-jdwp-tests.sh b/tools/run-jdwp-tests.sh index 6d5c74b82a..07c300e7a7 100755 --- a/tools/run-jdwp-tests.sh +++ b/tools/run-jdwp-tests.sh @@ -51,6 +51,12 @@ host="no" # Use JIT compiling by default. use_jit=true variant_cmdline_parameter="--variant=X32" +# Timeout of JDWP test in ms. +# +# Note: some tests expect a timeout to check that *no* reply/event is received for a specific case. +# A lower timeout can save up several minutes when running the whole test suite, especially for +# continuous testing. This value can be adjusted to fit the configuration of the host machine(s). +jdwp_test_timeout=10000 while true; do if [[ "$1" == "--mode=host" ]]; then @@ -150,6 +156,8 @@ vogar $vm_command \ $image_compiler_option \ --timeout 800 \ --vm-arg -Djpda.settings.verbose=true \ + --vm-arg -Djpda.settings.timeout=$jdwp_test_timeout \ + --vm-arg -Djpda.settings.waitingTime=$jdwp_test_timeout \ --vm-arg -Djpda.settings.transportAddress=127.0.0.1:55107 \ --vm-arg -Djpda.settings.debuggeeJavaPath="$art_debugee $image $debuggee_args" \ --classpath $test_jack \ |