From 4824c27988c8eeb302791624bb3ce1d557b0db6c Mon Sep 17 00:00:00 2001 From: Nicolas Geoffray Date: Wed, 24 Jun 2015 15:53:03 +0100 Subject: [PATCH] Use a flag from the verifier to know if we should compile. Only used for the lack of bottom type in the aget-object case for now. Could be used for more. bug:21865466 Change-Id: I64c2c84dfa1c0d259631e65e5f44b94e4139e6a7 --- compiler/dex/quick/quick_compiler.cc | 5 ++++ compiler/dex/verified_method.cc | 1 + compiler/dex/verified_method.h | 5 ++++ compiler/optimizing/optimizing_compiler.cc | 3 ++- runtime/verifier/method_verifier.cc | 1 + runtime/verifier/method_verifier.h | 4 +++ test/518-null-array-get/expected.txt | 0 test/518-null-array-get/info.txt | 3 +++ test/518-null-array-get/smali/NullArray.smali | 26 +++++++++++++++++++ test/518-null-array-get/src/Main.java | 37 +++++++++++++++++++++++++++ 10 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 test/518-null-array-get/expected.txt create mode 100644 test/518-null-array-get/info.txt create mode 100644 test/518-null-array-get/smali/NullArray.smali create mode 100644 test/518-null-array-get/src/Main.java diff --git a/compiler/dex/quick/quick_compiler.cc b/compiler/dex/quick/quick_compiler.cc index 2523a83c3..28c485a41 100644 --- a/compiler/dex/quick/quick_compiler.cc +++ b/compiler/dex/quick/quick_compiler.cc @@ -33,6 +33,7 @@ #include "dex/pass_driver_me_post_opt.h" #include "dex/pass_manager.h" #include "dex/quick/mir_to_lir.h" +#include "dex/verified_method.h" #include "driver/compiler_driver.h" #include "driver/compiler_options.h" #include "elf_writer_quick.h" @@ -661,6 +662,10 @@ CompiledMethod* QuickCompiler::Compile(const DexFile::CodeItem* code_item, return nullptr; } + if (driver->GetVerifiedMethod(&dex_file, method_idx)->HasRuntimeThrow()) { + return nullptr; + } + DCHECK(driver->GetCompilerOptions().IsCompilationEnabled()); Runtime* const runtime = Runtime::Current(); diff --git a/compiler/dex/verified_method.cc b/compiler/dex/verified_method.cc index 8a009cb3e..273b16283 100644 --- a/compiler/dex/verified_method.cc +++ b/compiler/dex/verified_method.cc @@ -41,6 +41,7 @@ const VerifiedMethod* VerifiedMethod::Create(verifier::MethodVerifier* method_ve bool compile) { std::unique_ptr verified_method(new VerifiedMethod); verified_method->has_verification_failures_ = method_verifier->HasFailures(); + verified_method->has_runtime_throw_ = method_verifier->HasInstructionThatWillThrow(); if (compile) { /* Generate a register map. */ if (!verified_method->GenerateGcMap(method_verifier)) { diff --git a/compiler/dex/verified_method.h b/compiler/dex/verified_method.h index 07f9a9bd9..bf11839cf 100644 --- a/compiler/dex/verified_method.h +++ b/compiler/dex/verified_method.h @@ -75,6 +75,10 @@ class VerifiedMethod { return has_verification_failures_; } + bool HasRuntimeThrow() const { + return has_runtime_throw_; + } + void SetStringInitPcRegMap(SafeMap>& string_init_pc_reg_map) { string_init_pc_reg_map_ = string_init_pc_reg_map; } @@ -121,6 +125,7 @@ class VerifiedMethod { SafeCastSet safe_cast_set_; bool has_verification_failures_ = false; + bool has_runtime_throw_ = false; // Copy of mapping generated by verifier of dex PCs of string init invocations // to the set of other registers that the receiver has been copied into. diff --git a/compiler/optimizing/optimizing_compiler.cc b/compiler/optimizing/optimizing_compiler.cc index 146636675..1944ba663 100644 --- a/compiler/optimizing/optimizing_compiler.cc +++ b/compiler/optimizing/optimizing_compiler.cc @@ -677,7 +677,8 @@ CompiledMethod* OptimizingCompiler::Compile(const DexFile::CodeItem* code_item, const DexFile& dex_file) const { CompilerDriver* compiler_driver = GetCompilerDriver(); CompiledMethod* method = nullptr; - if (compiler_driver->IsMethodVerifiedWithoutFailures(method_idx, class_def_idx, dex_file)) { + if (compiler_driver->IsMethodVerifiedWithoutFailures(method_idx, class_def_idx, dex_file) && + !compiler_driver->GetVerifiedMethod(&dex_file, method_idx)->HasRuntimeThrow()) { method = TryCompile(code_item, access_flags, invoke_type, class_def_idx, method_idx, jclass_loader, dex_file); } else { diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc index 5d685da54..5ba0f4b5e 100644 --- a/runtime/verifier/method_verifier.cc +++ b/runtime/verifier/method_verifier.cc @@ -3778,6 +3778,7 @@ void MethodVerifier::VerifyAGet(const Instruction* inst, } else { const RegType& array_type = work_line_->GetRegisterType(this, inst->VRegB_23x()); if (array_type.IsZero()) { + have_pending_runtime_throw_failure_ = true; // Null array class; this code path will fail at runtime. Infer a merge-able type from the // instruction type. TODO: have a proper notion of bottom here. if (!is_primitive || insn_type.IsCategory1Types()) { diff --git a/runtime/verifier/method_verifier.h b/runtime/verifier/method_verifier.h index 994616f6c..d7ddd6709 100644 --- a/runtime/verifier/method_verifier.h +++ b/runtime/verifier/method_verifier.h @@ -254,6 +254,10 @@ class MethodVerifier { bool HasCheckCasts() const; bool HasVirtualOrInterfaceInvokes() const; bool HasFailures() const; + bool HasInstructionThatWillThrow() const { + return have_pending_runtime_throw_failure_; + } + const RegType& ResolveCheckedClass(uint32_t class_idx) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); // Returns the method of a quick invoke or null if it cannot be found. diff --git a/test/518-null-array-get/expected.txt b/test/518-null-array-get/expected.txt new file mode 100644 index 000000000..e69de29bb diff --git a/test/518-null-array-get/info.txt b/test/518-null-array-get/info.txt new file mode 100644 index 000000000..407f590b2 --- /dev/null +++ b/test/518-null-array-get/info.txt @@ -0,0 +1,3 @@ +Regression test for Quick and Optimizing that used +to crash on an aget-object + int-to-byte sequence +(accepted by the verifier in the case the array was null). diff --git a/test/518-null-array-get/smali/NullArray.smali b/test/518-null-array-get/smali/NullArray.smali new file mode 100644 index 000000000..52abc3847 --- /dev/null +++ b/test/518-null-array-get/smali/NullArray.smali @@ -0,0 +1,26 @@ +# 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. + +.class public LNullArray; + +.super Ljava/lang/Object; + +.method public static method()B + .registers 2 + const/4 v0, 0 + const/4 v1, 0 + aget-object v0, v0, v1 + int-to-byte v0, v0 + return v0 +.end method diff --git a/test/518-null-array-get/src/Main.java b/test/518-null-array-get/src/Main.java new file mode 100644 index 000000000..66e50aacd --- /dev/null +++ b/test/518-null-array-get/src/Main.java @@ -0,0 +1,37 @@ +/* + * 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.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; + +public class Main { + // Workaround for b/18051191. + class InnerClass {} + + public static void main(String[] args) throws Exception { + Class c = Class.forName("NullArray"); + Method m = c.getMethod("method"); + Object[] arguments = { }; + try { + m.invoke(null, arguments); + throw new Error("Expected an InvocationTargetException"); + } catch (InvocationTargetException e) { + if (!(e.getCause() instanceof NullPointerException)) { + throw new Error("Expected a NullPointerException"); + } + } + } +} -- 2.11.0