diff options
37 files changed, 1214 insertions, 71 deletions
diff --git a/compiler/optimizing/bounds_check_elimination.cc b/compiler/optimizing/bounds_check_elimination.cc index 6511120794..3645f19f09 100644 --- a/compiler/optimizing/bounds_check_elimination.cc +++ b/compiler/optimizing/bounds_check_elimination.cc @@ -1064,7 +1064,7 @@ class BCEVisitor : public HGraphVisitor { }; void BoundsCheckElimination::Run() { - if (!graph_->HasArrayAccesses()) { + if (!graph_->HasBoundsChecks()) { return; } diff --git a/compiler/optimizing/bounds_check_elimination_test.cc b/compiler/optimizing/bounds_check_elimination_test.cc index 75cf1cf063..97be778dbd 100644 --- a/compiler/optimizing/bounds_check_elimination_test.cc +++ b/compiler/optimizing/bounds_check_elimination_test.cc @@ -43,7 +43,7 @@ TEST(BoundsCheckEliminationTest, NarrowingRangeArrayBoundsElimination) { ArenaAllocator allocator(&pool); HGraph* graph = new (&allocator) HGraph(&allocator); - graph->SetHasArrayAccesses(true); + graph->SetHasBoundsChecks(true); HBasicBlock* entry = new (&allocator) HBasicBlock(graph); graph->AddBlock(entry); @@ -148,7 +148,7 @@ TEST(BoundsCheckEliminationTest, OverflowArrayBoundsElimination) { ArenaAllocator allocator(&pool); HGraph* graph = new (&allocator) HGraph(&allocator); - graph->SetHasArrayAccesses(true); + graph->SetHasBoundsChecks(true); HBasicBlock* entry = new (&allocator) HBasicBlock(graph); graph->AddBlock(entry); @@ -220,7 +220,7 @@ TEST(BoundsCheckEliminationTest, UnderflowArrayBoundsElimination) { ArenaAllocator allocator(&pool); HGraph* graph = new (&allocator) HGraph(&allocator); - graph->SetHasArrayAccesses(true); + graph->SetHasBoundsChecks(true); HBasicBlock* entry = new (&allocator) HBasicBlock(graph); graph->AddBlock(entry); @@ -292,7 +292,7 @@ TEST(BoundsCheckEliminationTest, ConstantArrayBoundsElimination) { ArenaAllocator allocator(&pool); HGraph* graph = new (&allocator) HGraph(&allocator); - graph->SetHasArrayAccesses(true); + graph->SetHasBoundsChecks(true); HBasicBlock* entry = new (&allocator) HBasicBlock(graph); graph->AddBlock(entry); @@ -365,7 +365,7 @@ static HGraph* BuildSSAGraph1(ArenaAllocator* allocator, int increment, IfCondition cond = kCondGE) { HGraph* graph = new (allocator) HGraph(allocator); - graph->SetHasArrayAccesses(true); + graph->SetHasBoundsChecks(true); HBasicBlock* entry = new (allocator) HBasicBlock(graph); graph->AddBlock(entry); @@ -502,7 +502,7 @@ static HGraph* BuildSSAGraph2(ArenaAllocator* allocator, int increment = -1, IfCondition cond = kCondLE) { HGraph* graph = new (allocator) HGraph(allocator); - graph->SetHasArrayAccesses(true); + graph->SetHasBoundsChecks(true); HBasicBlock* entry = new (allocator) HBasicBlock(graph); graph->AddBlock(entry); @@ -633,7 +633,7 @@ static HGraph* BuildSSAGraph3(ArenaAllocator* allocator, int increment, IfCondition cond) { HGraph* graph = new (allocator) HGraph(allocator); - graph->SetHasArrayAccesses(true); + graph->SetHasBoundsChecks(true); HBasicBlock* entry = new (allocator) HBasicBlock(graph); graph->AddBlock(entry); @@ -744,7 +744,7 @@ static HGraph* BuildSSAGraph4(ArenaAllocator* allocator, int initial, IfCondition cond = kCondGE) { HGraph* graph = new (allocator) HGraph(allocator); - graph->SetHasArrayAccesses(true); + graph->SetHasBoundsChecks(true); HBasicBlock* entry = new (allocator) HBasicBlock(graph); graph->AddBlock(entry); @@ -869,7 +869,7 @@ TEST(BoundsCheckEliminationTest, BubbleSortArrayBoundsElimination) { ArenaAllocator allocator(&pool); HGraph* graph = new (&allocator) HGraph(&allocator); - graph->SetHasArrayAccesses(true); + graph->SetHasBoundsChecks(true); HBasicBlock* entry = new (&allocator) HBasicBlock(graph); graph->AddBlock(entry); diff --git a/compiler/optimizing/builder.cc b/compiler/optimizing/builder.cc index ebd8243c2b..96e08fd24c 100644 --- a/compiler/optimizing/builder.cc +++ b/compiler/optimizing/builder.cc @@ -963,7 +963,7 @@ void HGraphBuilder::BuildArrayAccess(const Instruction& instruction, current_block_->AddInstruction(new (arena_) HArrayGet(object, index, anticipated_type)); UpdateLocal(source_or_dest_reg, current_block_->GetLastInstruction()); } - graph_->SetHasArrayAccesses(true); + graph_->SetHasBoundsChecks(true); } void HGraphBuilder::BuildFilledNewArray(uint32_t dex_pc, @@ -1065,7 +1065,7 @@ void HGraphBuilder::BuildFillArrayData(const Instruction& instruction, uint32_t default: LOG(FATAL) << "Unknown element width for " << payload->element_width; } - graph_->SetHasArrayAccesses(true); + graph_->SetHasBoundsChecks(true); } void HGraphBuilder::BuildFillWideArrayData(HInstruction* object, diff --git a/compiler/optimizing/code_generator_arm.cc b/compiler/optimizing/code_generator_arm.cc index d1c318c05e..01748a9f5c 100644 --- a/compiler/optimizing/code_generator_arm.cc +++ b/compiler/optimizing/code_generator_arm.cc @@ -1242,8 +1242,12 @@ void InstructionCodeGeneratorARM::VisitReturn(HReturn* ret) { void LocationsBuilderARM::VisitInvokeStaticOrDirect(HInvokeStaticOrDirect* invoke) { // Explicit clinit checks triggered by static invokes must have been - // pruned by art::PrepareForRegisterAllocation. - DCHECK(!invoke->IsStaticWithExplicitClinitCheck()); + // pruned by art::PrepareForRegisterAllocation, but this step is not + // run in baseline. So we remove them manually here if we find them. + // TODO: Instead of this local workaround, address this properly. + if (invoke->IsStaticWithExplicitClinitCheck()) { + invoke->RemoveClinitCheckOrLoadClassAsLastInput(); + } IntrinsicLocationsBuilderARM intrinsic(GetGraph()->GetArena(), codegen_->GetInstructionSetFeatures()); diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc index 7beda96857..dada4ce5bd 100644 --- a/compiler/optimizing/code_generator_arm64.cc +++ b/compiler/optimizing/code_generator_arm64.cc @@ -1969,8 +1969,12 @@ void LocationsBuilderARM64::VisitInvokeVirtual(HInvokeVirtual* invoke) { void LocationsBuilderARM64::VisitInvokeStaticOrDirect(HInvokeStaticOrDirect* invoke) { // Explicit clinit checks triggered by static invokes must have been - // pruned by art::PrepareForRegisterAllocation. - DCHECK(!invoke->IsStaticWithExplicitClinitCheck()); + // pruned by art::PrepareForRegisterAllocation, but this step is not + // run in baseline. So we remove them manually here if we find them. + // TODO: Instead of this local workaround, address this properly. + if (invoke->IsStaticWithExplicitClinitCheck()) { + invoke->RemoveClinitCheckOrLoadClassAsLastInput(); + } IntrinsicLocationsBuilderARM64 intrinsic(GetGraph()->GetArena()); if (intrinsic.TryDispatch(invoke)) { diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc index 70e4440e7a..04999bedb0 100644 --- a/compiler/optimizing/code_generator_x86.cc +++ b/compiler/optimizing/code_generator_x86.cc @@ -1195,8 +1195,12 @@ void InstructionCodeGeneratorX86::VisitReturn(HReturn* ret) { void LocationsBuilderX86::VisitInvokeStaticOrDirect(HInvokeStaticOrDirect* invoke) { // Explicit clinit checks triggered by static invokes must have been - // pruned by art::PrepareForRegisterAllocation. - DCHECK(!invoke->IsStaticWithExplicitClinitCheck()); + // pruned by art::PrepareForRegisterAllocation, but this step is not + // run in baseline. So we remove them manually here if we find them. + // TODO: Instead of this local workaround, address this properly. + if (invoke->IsStaticWithExplicitClinitCheck()) { + invoke->RemoveClinitCheckOrLoadClassAsLastInput(); + } IntrinsicLocationsBuilderX86 intrinsic(codegen_); if (intrinsic.TryDispatch(invoke)) { @@ -3815,7 +3819,7 @@ void LocationsBuilderX86::VisitBoundsCheck(HBoundsCheck* instruction) { LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(instruction, LocationSummary::kNoCall); locations->SetInAt(0, Location::RegisterOrConstant(instruction->InputAt(0))); - locations->SetInAt(1, Location::RequiresRegister()); + locations->SetInAt(1, Location::RegisterOrConstant(instruction->InputAt(1))); if (instruction->HasUses()) { locations->SetOut(Location::SameAsFirstInput()); } @@ -3827,16 +3831,38 @@ void InstructionCodeGeneratorX86::VisitBoundsCheck(HBoundsCheck* instruction) { Location length_loc = locations->InAt(1); SlowPathCodeX86* slow_path = new (GetGraph()->GetArena()) BoundsCheckSlowPathX86(instruction, index_loc, length_loc); - codegen_->AddSlowPath(slow_path); - Register length = length_loc.AsRegister<Register>(); - if (index_loc.IsConstant()) { - int32_t value = CodeGenerator::GetInt32ValueOf(index_loc.GetConstant()); - __ cmpl(length, Immediate(value)); + if (length_loc.IsConstant()) { + int32_t length = CodeGenerator::GetInt32ValueOf(length_loc.GetConstant()); + if (index_loc.IsConstant()) { + // BCE will remove the bounds check if we are guarenteed to pass. + int32_t index = CodeGenerator::GetInt32ValueOf(index_loc.GetConstant()); + if (index < 0 || index >= length) { + codegen_->AddSlowPath(slow_path); + __ jmp(slow_path->GetEntryLabel()); + } else { + // Some optimization after BCE may have generated this, and we should not + // generate a bounds check if it is a valid range. + } + return; + } + + // We have to reverse the jump condition because the length is the constant. + Register index_reg = index_loc.AsRegister<Register>(); + __ cmpl(index_reg, Immediate(length)); + codegen_->AddSlowPath(slow_path); + __ j(kAboveEqual, slow_path->GetEntryLabel()); } else { - __ cmpl(length, index_loc.AsRegister<Register>()); + Register length = length_loc.AsRegister<Register>(); + if (index_loc.IsConstant()) { + int32_t value = CodeGenerator::GetInt32ValueOf(index_loc.GetConstant()); + __ cmpl(length, Immediate(value)); + } else { + __ cmpl(length, index_loc.AsRegister<Register>()); + } + codegen_->AddSlowPath(slow_path); + __ j(kBelowEqual, slow_path->GetEntryLabel()); } - __ j(kBelowEqual, slow_path->GetEntryLabel()); } void LocationsBuilderX86::VisitTemporary(HTemporary* temp) { diff --git a/compiler/optimizing/code_generator_x86_64.cc b/compiler/optimizing/code_generator_x86_64.cc index 9cf5c218d4..5ce932928b 100644 --- a/compiler/optimizing/code_generator_x86_64.cc +++ b/compiler/optimizing/code_generator_x86_64.cc @@ -1290,8 +1290,12 @@ Location InvokeDexCallingConventionVisitor::GetNextLocation(Primitive::Type type void LocationsBuilderX86_64::VisitInvokeStaticOrDirect(HInvokeStaticOrDirect* invoke) { // Explicit clinit checks triggered by static invokes must have been - // pruned by art::PrepareForRegisterAllocation. - DCHECK(!invoke->IsStaticWithExplicitClinitCheck()); + // pruned by art::PrepareForRegisterAllocation, but this step is not + // run in baseline. So we remove them manually here if we find them. + // TODO: Instead of this local workaround, address this properly. + if (invoke->IsStaticWithExplicitClinitCheck()) { + invoke->RemoveClinitCheckOrLoadClassAsLastInput(); + } IntrinsicLocationsBuilderX86_64 intrinsic(codegen_); if (intrinsic.TryDispatch(invoke)) { @@ -3756,7 +3760,7 @@ void LocationsBuilderX86_64::VisitBoundsCheck(HBoundsCheck* instruction) { LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(instruction, LocationSummary::kNoCall); locations->SetInAt(0, Location::RegisterOrConstant(instruction->InputAt(0))); - locations->SetInAt(1, Location::RequiresRegister()); + locations->SetInAt(1, Location::RegisterOrConstant(instruction->InputAt(1))); if (instruction->HasUses()) { locations->SetOut(Location::SameAsFirstInput()); } @@ -3768,16 +3772,38 @@ void InstructionCodeGeneratorX86_64::VisitBoundsCheck(HBoundsCheck* instruction) Location length_loc = locations->InAt(1); SlowPathCodeX86_64* slow_path = new (GetGraph()->GetArena()) BoundsCheckSlowPathX86_64(instruction, index_loc, length_loc); - codegen_->AddSlowPath(slow_path); - CpuRegister length = length_loc.AsRegister<CpuRegister>(); - if (index_loc.IsConstant()) { - int32_t value = CodeGenerator::GetInt32ValueOf(index_loc.GetConstant()); - __ cmpl(length, Immediate(value)); + if (length_loc.IsConstant()) { + int32_t length = CodeGenerator::GetInt32ValueOf(length_loc.GetConstant()); + if (index_loc.IsConstant()) { + // BCE will remove the bounds check if we are guarenteed to pass. + int32_t index = CodeGenerator::GetInt32ValueOf(index_loc.GetConstant()); + if (index < 0 || index >= length) { + codegen_->AddSlowPath(slow_path); + __ jmp(slow_path->GetEntryLabel()); + } else { + // Some optimization after BCE may have generated this, and we should not + // generate a bounds check if it is a valid range. + } + return; + } + + // We have to reverse the jump condition because the length is the constant. + CpuRegister index_reg = index_loc.AsRegister<CpuRegister>(); + __ cmpl(index_reg, Immediate(length)); + codegen_->AddSlowPath(slow_path); + __ j(kAboveEqual, slow_path->GetEntryLabel()); } else { - __ cmpl(length, index_loc.AsRegister<CpuRegister>()); + CpuRegister length = length_loc.AsRegister<CpuRegister>(); + if (index_loc.IsConstant()) { + int32_t value = CodeGenerator::GetInt32ValueOf(index_loc.GetConstant()); + __ cmpl(length, Immediate(value)); + } else { + __ cmpl(length, index_loc.AsRegister<CpuRegister>()); + } + codegen_->AddSlowPath(slow_path); + __ j(kBelowEqual, slow_path->GetEntryLabel()); } - __ j(kBelowEqual, slow_path->GetEntryLabel()); } void CodeGeneratorX86_64::MarkGCCard(CpuRegister temp, diff --git a/compiler/optimizing/graph_checker.cc b/compiler/optimizing/graph_checker.cc index 890676467f..dc3124b35f 100644 --- a/compiler/optimizing/graph_checker.cc +++ b/compiler/optimizing/graph_checker.cc @@ -121,6 +121,18 @@ void GraphChecker::VisitBasicBlock(HBasicBlock* block) { } } +void GraphChecker::VisitBoundsCheck(HBoundsCheck* check) { + if (!GetGraph()->HasBoundsChecks()) { + AddError(StringPrintf("Instruction %s:%d is a HBoundsCheck, " + "but HasBoundsChecks() returns false", + check->DebugName(), + check->GetId())); + } + + // Perform the instruction base checks too. + VisitInstruction(check); +} + void GraphChecker::VisitInstruction(HInstruction* instruction) { if (seen_ids_.IsBitSet(instruction->GetId())) { AddError(StringPrintf("Instruction id %d is duplicate in graph.", diff --git a/compiler/optimizing/graph_checker.h b/compiler/optimizing/graph_checker.h index 45e8804edb..b4314da03b 100644 --- a/compiler/optimizing/graph_checker.h +++ b/compiler/optimizing/graph_checker.h @@ -45,6 +45,9 @@ class GraphChecker : public HGraphDelegateVisitor { // Perform control-flow graph checks on instruction. void VisitInvokeStaticOrDirect(HInvokeStaticOrDirect* invoke) OVERRIDE; + // Check that the HasBoundsChecks() flag is set for bounds checks. + void VisitBoundsCheck(HBoundsCheck* check) OVERRIDE; + // Was the last visit of the graph valid? bool IsValid() const { return errors_.empty(); diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc index 37b57533c1..ada32db047 100644 --- a/compiler/optimizing/inliner.cc +++ b/compiler/optimizing/inliner.cc @@ -268,8 +268,8 @@ bool HInliner::TryBuildAndInline(Handle<mirror::ArtMethod> resolved_method, callee_graph->InlineInto(graph_, invoke_instruction); - if (callee_graph->HasArrayAccesses()) { - graph_->SetHasArrayAccesses(true); + if (callee_graph->HasBoundsChecks()) { + graph_->SetHasBoundsChecks(true); } return true; diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h index 6b9d72ddf6..938d6fcd64 100644 --- a/compiler/optimizing/nodes.h +++ b/compiler/optimizing/nodes.h @@ -127,7 +127,7 @@ class HGraph : public ArenaObject<kArenaAllocMisc> { number_of_vregs_(0), number_of_in_vregs_(0), temporaries_vreg_slots_(0), - has_array_accesses_(false), + has_bounds_checks_(false), debuggable_(debuggable), current_instruction_id_(start_instruction_id), cached_null_constant_(nullptr), @@ -230,12 +230,12 @@ class HGraph : public ArenaObject<kArenaAllocMisc> { return linear_order_; } - bool HasArrayAccesses() const { - return has_array_accesses_; + bool HasBoundsChecks() const { + return has_bounds_checks_; } - void SetHasArrayAccesses(bool value) { - has_array_accesses_ = value; + void SetHasBoundsChecks(bool value) { + has_bounds_checks_ = value; } bool IsDebuggable() const { return debuggable_; } @@ -295,8 +295,8 @@ class HGraph : public ArenaObject<kArenaAllocMisc> { // Number of vreg size slots that the temporaries use (used in baseline compiler). size_t temporaries_vreg_slots_; - // Has array accesses. We can totally skip BCE if it's false. - bool has_array_accesses_; + // Has bounds checks. We can totally skip BCE if it's false. + bool has_bounds_checks_; // Indicates whether the graph should be compiled in a way that // ensures full debuggability. If false, we can apply more @@ -2262,16 +2262,15 @@ class HInvokeStaticOrDirect : public HInvoke { return GetInvokeType() == kStatic; } - // Remove the art::HLoadClass instruction set as last input by - // art::PrepareForRegisterAllocation::VisitClinitCheck in lieu of - // the initial art::HClinitCheck instruction (only relevant for - // static calls with explicit clinit check). - void RemoveLoadClassAsLastInput() { + // Remove the art::HClinitCheck or art::HLoadClass instruction as + // last input (only relevant for static calls with explicit clinit + // check). + void RemoveClinitCheckOrLoadClassAsLastInput() { DCHECK(IsStaticWithExplicitClinitCheck()); size_t last_input_index = InputCount() - 1; HInstruction* last_input = InputAt(last_input_index); DCHECK(last_input != nullptr); - DCHECK(last_input->IsLoadClass()) << last_input->DebugName(); + DCHECK(last_input->IsClinitCheck() || last_input->IsLoadClass()) << last_input->DebugName(); RemoveAsUserOfInput(last_input_index); inputs_.DeleteAt(last_input_index); clinit_check_requirement_ = ClinitCheckRequirement::kImplicit; diff --git a/compiler/optimizing/prepare_for_register_allocation.cc b/compiler/optimizing/prepare_for_register_allocation.cc index 78d11857c3..fa6b3c292c 100644 --- a/compiler/optimizing/prepare_for_register_allocation.cc +++ b/compiler/optimizing/prepare_for_register_allocation.cc @@ -91,7 +91,7 @@ void PrepareForRegisterAllocation::VisitInvokeStaticOrDirect(HInvokeStaticOrDire // previously) by the graph builder during the creation of the // static invoke instruction, but is no longer required at this // stage (i.e., after inlining has been performed). - invoke->RemoveLoadClassAsLastInput(); + invoke->RemoveClinitCheckOrLoadClassAsLastInput(); // If the load class instruction is no longer used, remove it from // the graph. diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc index 2a3a346a4c..b764095fc5 100644 --- a/dex2oat/dex2oat.cc +++ b/dex2oat/dex2oat.cc @@ -1217,9 +1217,9 @@ class Dex2Oat FINAL { if (!UseSwap(image_, dex_files_)) { close(swap_fd_); swap_fd_ = -1; - LOG(INFO) << "Decided to run without swap."; + VLOG(compiler) << "Decided to run without swap."; } else { - LOG(INFO) << "Accepted running with swap."; + LOG(INFO) << "Large app, accepted running with swap."; } } // Note that dex2oat won't close the swap_fd_. The compiler driver's swap space will do that. diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index dc8bf2ac5e..8a0c315995 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -31,6 +31,7 @@ #include "base/scoped_flock.h" #include "base/stl_util.h" #include "base/unix_file/fd_file.h" +#include "base/value_object.h" #include "class_linker-inl.h" #include "compiler_callbacks.h" #include "debugger.h" @@ -81,6 +82,10 @@ namespace art { static constexpr bool kSanityCheckObjects = kIsDebugBuild; +// Do a simple class redefinition check in OpenDexFilesFromOat. This is a conservative check to +// avoid problems with compile-time class-path != runtime class-path. +static constexpr bool kCheckForDexCollisions = true; + static void ThrowNoClassDefFoundError(const char* fmt, ...) __attribute__((__format__(__printf__, 1, 2))) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); @@ -712,6 +717,186 @@ OatFile& ClassLinker::GetImageOatFile(gc::space::ImageSpace* space) { return *oat_file; } +class DexFileAndClassPair : ValueObject { + public: + DexFileAndClassPair(const DexFile* dex_file, size_t current_class_index, bool from_loaded_oat) + : cached_descriptor_(GetClassDescriptor(dex_file, current_class_index)), + dex_file_(dex_file), + current_class_index_(current_class_index), + from_loaded_oat_(from_loaded_oat) {} + + DexFileAndClassPair(const DexFileAndClassPair&) = default; + + DexFileAndClassPair& operator=(const DexFileAndClassPair& rhs) { + cached_descriptor_ = rhs.cached_descriptor_; + dex_file_ = rhs.dex_file_; + current_class_index_ = rhs.current_class_index_; + from_loaded_oat_ = rhs.from_loaded_oat_; + return *this; + } + + const char* GetCachedDescriptor() const { + return cached_descriptor_; + } + + bool operator<(const DexFileAndClassPair& rhs) const { + const char* lhsDescriptor = cached_descriptor_; + const char* rhsDescriptor = rhs.cached_descriptor_; + int cmp = strcmp(lhsDescriptor, rhsDescriptor); + if (cmp != 0) { + return cmp > 0; + } + return dex_file_ < rhs.dex_file_; + } + + bool DexFileHasMoreClasses() const { + return current_class_index_ + 1 < dex_file_->NumClassDefs(); + } + + DexFileAndClassPair GetNext() const { + return DexFileAndClassPair(dex_file_, current_class_index_ + 1, from_loaded_oat_); + } + + size_t GetCurrentClassIndex() const { + return current_class_index_; + } + + bool FromLoadedOat() const { + return from_loaded_oat_; + } + + const DexFile* GetDexFile() const { + return dex_file_; + } + + private: + static const char* GetClassDescriptor(const DexFile* dex_file, size_t index) { + const DexFile::ClassDef& class_def = dex_file->GetClassDef(static_cast<uint16_t>(index)); + return dex_file->StringByTypeIdx(class_def.class_idx_); + } + + const char* cached_descriptor_; + const DexFile* dex_file_; + size_t current_class_index_; + bool from_loaded_oat_; // We only need to compare mismatches between what we load now + // and what was loaded before. Any old duplicates must have been + // OK, and any new "internal" duplicates are as well (they must + // be from multidex, which resolves correctly). +}; + +static void AddDexFilesFromOat(const OatFile* oat_file, bool already_loaded, + std::priority_queue<DexFileAndClassPair>* heap) { + const std::vector<const OatDexFile*>& oat_dex_files = oat_file->GetOatDexFiles(); + for (const OatDexFile* oat_dex_file : oat_dex_files) { + std::string error; + std::unique_ptr<const DexFile> dex_file = oat_dex_file->OpenDexFile(&error); + if (dex_file.get() == nullptr) { + LOG(WARNING) << "Could not create dex file from oat file: " << error; + } else { + if (dex_file->NumClassDefs() > 0U) { + heap->emplace(dex_file.release(), 0U, already_loaded); + } + } + } +} + +static void AddNext(const DexFileAndClassPair& original, + std::priority_queue<DexFileAndClassPair>* heap) { + if (original.DexFileHasMoreClasses()) { + heap->push(original.GetNext()); + } else { + // Need to delete the dex file. + delete original.GetDexFile(); + } +} + +static void FreeDexFilesInHeap(std::priority_queue<DexFileAndClassPair>* heap) { + while (!heap->empty()) { + delete heap->top().GetDexFile(); + heap->pop(); + } +} + +// Check for class-def collisions in dex files. +// +// This works by maintaining a heap with one class from each dex file, sorted by the class +// descriptor. Then a dex-file/class pair is continually removed from the heap and compared +// against the following top element. If the descriptor is the same, it is now checked whether +// the two elements agree on whether their dex file was from an already-loaded oat-file or the +// new oat file. Any disagreement indicates a collision. +bool ClassLinker::HasCollisions(const OatFile* oat_file, std::string* error_msg) { + if (!kCheckForDexCollisions) { + return false; + } + + // Dex files are registered late - once a class is actually being loaded. We have to compare + // against the open oat files. + ReaderMutexLock mu(Thread::Current(), dex_lock_); + + std::priority_queue<DexFileAndClassPair> heap; + + // Add dex files from already loaded oat files, but skip boot. + { + // To grab the boot oat, look at the dex files in the boot classpath. + const OatFile* boot_oat = nullptr; + if (!boot_class_path_.empty()) { + const DexFile* boot_dex_file = boot_class_path_[0]; + // Is it from an oat file? + if (boot_dex_file->GetOatDexFile() != nullptr) { + boot_oat = boot_dex_file->GetOatDexFile()->GetOatFile(); + } + } + + for (const OatFile* loaded_oat_file : oat_files_) { + if (loaded_oat_file == boot_oat) { + continue; + } + AddDexFilesFromOat(loaded_oat_file, true, &heap); + } + } + + if (heap.empty()) { + // No other oat files, return early. + return false; + } + + // Add dex files from the oat file to check. + AddDexFilesFromOat(oat_file, false, &heap); + + // Now drain the heap. + while (!heap.empty()) { + DexFileAndClassPair compare_pop = heap.top(); + heap.pop(); + + // Compare against the following elements. + while (!heap.empty()) { + DexFileAndClassPair top = heap.top(); + + if (strcmp(compare_pop.GetCachedDescriptor(), top.GetCachedDescriptor()) == 0) { + // Same descriptor. Check whether it's crossing old-oat-files to new-oat-files. + if (compare_pop.FromLoadedOat() != top.FromLoadedOat()) { + *error_msg = + StringPrintf("Found duplicated class when checking oat files: '%s' in %s and %s", + compare_pop.GetCachedDescriptor(), + compare_pop.GetDexFile()->GetLocation().c_str(), + top.GetDexFile()->GetLocation().c_str()); + FreeDexFilesInHeap(&heap); + return true; + } + // Pop it. + heap.pop(); + AddNext(top, &heap); + } else { + // Something else. Done here. + break; + } + } + AddNext(compare_pop, &heap); + } + + return false; +} + std::vector<std::unique_ptr<const DexFile>> ClassLinker::OpenDexFilesFromOat( const char* dex_location, const char* oat_location, std::vector<std::string>* error_msgs) { @@ -757,8 +942,20 @@ std::vector<std::unique_ptr<const DexFile>> ClassLinker::OpenDexFilesFromOat( // Get the oat file on disk. std::unique_ptr<OatFile> oat_file = oat_file_assistant.GetBestOatFile(); if (oat_file.get() != nullptr) { - source_oat_file = oat_file.release(); - RegisterOatFile(source_oat_file); + // Take the file only if it has no collisions. + if (!HasCollisions(oat_file.get(), &error_msg)) { + source_oat_file = oat_file.release(); + RegisterOatFile(source_oat_file); + } else { + if (Runtime::Current()->IsDexFileFallbackEnabled()) { + LOG(WARNING) << "Found duplicate classes, falling back to interpreter mode for " + << dex_location; + } else { + LOG(WARNING) << "Found duplicate classes, dex-file-fallback disabled, will be failing to " + " load classes for " << dex_location; + } + LOG(WARNING) << error_msg; + } } } diff --git a/runtime/class_linker.h b/runtime/class_linker.h index 1bd9f0a7e9..57989b272b 100644 --- a/runtime/class_linker.h +++ b/runtime/class_linker.h @@ -663,6 +663,9 @@ class ClassLinker { // a recreation with a custom string. void ThrowEarlierClassFailure(mirror::Class* c) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); + // Check for duplicate class definitions of the given oat file against all open oat files. + bool HasCollisions(const OatFile* oat_file, std::string* error_msg) LOCKS_EXCLUDED(dex_lock_); + std::vector<const DexFile*> boot_class_path_; std::vector<std::unique_ptr<const DexFile>> opened_dex_files_; diff --git a/runtime/dex_file_verifier.cc b/runtime/dex_file_verifier.cc index 2603975910..a66c38e0fe 100644 --- a/runtime/dex_file_verifier.cc +++ b/runtime/dex_file_verifier.cc @@ -944,7 +944,7 @@ bool DexFileVerifier::CheckIntraDebugInfoItem() { uint32_t type_idx = DecodeUnsignedLeb128(&ptr_); if (type_idx != 0) { type_idx--; - if (!CheckIndex(type_idx, header_->string_ids_size_, "DBG_START_LOCAL type_idx")) { + if (!CheckIndex(type_idx, header_->type_ids_size_, "DBG_START_LOCAL type_idx")) { return false; } } @@ -975,7 +975,7 @@ bool DexFileVerifier::CheckIntraDebugInfoItem() { uint32_t type_idx = DecodeUnsignedLeb128(&ptr_); if (type_idx != 0) { type_idx--; - if (!CheckIndex(type_idx, header_->string_ids_size_, "DBG_START_LOCAL_EXTENDED type_idx")) { + if (!CheckIndex(type_idx, header_->type_ids_size_, "DBG_START_LOCAL_EXTENDED type_idx")) { return false; } } diff --git a/runtime/dex_file_verifier_test.cc b/runtime/dex_file_verifier_test.cc index 95a47cc6e9..9f1ffec35f 100644 --- a/runtime/dex_file_verifier_test.cc +++ b/runtime/dex_file_verifier_test.cc @@ -200,11 +200,11 @@ static std::unique_ptr<const DexFile> FixChecksumAndOpen(uint8_t* bytes, size_t return dex_file; } -static bool ModifyAndLoad(const char* location, size_t offset, uint8_t new_val, - std::string* error_msg) { +static bool ModifyAndLoad(const char* dex_file_content, const char* location, size_t offset, + uint8_t new_val, std::string* error_msg) { // Decode base64. size_t length; - std::unique_ptr<uint8_t[]> dex_bytes(DecodeBase64(kGoodTestDex, &length)); + std::unique_ptr<uint8_t[]> dex_bytes(DecodeBase64(dex_file_content, &length)); CHECK(dex_bytes.get() != nullptr); // Make modifications. @@ -221,7 +221,7 @@ TEST_F(DexFileVerifierTest, MethodId) { // Class error. ScratchFile tmp; std::string error_msg; - bool success = !ModifyAndLoad(tmp.GetFilename().c_str(), 220, 0xFFU, &error_msg); + bool success = !ModifyAndLoad(kGoodTestDex, tmp.GetFilename().c_str(), 220, 0xFFU, &error_msg); ASSERT_TRUE(success); ASSERT_NE(error_msg.find("inter_method_id_item class_idx"), std::string::npos) << error_msg; } @@ -230,7 +230,7 @@ TEST_F(DexFileVerifierTest, MethodId) { // Proto error. ScratchFile tmp; std::string error_msg; - bool success = !ModifyAndLoad(tmp.GetFilename().c_str(), 222, 0xFFU, &error_msg); + bool success = !ModifyAndLoad(kGoodTestDex, tmp.GetFilename().c_str(), 222, 0xFFU, &error_msg); ASSERT_TRUE(success); ASSERT_NE(error_msg.find("inter_method_id_item proto_idx"), std::string::npos) << error_msg; } @@ -239,10 +239,81 @@ TEST_F(DexFileVerifierTest, MethodId) { // Name error. ScratchFile tmp; std::string error_msg; - bool success = !ModifyAndLoad(tmp.GetFilename().c_str(), 224, 0xFFU, &error_msg); + bool success = !ModifyAndLoad(kGoodTestDex, tmp.GetFilename().c_str(), 224, 0xFFU, &error_msg); ASSERT_TRUE(success); ASSERT_NE(error_msg.find("inter_method_id_item name_idx"), std::string::npos) << error_msg; } } +// Generated from: +// +// .class public LTest; +// .super Ljava/lang/Object; +// .source "Test.java" +// +// .method public constructor <init>()V +// .registers 1 +// +// .prologue +// .line 1 +// invoke-direct {p0}, Ljava/lang/Object;-><init>()V +// +// return-void +// .end method +// +// .method public static main()V +// .registers 2 +// +// const-string v0, "a" +// const-string v0, "b" +// const-string v0, "c" +// const-string v0, "d" +// const-string v0, "e" +// const-string v0, "f" +// const-string v0, "g" +// const-string v0, "h" +// const-string v0, "i" +// const-string v0, "j" +// const-string v0, "k" +// +// .local v1, "local_var":Ljava/lang/String; +// const-string v1, "test" +// .end method + +static const char kDebugInfoTestDex[] = + "ZGV4CjAzNQCHRkHix2eIMQgvLD/0VGrlllZLo0Rb6VyUAgAAcAAAAHhWNBIAAAAAAAAAAAwCAAAU" + "AAAAcAAAAAQAAADAAAAAAQAAANAAAAAAAAAAAAAAAAMAAADcAAAAAQAAAPQAAACAAQAAFAEAABQB" + "AAAcAQAAJAEAADgBAABMAQAAVwEAAFoBAABdAQAAYAEAAGMBAABmAQAAaQEAAGwBAABvAQAAcgEA" + "AHUBAAB4AQAAewEAAIYBAACMAQAAAQAAAAIAAAADAAAABQAAAAUAAAADAAAAAAAAAAAAAAAAAAAA" + "AAAAABIAAAABAAAAAAAAAAAAAAABAAAAAQAAAAAAAAAEAAAAAAAAAPwBAAAAAAAABjxpbml0PgAG" + "TFRlc3Q7ABJMamF2YS9sYW5nL09iamVjdDsAEkxqYXZhL2xhbmcvU3RyaW5nOwAJVGVzdC5qYXZh" + "AAFWAAFhAAFiAAFjAAFkAAFlAAFmAAFnAAFoAAFpAAFqAAFrAAlsb2NhbF92YXIABG1haW4ABHRl" + "c3QAAAABAAcOAAAAARYDARIDAAAAAQABAAEAAACUAQAABAAAAHAQAgAAAA4AAgAAAAAAAACZAQAA" + "GAAAABoABgAaAAcAGgAIABoACQAaAAoAGgALABoADAAaAA0AGgAOABoADwAaABAAGgETAAAAAgAA" + "gYAEpAMBCbwDAAALAAAAAAAAAAEAAAAAAAAAAQAAABQAAABwAAAAAgAAAAQAAADAAAAAAwAAAAEA" + "AADQAAAABQAAAAMAAADcAAAABgAAAAEAAAD0AAAAAiAAABQAAAAUAQAAAyAAAAIAAACUAQAAASAA" + "AAIAAACkAQAAACAAAAEAAAD8AQAAABAAAAEAAAAMAgAA"; + +TEST_F(DexFileVerifierTest, DebugInfoTypeIdxTest) { + { + // The input dex file should be good before modification. + ScratchFile tmp; + std::string error_msg; + std::unique_ptr<const DexFile> raw(OpenDexFileBase64(kDebugInfoTestDex, + tmp.GetFilename().c_str(), + &error_msg)); + ASSERT_TRUE(raw.get() != nullptr) << error_msg; + } + + { + // Modify the debug information entry. + ScratchFile tmp; + std::string error_msg; + bool success = !ModifyAndLoad(kDebugInfoTestDex, tmp.GetFilename().c_str(), 416, 0x14U, + &error_msg); + ASSERT_TRUE(success); + ASSERT_NE(error_msg.find("DBG_START_LOCAL type_idx"), std::string::npos) << error_msg; + } +} + } // namespace art diff --git a/runtime/native/dalvik_system_ZygoteHooks.cc b/runtime/native/dalvik_system_ZygoteHooks.cc index af01a02787..8524348164 100644 --- a/runtime/native/dalvik_system_ZygoteHooks.cc +++ b/runtime/native/dalvik_system_ZygoteHooks.cc @@ -65,6 +65,7 @@ static void EnableDebugFeatures(uint32_t debug_flags) { DEBUG_ENABLE_SAFEMODE = 1 << 3, DEBUG_ENABLE_JNI_LOGGING = 1 << 4, DEBUG_ENABLE_JIT = 1 << 5, + DEBUG_GENERATE_CFI = 1 << 6, }; Runtime* const runtime = Runtime::Current(); @@ -111,6 +112,12 @@ static void EnableDebugFeatures(uint32_t debug_flags) { } runtime->GetJITOptions()->SetUseJIT(use_jit); + const bool generate_cfi = (debug_flags & DEBUG_GENERATE_CFI) != 0; + if (generate_cfi) { + runtime->AddCompilerOption("--include-cfi"); + debug_flags &= ~DEBUG_GENERATE_CFI; + } + // This is for backwards compatibility with Dalvik. debug_flags &= ~DEBUG_ENABLE_ASSERT; diff --git a/runtime/thread.cc b/runtime/thread.cc index b27ad4ae3e..9f7c303af9 100644 --- a/runtime/thread.cc +++ b/runtime/thread.cc @@ -572,13 +572,13 @@ void Thread::ShortDump(std::ostream& os) const { if (GetThreadId() != 0) { // If we're in kStarting, we won't have a thin lock id or tid yet. os << GetThreadId() - << ",tid=" << GetTid() << ','; + << ",tid=" << GetTid() << ','; } os << GetState() - << ",Thread*=" << this - << ",peer=" << tlsPtr_.opeer - << ",\"" << *tlsPtr_.name << "\"" - << "]"; + << ",Thread*=" << this + << ",peer=" << tlsPtr_.opeer + << ",\"" << (tlsPtr_.name != nullptr ? *tlsPtr_.name : "null") << "\"" + << "]"; } void Thread::Dump(std::ostream& os) const { diff --git a/test/138-duplicate-classes-check/build b/test/138-duplicate-classes-check/build new file mode 100755 index 0000000000..7ddc81d9b8 --- /dev/null +++ b/test/138-duplicate-classes-check/build @@ -0,0 +1,31 @@ +#!/bin/bash +# +# Copyright (C) 2015 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. + +# Stop if something fails. +set -e + +mkdir classes +${JAVAC} -d classes `find src -name '*.java'` + +mkdir classes-ex +${JAVAC} -d classes-ex `find src-ex -name '*.java'` + +if [ ${NEED_DEX} = "true" ]; then + ${DX} -JXmx256m --debug --dex --dump-to=classes.lst --output=classes.dex --dump-width=1000 classes + zip $TEST_NAME.jar classes.dex + ${DX} -JXmx256m --debug --dex --dump-to=classes-ex.lst --output=classes.dex --dump-width=1000 classes-ex + zip ${TEST_NAME}-ex.jar classes.dex +fi diff --git a/test/138-duplicate-classes-check/expected.txt b/test/138-duplicate-classes-check/expected.txt new file mode 100644 index 0000000000..b2f7f08c17 --- /dev/null +++ b/test/138-duplicate-classes-check/expected.txt @@ -0,0 +1,2 @@ +10 +10 diff --git a/test/138-duplicate-classes-check/info.txt b/test/138-duplicate-classes-check/info.txt new file mode 100644 index 0000000000..22a66a26fd --- /dev/null +++ b/test/138-duplicate-classes-check/info.txt @@ -0,0 +1 @@ +Check whether a duplicate class is detected. diff --git a/test/138-duplicate-classes-check/src-ex/A.java b/test/138-duplicate-classes-check/src-ex/A.java new file mode 100644 index 0000000000..8e52cb3a5d --- /dev/null +++ b/test/138-duplicate-classes-check/src-ex/A.java @@ -0,0 +1,23 @@ +/* + * Copyright (C) 2015 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 A { + public volatile int i; + + public A() { + i = 10; + } +} diff --git a/test/138-duplicate-classes-check/src-ex/TestEx.java b/test/138-duplicate-classes-check/src-ex/TestEx.java new file mode 100644 index 0000000000..87558fafac --- /dev/null +++ b/test/138-duplicate-classes-check/src-ex/TestEx.java @@ -0,0 +1,21 @@ +/* + * Copyright (C) 2015 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 TestEx { + public static void test() { + System.out.println(new A().i); + } +} diff --git a/test/138-duplicate-classes-check/src/A.java b/test/138-duplicate-classes-check/src/A.java new file mode 100644 index 0000000000..e1773e5bc6 --- /dev/null +++ b/test/138-duplicate-classes-check/src/A.java @@ -0,0 +1,28 @@ +/* + * Copyright (C) 2015 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 A { + // Object fields add padding in the Foo class object layout. Therefore the field 'i' should + // be at a different offset compared to the A class from the ex DEX file. + public final Object anObject = null; + public final Object anotherObject = null; + // Use volatile to defeat inlining of the constructor + load-elimination. + public volatile int i; + + public A() { + i = 10; + } +} diff --git a/test/138-duplicate-classes-check/src/FancyLoader.java b/test/138-duplicate-classes-check/src/FancyLoader.java new file mode 100644 index 0000000000..03ec948767 --- /dev/null +++ b/test/138-duplicate-classes-check/src/FancyLoader.java @@ -0,0 +1,229 @@ +/* + * Copyright (C) 2008 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.io.File; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.RandomAccessFile; +import java.lang.reflect.Constructor; +import java.lang.reflect.Method; +import java.lang.reflect.InvocationTargetException; + +/** + * A class loader with atypical behavior: we try to load a private + * class implementation before asking the system or boot loader. This + * is used to create multiple classes with identical names in a single VM. + * + * If DexFile is available, we use that; if not, we assume we're not in + * Dalvik and instantiate the class with defineClass(). + * + * The location of the DEX files and class data is dependent upon the + * test framework. + */ +public class FancyLoader extends ClassLoader { + /* this is where the "alternate" .class files live */ + static final String CLASS_PATH = "classes-ex/"; + + /* this is the "alternate" DEX/Jar file */ + static final String DEX_FILE = System.getenv("DEX_LOCATION") + + "/138-duplicate-classes-check-ex.jar"; + + /* on Dalvik, this is a DexFile; otherwise, it's null */ + private Class mDexClass; + + private Object mDexFile; + + /** + * Construct FancyLoader, grabbing a reference to the DexFile class + * if we're running under Dalvik. + */ + public FancyLoader(ClassLoader parent) { + super(parent); + + try { + mDexClass = parent.loadClass("dalvik.system.DexFile"); + } catch (ClassNotFoundException cnfe) { + // ignore -- not running Dalvik + } + } + + /** + * Finds the class with the specified binary name. + * + * We search for a file in CLASS_PATH or pull an entry from DEX_FILE. + * If we don't find a match, we throw an exception. + */ + protected Class<?> findClass(String name) throws ClassNotFoundException + { + if (mDexClass != null) { + return findClassDalvik(name); + } else { + return findClassNonDalvik(name); + } + } + + /** + * Finds the class with the specified binary name, from a DEX file. + */ + private Class<?> findClassDalvik(String name) + throws ClassNotFoundException { + + if (mDexFile == null) { + synchronized (FancyLoader.class) { + Constructor ctor; + /* + * Construct a DexFile object through reflection. + */ + try { + ctor = mDexClass.getConstructor(new Class[] {String.class}); + } catch (NoSuchMethodException nsme) { + throw new ClassNotFoundException("getConstructor failed", + nsme); + } + + try { + mDexFile = ctor.newInstance(DEX_FILE); + } catch (InstantiationException ie) { + throw new ClassNotFoundException("newInstance failed", ie); + } catch (IllegalAccessException iae) { + throw new ClassNotFoundException("newInstance failed", iae); + } catch (InvocationTargetException ite) { + throw new ClassNotFoundException("newInstance failed", ite); + } + } + } + + /* + * Call DexFile.loadClass(String, ClassLoader). + */ + Method meth; + + try { + meth = mDexClass.getMethod("loadClass", + new Class[] { String.class, ClassLoader.class }); + } catch (NoSuchMethodException nsme) { + throw new ClassNotFoundException("getMethod failed", nsme); + } + + try { + meth.invoke(mDexFile, name, this); + } catch (IllegalAccessException iae) { + throw new ClassNotFoundException("loadClass failed", iae); + } catch (InvocationTargetException ite) { + throw new ClassNotFoundException("loadClass failed", + ite.getCause()); + } + + return null; + } + + /** + * Finds the class with the specified binary name, from .class files. + */ + private Class<?> findClassNonDalvik(String name) + throws ClassNotFoundException { + + String pathName = CLASS_PATH + name + ".class"; + //System.out.println("--- Fancy: looking for " + pathName); + + File path = new File(pathName); + RandomAccessFile raf; + + try { + raf = new RandomAccessFile(path, "r"); + } catch (FileNotFoundException fnfe) { + throw new ClassNotFoundException("Not found: " + pathName); + } + + /* read the entire file in */ + byte[] fileData; + try { + fileData = new byte[(int) raf.length()]; + raf.readFully(fileData); + } catch (IOException ioe) { + throw new ClassNotFoundException("Read error: " + pathName); + } finally { + try { + raf.close(); + } catch (IOException ioe) { + // drop + } + } + + /* create the class */ + //System.out.println("--- Fancy: defining " + name); + try { + return defineClass(name, fileData, 0, fileData.length); + } catch (Throwable th) { + throw new ClassNotFoundException("defineClass failed", th); + } + } + + /** + * Load a class. + * + * Normally a class loader wouldn't override this, but we want our + * version of the class to take precedence over an already-loaded + * version. + * + * We still want the system classes (e.g. java.lang.Object) from the + * bootstrap class loader. + */ + protected Class<?> loadClass(String name, boolean resolve) + throws ClassNotFoundException + { + Class res; + + /* + * 1. Invoke findLoadedClass(String) to check if the class has + * already been loaded. + * + * This doesn't change. + */ + res = findLoadedClass(name); + if (res != null) { + System.out.println("FancyLoader.loadClass: " + + name + " already loaded"); + if (resolve) + resolveClass(res); + return res; + } + + /* + * 3. Invoke the findClass(String) method to find the class. + */ + try { + res = findClass(name); + if (resolve) + resolveClass(res); + } + catch (ClassNotFoundException e) { + // we couldn't find it, so eat the exception and keep going + } + + /* + * 2. Invoke the loadClass method on the parent class loader. If + * the parent loader is null the class loader built-in to the + * virtual machine is used, instead. + * + * (Since we're not in java.lang, we can't actually invoke the + * parent's loadClass() method, but we passed our parent to the + * super-class which can take care of it for us.) + */ + res = super.loadClass(name, resolve); // returns class or throws + return res; + } +} diff --git a/test/138-duplicate-classes-check/src/Main.java b/test/138-duplicate-classes-check/src/Main.java new file mode 100644 index 0000000000..a9b5bb04ea --- /dev/null +++ b/test/138-duplicate-classes-check/src/Main.java @@ -0,0 +1,43 @@ +/* + * Copyright (C) 2015 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.io.File; +import java.lang.reflect.Method; + +/** + * Structural hazard test. + */ +public class Main { + public static void main(String[] args) { + new Main().run(); + } + + private void run() { + System.out.println(new A().i); + + // Now run the class from the -ex file. + + FancyLoader loader = new FancyLoader(getClass().getClassLoader()); + + try { + Class testEx = loader.loadClass("TestEx"); + Method test = testEx.getDeclaredMethod("test"); + test.invoke(null); + } catch (Exception exc) { + exc.printStackTrace(); + } + } +} diff --git a/test/138-duplicate-classes-check2/build b/test/138-duplicate-classes-check2/build new file mode 100755 index 0000000000..abcbbb8437 --- /dev/null +++ b/test/138-duplicate-classes-check2/build @@ -0,0 +1,32 @@ +#!/bin/bash +# +# Copyright (C) 2015 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. + +# Stop if something fails. +set -e + +mkdir classes +${JAVAC} -d classes `find src -name '*.java'` + +mkdir classes-ex +${JAVAC} -d classes-ex `find src-ex -name '*.java'` +rm classes-ex/A.class + +if [ ${NEED_DEX} = "true" ]; then + ${DX} -JXmx256m --debug --dex --dump-to=classes.lst --output=classes.dex --dump-width=1000 classes + zip $TEST_NAME.jar classes.dex + ${DX} -JXmx256m --debug --dex --dump-to=classes-ex.lst --output=classes.dex --dump-width=1000 classes-ex + zip ${TEST_NAME}-ex.jar classes.dex +fi diff --git a/test/138-duplicate-classes-check2/expected.txt b/test/138-duplicate-classes-check2/expected.txt new file mode 100644 index 0000000000..b2f7f08c17 --- /dev/null +++ b/test/138-duplicate-classes-check2/expected.txt @@ -0,0 +1,2 @@ +10 +10 diff --git a/test/138-duplicate-classes-check2/info.txt b/test/138-duplicate-classes-check2/info.txt new file mode 100644 index 0000000000..7100122a4d --- /dev/null +++ b/test/138-duplicate-classes-check2/info.txt @@ -0,0 +1,2 @@ +Check whether a duplicate class is not detected, even though we compiled against one (but removed +it before creating the dex file). diff --git a/test/138-duplicate-classes-check2/run b/test/138-duplicate-classes-check2/run new file mode 100755 index 0000000000..8494ad9aad --- /dev/null +++ b/test/138-duplicate-classes-check2/run @@ -0,0 +1,19 @@ +#!/bin/bash +# +# Copyright (C) 2015 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. + +# We want to run as no-dex-file-fallback to confirm that even though the -ex file has a symbolic +# reference to A, there's no class-def, so we don't detect a collision. +exec ${RUN} --runtime-option -Xno-dex-file-fallback "${@}" diff --git a/test/138-duplicate-classes-check2/src-ex/A.java b/test/138-duplicate-classes-check2/src-ex/A.java new file mode 100644 index 0000000000..8e52cb3a5d --- /dev/null +++ b/test/138-duplicate-classes-check2/src-ex/A.java @@ -0,0 +1,23 @@ +/* + * Copyright (C) 2015 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 A { + public volatile int i; + + public A() { + i = 10; + } +} diff --git a/test/138-duplicate-classes-check2/src-ex/TestEx.java b/test/138-duplicate-classes-check2/src-ex/TestEx.java new file mode 100644 index 0000000000..87558fafac --- /dev/null +++ b/test/138-duplicate-classes-check2/src-ex/TestEx.java @@ -0,0 +1,21 @@ +/* + * Copyright (C) 2015 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 TestEx { + public static void test() { + System.out.println(new A().i); + } +} diff --git a/test/138-duplicate-classes-check2/src/A.java b/test/138-duplicate-classes-check2/src/A.java new file mode 100644 index 0000000000..e1773e5bc6 --- /dev/null +++ b/test/138-duplicate-classes-check2/src/A.java @@ -0,0 +1,28 @@ +/* + * Copyright (C) 2015 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 A { + // Object fields add padding in the Foo class object layout. Therefore the field 'i' should + // be at a different offset compared to the A class from the ex DEX file. + public final Object anObject = null; + public final Object anotherObject = null; + // Use volatile to defeat inlining of the constructor + load-elimination. + public volatile int i; + + public A() { + i = 10; + } +} diff --git a/test/138-duplicate-classes-check2/src/FancyLoader.java b/test/138-duplicate-classes-check2/src/FancyLoader.java new file mode 100644 index 0000000000..7e2bb08a5c --- /dev/null +++ b/test/138-duplicate-classes-check2/src/FancyLoader.java @@ -0,0 +1,229 @@ +/* + * Copyright (C) 2008 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.io.File; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.RandomAccessFile; +import java.lang.reflect.Constructor; +import java.lang.reflect.Method; +import java.lang.reflect.InvocationTargetException; + +/** + * A class loader with atypical behavior: we try to load a private + * class implementation before asking the system or boot loader. This + * is used to create multiple classes with identical names in a single VM. + * + * If DexFile is available, we use that; if not, we assume we're not in + * Dalvik and instantiate the class with defineClass(). + * + * The location of the DEX files and class data is dependent upon the + * test framework. + */ +public class FancyLoader extends ClassLoader { + /* this is where the "alternate" .class files live */ + static final String CLASS_PATH = "classes-ex/"; + + /* this is the "alternate" DEX/Jar file */ + static final String DEX_FILE = System.getenv("DEX_LOCATION") + + "/138-duplicate-classes-check2-ex.jar"; + + /* on Dalvik, this is a DexFile; otherwise, it's null */ + private Class mDexClass; + + private Object mDexFile; + + /** + * Construct FancyLoader, grabbing a reference to the DexFile class + * if we're running under Dalvik. + */ + public FancyLoader(ClassLoader parent) { + super(parent); + + try { + mDexClass = parent.loadClass("dalvik.system.DexFile"); + } catch (ClassNotFoundException cnfe) { + // ignore -- not running Dalvik + } + } + + /** + * Finds the class with the specified binary name. + * + * We search for a file in CLASS_PATH or pull an entry from DEX_FILE. + * If we don't find a match, we throw an exception. + */ + protected Class<?> findClass(String name) throws ClassNotFoundException + { + if (mDexClass != null) { + return findClassDalvik(name); + } else { + return findClassNonDalvik(name); + } + } + + /** + * Finds the class with the specified binary name, from a DEX file. + */ + private Class<?> findClassDalvik(String name) + throws ClassNotFoundException { + + if (mDexFile == null) { + synchronized (FancyLoader.class) { + Constructor ctor; + /* + * Construct a DexFile object through reflection. + */ + try { + ctor = mDexClass.getConstructor(new Class[] {String.class}); + } catch (NoSuchMethodException nsme) { + throw new ClassNotFoundException("getConstructor failed", + nsme); + } + + try { + mDexFile = ctor.newInstance(DEX_FILE); + } catch (InstantiationException ie) { + throw new ClassNotFoundException("newInstance failed", ie); + } catch (IllegalAccessException iae) { + throw new ClassNotFoundException("newInstance failed", iae); + } catch (InvocationTargetException ite) { + throw new ClassNotFoundException("newInstance failed", ite); + } + } + } + + /* + * Call DexFile.loadClass(String, ClassLoader). + */ + Method meth; + + try { + meth = mDexClass.getMethod("loadClass", + new Class[] { String.class, ClassLoader.class }); + } catch (NoSuchMethodException nsme) { + throw new ClassNotFoundException("getMethod failed", nsme); + } + + try { + meth.invoke(mDexFile, name, this); + } catch (IllegalAccessException iae) { + throw new ClassNotFoundException("loadClass failed", iae); + } catch (InvocationTargetException ite) { + throw new ClassNotFoundException("loadClass failed", + ite.getCause()); + } + + return null; + } + + /** + * Finds the class with the specified binary name, from .class files. + */ + private Class<?> findClassNonDalvik(String name) + throws ClassNotFoundException { + + String pathName = CLASS_PATH + name + ".class"; + //System.out.println("--- Fancy: looking for " + pathName); + + File path = new File(pathName); + RandomAccessFile raf; + + try { + raf = new RandomAccessFile(path, "r"); + } catch (FileNotFoundException fnfe) { + throw new ClassNotFoundException("Not found: " + pathName); + } + + /* read the entire file in */ + byte[] fileData; + try { + fileData = new byte[(int) raf.length()]; + raf.readFully(fileData); + } catch (IOException ioe) { + throw new ClassNotFoundException("Read error: " + pathName); + } finally { + try { + raf.close(); + } catch (IOException ioe) { + // drop + } + } + + /* create the class */ + //System.out.println("--- Fancy: defining " + name); + try { + return defineClass(name, fileData, 0, fileData.length); + } catch (Throwable th) { + throw new ClassNotFoundException("defineClass failed", th); + } + } + + /** + * Load a class. + * + * Normally a class loader wouldn't override this, but we want our + * version of the class to take precedence over an already-loaded + * version. + * + * We still want the system classes (e.g. java.lang.Object) from the + * bootstrap class loader. + */ + protected Class<?> loadClass(String name, boolean resolve) + throws ClassNotFoundException + { + Class res; + + /* + * 1. Invoke findLoadedClass(String) to check if the class has + * already been loaded. + * + * This doesn't change. + */ + res = findLoadedClass(name); + if (res != null) { + System.out.println("FancyLoader.loadClass: " + + name + " already loaded"); + if (resolve) + resolveClass(res); + return res; + } + + /* + * 3. Invoke the findClass(String) method to find the class. + */ + try { + res = findClass(name); + if (resolve) + resolveClass(res); + } + catch (ClassNotFoundException e) { + // we couldn't find it, so eat the exception and keep going + } + + /* + * 2. Invoke the loadClass method on the parent class loader. If + * the parent loader is null the class loader built-in to the + * virtual machine is used, instead. + * + * (Since we're not in java.lang, we can't actually invoke the + * parent's loadClass() method, but we passed our parent to the + * super-class which can take care of it for us.) + */ + res = super.loadClass(name, resolve); // returns class or throws + return res; + } +} diff --git a/test/138-duplicate-classes-check2/src/Main.java b/test/138-duplicate-classes-check2/src/Main.java new file mode 100644 index 0000000000..a9b5bb04ea --- /dev/null +++ b/test/138-duplicate-classes-check2/src/Main.java @@ -0,0 +1,43 @@ +/* + * Copyright (C) 2015 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.io.File; +import java.lang.reflect.Method; + +/** + * Structural hazard test. + */ +public class Main { + public static void main(String[] args) { + new Main().run(); + } + + private void run() { + System.out.println(new A().i); + + // Now run the class from the -ex file. + + FancyLoader loader = new FancyLoader(getClass().getClassLoader()); + + try { + Class testEx = loader.loadClass("TestEx"); + Method test = testEx.getDeclaredMethod("test"); + test.invoke(null); + } catch (Exception exc) { + exc.printStackTrace(); + } + } +} diff --git a/test/Android.run-test.mk b/test/Android.run-test.mk index c5abd4625c..93340fb810 100644 --- a/test/Android.run-test.mk +++ b/test/Android.run-test.mk @@ -95,7 +95,7 @@ ifeq ($(ART_TEST_RUN_TEST_NO_RELOCATE),true) RELOCATE_TYPES += no-relocate endif ifeq ($(ART_TEST_RUN_TEST_RELOCATE_NO_PATCHOAT),true) - RELOCATE_TYPES := relocate-npatchoat + RELOCATE_TYPES += relocate-npatchoat endif TRACE_TYPES := ntrace ifeq ($(ART_TEST_TRACE),true) @@ -250,6 +250,12 @@ ART_TEST_KNOWN_BROKEN += $(call all-run-test-names,target,$(RUN_TYPES),$(PREBUIL $(COMPILER_TYPES),$(RELOCATE_TYPES),$(TRACE_TYPES),$(GC_TYPES),$(JNI_TYPES),$(IMAGE_TYPES), \ $(PICTEST_TYPES),$(DEBUGGABLE_TYPES),130-hprof,$(ALL_ADDRESS_SIZES)) +# 131 is an old test. The functionality has been implemented at an earlier stage and is checked +# in tests 138. +ART_TEST_KNOWN_BROKEN += $(call all-run-test-names,$(TARGET_TYPES),$(RUN_TYPES),$(PREBUILD_TYPES), \ + $(COMPILER_TYPES),$(RELOCATE_TYPES),$(TRACE_TYPES),$(GC_TYPES),$(JNI_TYPES),$(IMAGE_TYPES), \ + $(PICTEST_TYPES),$(DEBUGGABLE_TYPES),131-structural-change,$(ALL_ADDRESS_SIZES)) + # All these tests check that we have sane behavior if we don't have a patchoat or dex2oat. # Therefore we shouldn't run them in situations where we actually don't have these since they # explicitly test for them. These all also assume we have an image. @@ -257,7 +263,12 @@ TEST_ART_BROKEN_FALLBACK_RUN_TESTS := \ 116-nodex2oat \ 117-nopatchoat \ 118-noimage-dex2oat \ - 119-noimage-patchoat + 119-noimage-patchoat \ + 138-duplicate-classes-check2 + +# This test fails without an image. +TEST_ART_BROKEN_NO_IMAGE_RUN_TESTS := \ + 138-duplicate-classes-check ifneq (,$(filter no-dex2oat,$(PREBUILD_TYPES))) ART_TEST_KNOWN_BROKEN += $(call all-run-test-names,$(TARGET_TYPES),$(RUN_TYPES),no-dex2oat, \ @@ -270,6 +281,9 @@ ifneq (,$(filter no-image,$(IMAGE_TYPES))) ART_TEST_KNOWN_BROKEN += $(call all-run-test-names,$(TARGET_TYPES),$(RUN_TYPES),$(PREBUILD_TYPES), \ $(COMPILER_TYPES), $(RELOCATE_TYPES),$(TRACE_TYPES),$(GC_TYPES),$(JNI_TYPES),no-image, \ $(PICTEST_TYPES), $(DEBUGGABLE_TYPES), $(TEST_ART_BROKEN_FALLBACK_RUN_TESTS),$(ALL_ADDRESS_SIZES)) + ART_TEST_KNOWN_BROKEN += $(call all-run-test-names,$(TARGET_TYPES),$(RUN_TYPES),$(PREBUILD_TYPES), \ + $(COMPILER_TYPES), $(RELOCATE_TYPES),$(TRACE_TYPES),$(GC_TYPES),$(JNI_TYPES),no-image, \ + $(PICTEST_TYPES), $(DEBUGGABLE_TYPES), $(TEST_ART_BROKEN_NO_IMAGE_RUN_TESTS),$(ALL_ADDRESS_SIZES)) endif ifneq (,$(filter relocate-npatchoat,$(RELOCATE_TYPES))) |