From 8b3f9b246d5bdbf67faeb2b872b75b8d72777bc0 Mon Sep 17 00:00:00 2001 From: Aart Bik Date: Wed, 6 Apr 2016 11:22:12 -0700 Subject: [PATCH] Avoid constructing types with errors. BUG=27626735 Rationale: Do not construct classes with a link error. Without this, the error type thought it was Object (mirror's method IsObjectClass() returns true if there is no superclass). Change-Id: I55ca8cc8cfc042210edf748aab10da4c6e345980 --- build/Android.gtest.mk | 1 + compiler/optimizing/inliner.cc | 3 +- compiler/optimizing/nodes.cc | 1 + compiler/optimizing/reference_type_propagation.cc | 15 +- compiler/optimizing/reference_type_propagation.h | 2 + .../optimizing/reference_type_propagation_test.cc | 190 +++++++++++++++++++++ 6 files changed, 206 insertions(+), 6 deletions(-) create mode 100644 compiler/optimizing/reference_type_propagation_test.cc diff --git a/build/Android.gtest.mk b/build/Android.gtest.mk index 8309bd854..ab01d1382 100644 --- a/build/Android.gtest.mk +++ b/build/Android.gtest.mk @@ -269,6 +269,7 @@ COMPILER_GTEST_COMMON_SRC_FILES := \ compiler/optimizing/nodes_test.cc \ compiler/optimizing/parallel_move_test.cc \ compiler/optimizing/pretty_printer_test.cc \ + compiler/optimizing/reference_type_propagation_test.cc \ compiler/optimizing/side_effects_test.cc \ compiler/optimizing/ssa_test.cc \ compiler/optimizing/stack_map_test.cc \ diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc index 77e0cbc60..ba6ae6ffb 100644 --- a/compiler/optimizing/inliner.cc +++ b/compiler/optimizing/inliner.cc @@ -1303,11 +1303,12 @@ void HInliner::FixUpReturnReferenceType(HInvoke* invoke_instruction, DCHECK(return_replacement->IsPhi()); size_t pointer_size = Runtime::Current()->GetClassLinker()->GetImagePointerSize(); mirror::Class* cls = resolved_method->GetReturnType(false /* resolve */, pointer_size); - if (cls != nullptr) { + if (cls != nullptr && !cls->IsErroneous()) { ReferenceTypeInfo::TypeHandle return_handle = handles_->NewHandle(cls); return_replacement->SetReferenceTypeInfo(ReferenceTypeInfo::Create( return_handle, return_handle->CannotBeAssignedFromOtherTypes() /* is_exact */)); } else { + // Return inexact object type on failures. return_replacement->SetReferenceTypeInfo(graph_->GetInexactObjectRti()); } } diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc index 1afa36a89..af5ef04bf 100644 --- a/compiler/optimizing/nodes.cc +++ b/compiler/optimizing/nodes.cc @@ -2190,6 +2190,7 @@ ReferenceTypeInfo ReferenceTypeInfo::Create(TypeHandle type_handle, bool is_exac if (kIsDebugBuild) { ScopedObjectAccess soa(Thread::Current()); DCHECK(IsValidHandle(type_handle)); + DCHECK(!type_handle->IsErroneous()); if (!is_exact) { DCHECK(!type_handle->CannotBeAssignedFromOtherTypes()) << "Callers of ReferenceTypeInfo::Create should ensure is_exact is properly computed"; diff --git a/compiler/optimizing/reference_type_propagation.cc b/compiler/optimizing/reference_type_propagation.cc index 95f10e072..0f93a4bc3 100644 --- a/compiler/optimizing/reference_type_propagation.cc +++ b/compiler/optimizing/reference_type_propagation.cc @@ -431,9 +431,14 @@ void ReferenceTypePropagation::RTPVisitor::SetClassAsTypeInfo(HInstruction* inst ReferenceTypeInfo::Create(handle_cache_->GetStringClassHandle(), /* is_exact */ true)); } else if (klass != nullptr) { ScopedObjectAccess soa(Thread::Current()); - ReferenceTypeInfo::TypeHandle handle = handle_cache_->NewHandle(klass); - is_exact = is_exact || handle->CannotBeAssignedFromOtherTypes(); - instr->SetReferenceTypeInfo(ReferenceTypeInfo::Create(handle, is_exact)); + if (klass->IsErroneous()) { + // Set inexact object type for erroneous types. + instr->SetReferenceTypeInfo(instr->GetBlock()->GetGraph()->GetInexactObjectRti()); + } else { + ReferenceTypeInfo::TypeHandle handle = handle_cache_->NewHandle(klass); + is_exact = is_exact || handle->CannotBeAssignedFromOtherTypes(); + instr->SetReferenceTypeInfo(ReferenceTypeInfo::Create(handle, is_exact)); + } } else { instr->SetReferenceTypeInfo(instr->GetBlock()->GetGraph()->GetInexactObjectRti()); } @@ -534,7 +539,7 @@ void ReferenceTypePropagation::RTPVisitor::VisitLoadClass(HLoadClass* instr) { // Get type from dex cache assuming it was populated by the verifier. mirror::Class* resolved_class = GetClassFromDexCache(soa.Self(), instr->GetDexFile(), instr->GetTypeIndex()); - if (resolved_class != nullptr) { + if (resolved_class != nullptr && !resolved_class->IsErroneous()) { instr->SetLoadedClassRTI(ReferenceTypeInfo::Create( handle_cache_->NewHandle(resolved_class), /* is_exact */ true)); } @@ -718,7 +723,7 @@ void ReferenceTypePropagation::UpdateArrayGet(HArrayGet* instr, HandleCache* han } Handle handle = parent_rti.GetTypeHandle(); - if (handle->IsObjectArrayClass()) { + if (handle->IsObjectArrayClass() && !handle->GetComponentType()->IsErroneous()) { ReferenceTypeInfo::TypeHandle component_handle = handle_cache->NewHandle(handle->GetComponentType()); bool is_exact = component_handle->CannotBeAssignedFromOtherTypes(); diff --git a/compiler/optimizing/reference_type_propagation.h b/compiler/optimizing/reference_type_propagation.h index 028a6fc51..01b578183 100644 --- a/compiler/optimizing/reference_type_propagation.h +++ b/compiler/optimizing/reference_type_propagation.h @@ -99,6 +99,8 @@ class ReferenceTypePropagation : public HOptimization { static constexpr size_t kDefaultWorklistSize = 8; + friend class ReferenceTypePropagationTest; + DISALLOW_COPY_AND_ASSIGN(ReferenceTypePropagation); }; diff --git a/compiler/optimizing/reference_type_propagation_test.cc b/compiler/optimizing/reference_type_propagation_test.cc new file mode 100644 index 000000000..99d5f822c --- /dev/null +++ b/compiler/optimizing/reference_type_propagation_test.cc @@ -0,0 +1,190 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "base/arena_allocator.h" +#include "builder.h" +#include "nodes.h" +#include "object_lock.h" +#include "optimizing_unit_test.h" +#include "reference_type_propagation.h" + +namespace art { + +/** + * Fixture class for unit testing the ReferenceTypePropagation phase. Used to verify the + * functionality of methods and situations that are hard to set up with checker tests. + */ +class ReferenceTypePropagationTest : public CommonCompilerTest { + public: + ReferenceTypePropagationTest() : pool_(), allocator_(&pool_) { + graph_ = CreateGraph(&allocator_); + } + + ~ReferenceTypePropagationTest() { } + + void SetupPropagation(StackHandleScopeCollection* handles) { + graph_->InitializeInexactObjectRTI(handles); + propagation_ = new (&allocator_) ReferenceTypePropagation(graph_, handles, true, "test_prop"); + } + + // Relay method to merge type in reference type propagation. + ReferenceTypeInfo MergeTypes(const ReferenceTypeInfo& a, + const ReferenceTypeInfo& b) SHARED_REQUIRES(Locks::mutator_lock_) { + return propagation_->MergeTypes(a, b); + } + + // Helper method to construct an invalid type. + ReferenceTypeInfo InvalidType() { + return ReferenceTypeInfo::CreateInvalid(); + } + + // Helper method to construct the Object type. + ReferenceTypeInfo ObjectType(bool is_exact = true) SHARED_REQUIRES(Locks::mutator_lock_) { + return ReferenceTypeInfo::Create(propagation_->handle_cache_.GetObjectClassHandle(), is_exact); + } + + // Helper method to construct the String type. + ReferenceTypeInfo StringType(bool is_exact = true) SHARED_REQUIRES(Locks::mutator_lock_) { + return ReferenceTypeInfo::Create(propagation_->handle_cache_.GetStringClassHandle(), is_exact); + } + + // Helper method to construct the Throwable type. + ReferenceTypeInfo ThrowableType(bool is_exact = true) SHARED_REQUIRES(Locks::mutator_lock_) { + return ReferenceTypeInfo::Create(propagation_->handle_cache_.GetThrowableClassHandle(), is_exact); + } + + // Helper method to provide access to Throwable handle. + Handle GetThrowableHandle() { + return propagation_->handle_cache_.GetThrowableClassHandle(); + } + + // General building fields. + ArenaPool pool_; + ArenaAllocator allocator_; + HGraph* graph_; + + ReferenceTypePropagation* propagation_; +}; + +// +// The actual ReferenceTypePropgation unit tests. +// + +TEST_F(ReferenceTypePropagationTest, ProperSetup) { + ScopedObjectAccess soa(Thread::Current()); + StackHandleScopeCollection handles(soa.Self()); + SetupPropagation(&handles); + + EXPECT_TRUE(propagation_ != nullptr); + EXPECT_TRUE(graph_->GetInexactObjectRti().IsEqual(ObjectType(false))); +} + +TEST_F(ReferenceTypePropagationTest, MergeInvalidTypes) { + ScopedObjectAccess soa(Thread::Current()); + StackHandleScopeCollection handles(soa.Self()); + SetupPropagation(&handles); + + // Two invalid types. + ReferenceTypeInfo t1(MergeTypes(InvalidType(), InvalidType())); + EXPECT_FALSE(t1.IsValid()); + EXPECT_FALSE(t1.IsExact()); + EXPECT_TRUE(t1.IsEqual(InvalidType())); + + // Valid type on right. + ReferenceTypeInfo t2(MergeTypes(InvalidType(), ObjectType())); + EXPECT_TRUE(t2.IsValid()); + EXPECT_TRUE(t2.IsExact()); + EXPECT_TRUE(t2.IsEqual(ObjectType())); + ReferenceTypeInfo t3(MergeTypes(InvalidType(), StringType())); + EXPECT_TRUE(t3.IsValid()); + EXPECT_TRUE(t3.IsExact()); + EXPECT_TRUE(t3.IsEqual(StringType())); + + // Valid type on left. + ReferenceTypeInfo t4(MergeTypes(ObjectType(), InvalidType())); + EXPECT_TRUE(t4.IsValid()); + EXPECT_TRUE(t4.IsExact()); + EXPECT_TRUE(t4.IsEqual(ObjectType())); + ReferenceTypeInfo t5(MergeTypes(StringType(), InvalidType())); + EXPECT_TRUE(t5.IsValid()); + EXPECT_TRUE(t5.IsExact()); + EXPECT_TRUE(t5.IsEqual(StringType())); +} + +TEST_F(ReferenceTypePropagationTest, CreateErroneousType) { + ScopedObjectAccess soa(Thread::Current()); + StackHandleScopeCollection handles(soa.Self()); + SetupPropagation(&handles); + + // Some trickery to make the runtime think something went wrong with loading + // the throwable class, making this an erroneous type from here on. + soa.Self()->SetException(Thread::GetDeoptimizationException()); + Handle klass = GetThrowableHandle(); + ObjectLock lock(soa.Self(), klass); + mirror::Class::SetStatus(klass, mirror::Class::kStatusError, soa.Self()); + soa.Self()->ClearException(); + ASSERT_TRUE(klass->IsErroneous()); + + // Trying to get an erroneous type crashes (debug mode only). + if (kIsDebugBuild) { + EXPECT_DEATH(ThrowableType(), "Check failed: !type_handle->IsErroneous()"); + } +} + +TEST_F(ReferenceTypePropagationTest, MergeValidTypes) { + ScopedObjectAccess soa(Thread::Current()); + StackHandleScopeCollection handles(soa.Self()); + SetupPropagation(&handles); + + // Same types. + ReferenceTypeInfo t1(MergeTypes(ObjectType(), ObjectType())); + EXPECT_TRUE(t1.IsValid()); + EXPECT_TRUE(t1.IsExact()); + EXPECT_TRUE(t1.IsEqual(ObjectType())); + ReferenceTypeInfo t2(MergeTypes(StringType(), StringType())); + EXPECT_TRUE(t2.IsValid()); + EXPECT_TRUE(t2.IsExact()); + EXPECT_TRUE(t2.IsEqual(StringType())); + + // Left is super class of right. + ReferenceTypeInfo t3(MergeTypes(ObjectType(), StringType())); + EXPECT_TRUE(t3.IsValid()); + EXPECT_FALSE(t3.IsExact()); + EXPECT_TRUE(t3.IsEqual(ObjectType(false))); + + // Right is super class of left. + ReferenceTypeInfo t4(MergeTypes(StringType(), ObjectType())); + EXPECT_TRUE(t4.IsValid()); + EXPECT_FALSE(t4.IsExact()); + EXPECT_TRUE(t4.IsEqual(ObjectType(false))); + + // Same types, but one or both are inexact. + ReferenceTypeInfo t5(MergeTypes(ObjectType(false), ObjectType())); + EXPECT_TRUE(t5.IsValid()); + EXPECT_FALSE(t5.IsExact()); + EXPECT_TRUE(t5.IsEqual(ObjectType(false))); + ReferenceTypeInfo t6(MergeTypes(ObjectType(), ObjectType(false))); + EXPECT_TRUE(t6.IsValid()); + EXPECT_FALSE(t6.IsExact()); + EXPECT_TRUE(t6.IsEqual(ObjectType(false))); + ReferenceTypeInfo t7(MergeTypes(ObjectType(false), ObjectType(false))); + EXPECT_TRUE(t7.IsValid()); + EXPECT_FALSE(t7.IsExact()); + EXPECT_TRUE(t7.IsEqual(ObjectType(false))); +} + +} // namespace art + -- 2.11.0