diff options
25 files changed, 666 insertions, 134 deletions
diff --git a/cmdline/cmdline_parser_test.cc b/cmdline/cmdline_parser_test.cc index 1536339515..c438c54cea 100644 --- a/cmdline/cmdline_parser_test.cc +++ b/cmdline/cmdline_parser_test.cc @@ -244,7 +244,7 @@ TEST_F(CmdlineParserTest, TestLogVerbosity) { { const char* log_args = "-verbose:" "class,compiler,gc,heap,jdwp,jni,monitor,profiler,signals,simulator,startup," - "third-party-jni,threads,verifier"; + "third-party-jni,threads,verifier,verifier-debug"; LogVerbosity log_verbosity = LogVerbosity(); log_verbosity.class_linker = true; @@ -261,6 +261,7 @@ TEST_F(CmdlineParserTest, TestLogVerbosity) { log_verbosity.third_party_jni = true; log_verbosity.threads = true; log_verbosity.verifier = true; + log_verbosity.verifier_debug = true; EXPECT_SINGLE_PARSE_VALUE(log_verbosity, log_args, M::Verbose); } diff --git a/cmdline/cmdline_types.h b/cmdline/cmdline_types.h index 37bdcdc5e2..f12ef971af 100644 --- a/cmdline/cmdline_types.h +++ b/cmdline/cmdline_types.h @@ -669,6 +669,8 @@ struct CmdlineType<LogVerbosity> : CmdlineTypeParser<LogVerbosity> { log_verbosity.threads = true; } else if (verbose_options[j] == "verifier") { log_verbosity.verifier = true; + } else if (verbose_options[j] == "verifier-debug") { + log_verbosity.verifier_debug = true; } else if (verbose_options[j] == "image") { log_verbosity.image = true; } else if (verbose_options[j] == "systrace-locks") { diff --git a/compiler/optimizing/instruction_simplifier.cc b/compiler/optimizing/instruction_simplifier.cc index a65d7b1984..a42a85dc1d 100644 --- a/compiler/optimizing/instruction_simplifier.cc +++ b/compiler/optimizing/instruction_simplifier.cc @@ -1081,6 +1081,58 @@ static inline bool TryReplaceFieldOrArrayGetType(HInstruction* maybe_get, DataTy } } +// The type conversion is only used for storing into a field/element of the +// same/narrower size. +static bool IsTypeConversionForStoringIntoNoWiderFieldOnly(HTypeConversion* type_conversion) { + if (type_conversion->HasEnvironmentUses()) { + return false; + } + DataType::Type input_type = type_conversion->GetInputType(); + DataType::Type result_type = type_conversion->GetResultType(); + if (!DataType::IsIntegralType(input_type) || + !DataType::IsIntegralType(result_type) || + input_type == DataType::Type::kInt64 || + result_type == DataType::Type::kInt64) { + // Type conversion is needed if non-integer types are involved, or 64-bit + // types are involved, which may use different number of registers. + return false; + } + if (DataType::Size(input_type) >= DataType::Size(result_type)) { + // Type conversion is not necessary when storing to a field/element of the + // same/smaller size. + } else { + // We do not handle this case here. + return false; + } + + // Check if the converted value is only used for storing into heap. + for (const HUseListNode<HInstruction*>& use : type_conversion->GetUses()) { + HInstruction* instruction = use.GetUser(); + if (instruction->IsInstanceFieldSet() && + instruction->AsInstanceFieldSet()->GetFieldType() == result_type) { + DCHECK_EQ(instruction->AsInstanceFieldSet()->GetValue(), type_conversion); + continue; + } + if (instruction->IsStaticFieldSet() && + instruction->AsStaticFieldSet()->GetFieldType() == result_type) { + DCHECK_EQ(instruction->AsStaticFieldSet()->GetValue(), type_conversion); + continue; + } + if (instruction->IsArraySet() && + instruction->AsArraySet()->GetComponentType() == result_type && + // not index use. + instruction->AsArraySet()->GetIndex() != type_conversion) { + DCHECK_EQ(instruction->AsArraySet()->GetValue(), type_conversion); + continue; + } + // The use is not as a store value, or the field/element type is not the + // same as the result_type, keep the type conversion. + return false; + } + // Codegen automatically handles the type conversion during the store. + return true; +} + void InstructionSimplifierVisitor::VisitTypeConversion(HTypeConversion* instruction) { HInstruction* input = instruction->GetInput(); DataType::Type input_type = input->GetType(); @@ -1169,6 +1221,13 @@ void InstructionSimplifierVisitor::VisitTypeConversion(HTypeConversion* instruct return; } } + + if (IsTypeConversionForStoringIntoNoWiderFieldOnly(instruction)) { + instruction->ReplaceWith(input); + instruction->GetBlock()->RemoveInstruction(instruction); + RecordSimplification(); + return; + } } void InstructionSimplifierVisitor::VisitAdd(HAdd* instruction) { diff --git a/compiler/optimizing/load_store_elimination.cc b/compiler/optimizing/load_store_elimination.cc index f03fffabe2..88326d321b 100644 --- a/compiler/optimizing/load_store_elimination.cc +++ b/compiler/optimizing/load_store_elimination.cc @@ -74,6 +74,20 @@ class LSEVisitor : public HGraphDelegateVisitor { HGraphVisitor::VisitBasicBlock(block); } + HTypeConversion* AddTypeConversionIfNecessary(HInstruction* instruction, + HInstruction* value, + DataType::Type expected_type) { + HTypeConversion* type_conversion = nullptr; + // Should never add type conversion into boolean value. + if (expected_type != DataType::Type::kBool && + !DataType::IsTypeConversionImplicit(value->GetType(), expected_type)) { + type_conversion = new (GetGraph()->GetAllocator()) HTypeConversion( + expected_type, value, instruction->GetDexPc()); + instruction->GetBlock()->InsertInstructionBefore(type_conversion, instruction); + } + return type_conversion; + } + // Find an instruction's substitute if it should be removed. // Return the same instruction if it should not be removed. HInstruction* FindSubstitute(HInstruction* instruction) { @@ -86,13 +100,59 @@ class LSEVisitor : public HGraphDelegateVisitor { return instruction; } + void AddRemovedLoad(HInstruction* load, HInstruction* heap_value) { + DCHECK_EQ(FindSubstitute(heap_value), heap_value) << + "Unexpected heap_value that has a substitute " << heap_value->DebugName(); + removed_loads_.push_back(load); + substitute_instructions_for_loads_.push_back(heap_value); + } + + // Scan the list of removed loads to see if we can reuse `type_conversion`, if + // the other removed load has the same substitute and type and is dominated + // by `type_conversioni`. + void TryToReuseTypeConversion(HInstruction* type_conversion, size_t index) { + size_t size = removed_loads_.size(); + HInstruction* load = removed_loads_[index]; + HInstruction* substitute = substitute_instructions_for_loads_[index]; + for (size_t j = index + 1; j < size; j++) { + HInstruction* load2 = removed_loads_[j]; + HInstruction* substitute2 = substitute_instructions_for_loads_[j]; + if (load2 == nullptr) { + DCHECK(substitute2->IsTypeConversion()); + continue; + } + DCHECK(load2->IsInstanceFieldGet() || + load2->IsStaticFieldGet() || + load2->IsArrayGet()); + DCHECK(substitute2 != nullptr); + if (substitute2 == substitute && + load2->GetType() == load->GetType() && + type_conversion->GetBlock()->Dominates(load2->GetBlock()) && + // Don't share across irreducible loop headers. + // TODO: can be more fine-grained than this by testing each dominator. + (load2->GetBlock() == type_conversion->GetBlock() || + !GetGraph()->HasIrreducibleLoops())) { + // The removed_loads_ are added in reverse post order. + DCHECK(type_conversion->StrictlyDominates(load2)); + load2->ReplaceWith(type_conversion); + load2->GetBlock()->RemoveInstruction(load2); + removed_loads_[j] = nullptr; + substitute_instructions_for_loads_[j] = type_conversion; + } + } + } + // Remove recorded instructions that should be eliminated. void RemoveInstructions() { size_t size = removed_loads_.size(); DCHECK_EQ(size, substitute_instructions_for_loads_.size()); for (size_t i = 0; i < size; i++) { HInstruction* load = removed_loads_[i]; - DCHECK(load != nullptr); + if (load == nullptr) { + // The load has been handled in the scan for type conversion below. + DCHECK(substitute_instructions_for_loads_[i]->IsTypeConversion()); + continue; + } DCHECK(load->IsInstanceFieldGet() || load->IsStaticFieldGet() || load->IsArrayGet()); @@ -102,7 +162,28 @@ class LSEVisitor : public HGraphDelegateVisitor { // a load that has a substitute should not be observed as a heap // location value. DCHECK_EQ(FindSubstitute(substitute), substitute); - load->ReplaceWith(substitute); + + // The load expects to load the heap value as type load->GetType(). + // However the tracked heap value may not be of that type. An explicit + // type conversion may be needed. + // There are actually three types involved here: + // (1) tracked heap value's type (type A) + // (2) heap location (field or element)'s type (type B) + // (3) load's type (type C) + // We guarantee that type A stored as type B and then fetched out as + // type C is the same as casting from type A to type C directly, since + // type B and type C will have the same size which is guarenteed in + // HInstanceFieldGet/HStaticFieldGet/HArrayGet's SetType(). + // So we only need one type conversion from type A to type C. + HTypeConversion* type_conversion = AddTypeConversionIfNecessary( + load, substitute, load->GetType()); + if (type_conversion != nullptr) { + TryToReuseTypeConversion(type_conversion, i); + load->ReplaceWith(type_conversion); + substitute_instructions_for_loads_[i] = type_conversion; + } else { + load->ReplaceWith(substitute); + } load->GetBlock()->RemoveInstruction(load); } @@ -338,8 +419,7 @@ class LSEVisitor : public HGraphDelegateVisitor { HInstruction* heap_value = heap_values[idx]; if (heap_value == kDefaultHeapValue) { HInstruction* constant = GetDefaultValue(instruction->GetType()); - removed_loads_.push_back(instruction); - substitute_instructions_for_loads_.push_back(constant); + AddRemovedLoad(instruction, constant); heap_values[idx] = constant; return; } @@ -374,8 +454,7 @@ class LSEVisitor : public HGraphDelegateVisitor { } return; } - removed_loads_.push_back(instruction); - substitute_instructions_for_loads_.push_back(heap_value); + AddRemovedLoad(instruction, heap_value); TryRemovingNullCheck(instruction); } } diff --git a/openjdkjvmti/OpenjdkJvmTi.cc b/openjdkjvmti/OpenjdkJvmTi.cc index 62f723dec1..6b2d5d6a5c 100644 --- a/openjdkjvmti/OpenjdkJvmTi.cc +++ b/openjdkjvmti/OpenjdkJvmTi.cc @@ -1437,6 +1437,7 @@ class JvmtiFunctions { art::gLogVerbosity.third_party_jni = val; art::gLogVerbosity.threads = val; art::gLogVerbosity.verifier = val; + // Do not set verifier-debug. art::gLogVerbosity.image = val; // Note: can't switch systrace_lock_logging. That requires changing entrypoints. diff --git a/runtime/base/logging.h b/runtime/base/logging.h index 15f935395e..5703b3c746 100644 --- a/runtime/base/logging.h +++ b/runtime/base/logging.h @@ -53,6 +53,7 @@ struct LogVerbosity { bool third_party_jni; // Enabled with "-verbose:third-party-jni". bool threads; bool verifier; + bool verifier_debug; // Only works in debug builds. bool image; bool systrace_lock_logging; // Enabled with "-verbose:sys-locks". bool agents; diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc index be58a57c24..f1af2529bd 100644 --- a/runtime/verifier/method_verifier.cc +++ b/runtime/verifier/method_verifier.cc @@ -61,8 +61,6 @@ namespace verifier { using android::base::StringPrintf; static constexpr bool kTimeVerifyMethod = !kIsDebugBuild; -static constexpr bool kDebugVerify = false; -// TODO: Add a constant to method_verifier to turn on verbose logging? // On VLOG(verifier), should we dump the whole state when we run into a hard failure? static constexpr bool kDumpRegLinesOnHardFailureIfVLOG = true; @@ -408,6 +406,10 @@ MethodVerifier::FailureData MethodVerifier::VerifyMethod(Thread* self, verifier.DumpFailures(VLOG_STREAM(verifier) << "Soft verification failures in " << dex_file->PrettyMethod(method_idx) << "\n"); } + if (VLOG_IS_ON(verifier_debug)) { + std::cout << "\n" << verifier.info_messages_.str(); + verifier.Dump(std::cout); + } result.kind = FailureKind::kSoftFailure; if (method != nullptr && !CanCompilerHandleVerificationFailure(verifier.encountered_failure_types_)) { @@ -481,7 +483,7 @@ MethodVerifier::FailureData MethodVerifier::VerifyMethod(Thread* self, callbacks->ClassRejected(ref); } } - if (VLOG_IS_ON(verifier)) { + if (VLOG_IS_ON(verifier) || VLOG_IS_ON(verifier_debug)) { std::cout << "\n" << verifier.info_messages_.str(); verifier.Dump(std::cout); } @@ -1935,7 +1937,7 @@ bool MethodVerifier::CodeFlowVerifyMethod() { GetInstructionFlags(insn_idx).ClearChanged(); } - if (kDebugVerify) { + if (UNLIKELY(VLOG_IS_ON(verifier_debug))) { /* * Scan for dead code. There's nothing "evil" about dead code * (besides the wasted space), but it indicates a flaw somewhere @@ -2079,7 +2081,7 @@ bool MethodVerifier::CodeFlowVerifyInstruction(uint32_t* start_guess) { int32_t branch_target = 0; bool just_set_result = false; - if (kDebugVerify) { + if (UNLIKELY(VLOG_IS_ON(verifier_debug))) { // Generate processing back trace to debug verifier LogVerifyInfo() << "Processing " << inst->DumpString(dex_file_) << "\n" << work_line_->Dump(this) << "\n"; @@ -4691,12 +4693,19 @@ void MethodVerifier::VerifyAGet(const Instruction* inst, } else { const RegType& array_type = work_line_->GetRegisterType(this, inst->VRegB_23x()); if (array_type.IsZeroOrNull()) { - 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()) { - // Reference or category 1 - work_line_->SetRegisterType<LockOp::kClear>(this, inst->VRegA_23x(), reg_types_.Zero()); + // instruction type. + if (!is_primitive) { + work_line_->SetRegisterType<LockOp::kClear>(this, inst->VRegA_23x(), reg_types_.Null()); + } else if (insn_type.IsInteger()) { + // Pick a non-zero constant (to distinguish with null) that can fit in any primitive. + // We cannot use 'insn_type' as it could be a float array or an int array. + work_line_->SetRegisterType<LockOp::kClear>( + this, inst->VRegA_23x(), DetermineCat1Constant(1, need_precise_constants_)); + } else if (insn_type.IsCategory1Types()) { + // Category 1 + // The 'insn_type' is exactly the type we need. + work_line_->SetRegisterType<LockOp::kClear>(this, inst->VRegA_23x(), insn_type); } else { // Category 2 work_line_->SetRegisterTypeWide(this, inst->VRegA_23x(), @@ -5343,7 +5352,7 @@ bool MethodVerifier::UpdateRegisters(uint32_t next_insn, RegisterLine* merge_lin } } else { RegisterLineArenaUniquePtr copy; - if (kDebugVerify) { + if (UNLIKELY(VLOG_IS_ON(verifier_debug))) { copy.reset(RegisterLine::Create(target_line->NumRegs(), this)); copy->CopyFromLine(target_line); } @@ -5351,7 +5360,7 @@ bool MethodVerifier::UpdateRegisters(uint32_t next_insn, RegisterLine* merge_lin if (have_pending_hard_failure_) { return false; } - if (kDebugVerify && changed) { + if (UNLIKELY(VLOG_IS_ON(verifier_debug)) && changed) { LogVerifyInfo() << "Merging at [" << reinterpret_cast<void*>(work_insn_idx_) << "]" << " to [" << reinterpret_cast<void*>(next_insn) << "]: " << "\n" << copy->Dump(this) << " MERGE\n" diff --git a/test/518-null-array-get/expected.txt b/test/518-null-array-get/expected.txt index e69de29bb2..ae5318e53d 100644 --- a/test/518-null-array-get/expected.txt +++ b/test/518-null-array-get/expected.txt @@ -0,0 +1,6 @@ +NullArrayFailInt2Object +NullArrayFailObject2Int +NullArraySuccessInt +NullArraySuccessInt2Float +NullArraySuccessShort +NullArraySuccessRef diff --git a/test/518-null-array-get/info.txt b/test/518-null-array-get/info.txt index 407f590b2b..71e0332e62 100644 --- a/test/518-null-array-get/info.txt +++ b/test/518-null-array-get/info.txt @@ -1,3 +1,9 @@ -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). +Codifies that the verifier should reject type-unsafe +instructions in dead code after aget on null, but pass +type-safe dead code. + +Previously verification stopped after aget on null and +punted the method to the interpreter in an effort to avoid +compiler crashes. As broken code appears very uncommon, +ensure verifier strictness and help the compilers see more +code. diff --git a/test/706-jit-skip-compilation/run b/test/518-null-array-get/smali/NullArrayFailInt2Object.smali index 6c5720a099..ca4ed10660 100644 --- a/test/706-jit-skip-compilation/run +++ b/test/518-null-array-get/smali/NullArrayFailInt2Object.smali @@ -1,12 +1,10 @@ -#!/bin/bash -# -# Copyright (C) 2016 The Android Open Source Project +# 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 +# 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, @@ -14,6 +12,17 @@ # See the License for the specific language governing permissions and # limitations under the License. -# Run without the app image, otherwise the verification results will be cached -# in the ArtMethod of the image and the test will be skewed. -exec ${RUN} "${@}" --no-app-image +# Check that the result of aget on null cannot be used as a reference. + +.class public LNullArrayFailInt2Object; + +.super Ljava/lang/Object; + +.method public static method()V + .registers 2 + const/4 v0, 0 + const/4 v1, 0 + aget v0, v0, v1 + invoke-virtual { v0 }, Ljava/lang/Object;->toString()Ljava/lang/String; + return-void +.end method diff --git a/test/518-null-array-get/smali/NullArray.smali b/test/518-null-array-get/smali/NullArrayFailObject2Int.smali index 52abc38473..83823a24e5 100644 --- a/test/518-null-array-get/smali/NullArray.smali +++ b/test/518-null-array-get/smali/NullArrayFailObject2Int.smali @@ -12,7 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -.class public LNullArray; +# Check that the result of aget-object on null cannot be used as an integral. + +.class public LNullArrayFailObject2Int; .super Ljava/lang/Object; diff --git a/test/518-null-array-get/smali/NullArraySuccessInt.smali b/test/518-null-array-get/smali/NullArraySuccessInt.smali new file mode 100644 index 0000000000..01cf1c92ab --- /dev/null +++ b/test/518-null-array-get/smali/NullArraySuccessInt.smali @@ -0,0 +1,33 @@ +# 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. + +# Check that the result of aget on null can be used as an int. + +.class public LNullArraySuccessInt; + +.super Ljava/lang/Object; + +.method public static intMethod(I)V + .registers 1 + return-void +.end method + +.method public static method()V + .registers 2 + const/4 v0, 0 + const/4 v1, 0 + aget v0, v0, v1 + invoke-static { v0 }, LNullArraySuccessInt;->intMethod(I)V + return-void +.end method diff --git a/test/518-null-array-get/smali/NullArraySuccessInt2Float.smali b/test/518-null-array-get/smali/NullArraySuccessInt2Float.smali new file mode 100644 index 0000000000..bd59d5f68e --- /dev/null +++ b/test/518-null-array-get/smali/NullArraySuccessInt2Float.smali @@ -0,0 +1,33 @@ +# 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. + +# Check that the result of aget on null can be used as a float. + +.class public LNullArraySuccessInt2Float; + +.super Ljava/lang/Object; + +.method public static floatMethod(F)V + .registers 1 + return-void +.end method + +.method public static method()V + .registers 2 + const/4 v0, 0 + const/4 v1, 0 + aget v0, v0, v1 + invoke-static { v0 }, LNullArraySuccessInt2Float;->floatMethod(F)V + return-void +.end method diff --git a/test/518-null-array-get/smali/NullArraySuccessRef.smali b/test/518-null-array-get/smali/NullArraySuccessRef.smali new file mode 100644 index 0000000000..2f512d4089 --- /dev/null +++ b/test/518-null-array-get/smali/NullArraySuccessRef.smali @@ -0,0 +1,33 @@ +# 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. + +# Check that the result of aget-object on null can be used as a reference. + +.class public LNullArraySuccessRef; + +.super Ljava/lang/Object; + +.method public voidMethod()V + .registers 1 + return-void +.end method + +.method public static method()V + .registers 2 + const/4 v0, 0 + const/4 v1, 0 + aget-object v0, v0, v1 + invoke-virtual { v0 }, LNullArraySuccessRef;->voidMethod()V + return-void +.end method diff --git a/test/518-null-array-get/smali/NullArraySuccessShort.smali b/test/518-null-array-get/smali/NullArraySuccessShort.smali new file mode 100644 index 0000000000..d332e51f52 --- /dev/null +++ b/test/518-null-array-get/smali/NullArraySuccessShort.smali @@ -0,0 +1,33 @@ +# 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. + +# Check that the result of aget-short on null can be used as a short. + +.class public LNullArraySuccessShort; + +.super Ljava/lang/Object; + +.method public static shortMethod(S)V + .registers 1 + return-void +.end method + +.method public static method()V + .registers 2 + const/4 v0, 0 + const/4 v1, 0 + aget-short v0, v0, v1 + invoke-static { v0 }, LNullArraySuccessShort;->shortMethod(S)V + return-void +.end method diff --git a/test/518-null-array-get/src/Main.java b/test/518-null-array-get/src/Main.java index 66e50aacd7..678aef1f43 100644 --- a/test/518-null-array-get/src/Main.java +++ b/test/518-null-array-get/src/Main.java @@ -22,16 +22,36 @@ public class Main { class InnerClass {} public static void main(String[] args) throws Exception { - Class<?> c = Class.forName("NullArray"); - Method m = c.getMethod("method"); - Object[] arguments = { }; + checkLoad("NullArrayFailInt2Object", true); + checkLoad("NullArrayFailObject2Int", true); + checkLoad("NullArraySuccessInt", false); + checkLoad("NullArraySuccessInt2Float", false); + checkLoad("NullArraySuccessShort", false); + checkLoad("NullArraySuccessRef", false); + } + + private static void checkLoad(String className, boolean expectError) throws Exception { + Class<?> c; 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"); + c = Class.forName(className); + if (expectError) { + throw new RuntimeException("Expected error for " + className); + } + Method m = c.getMethod("method"); + try { + m.invoke(null); + throw new RuntimeException("Expected an InvocationTargetException"); + } catch (InvocationTargetException e) { + if (!(e.getCause() instanceof NullPointerException)) { + throw new RuntimeException("Expected a NullPointerException"); + } + System.out.println(className); + } + } catch (VerifyError e) { + if (!expectError) { + throw new RuntimeException(e); } + System.out.println(className); } } } diff --git a/test/530-checker-lse3/expected.txt b/test/530-checker-lse3/expected.txt new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/test/530-checker-lse3/expected.txt diff --git a/test/530-checker-lse3/info.txt b/test/530-checker-lse3/info.txt new file mode 100644 index 0000000000..29b4cb82ab --- /dev/null +++ b/test/530-checker-lse3/info.txt @@ -0,0 +1,4 @@ +Regression test for load store elimination not respecting the loaded type. When +a wider value is stored in a narrower field and then loaded from that field, +LSE needs to replace the value to be stored with a type conversion to the +narrower type. diff --git a/test/530-checker-lse3/smali/StoreLoad.smali b/test/530-checker-lse3/smali/StoreLoad.smali new file mode 100644 index 0000000000..7fb582c8d1 --- /dev/null +++ b/test/530-checker-lse3/smali/StoreLoad.smali @@ -0,0 +1,62 @@ +# 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. + +.class public LStoreLoad; + +.super Ljava/lang/Object; + +## CHECK-START: int StoreLoad.test(int) load_store_elimination (before) +## CHECK-DAG: <<Arg:i\d+>> ParameterValue +## CHECK-DAG: StaticFieldSet [{{l\d+}},<<Arg>>] field_name:StoreLoad.byteField +## CHECK-DAG: StaticFieldSet [{{l\d+}},<<Arg>>] field_name:StoreLoad.byteField2 +## CHECK-DAG: <<Val:b\d+>> StaticFieldGet [{{l\d+}}] field_name:StoreLoad.byteField +## CHECK-DAG: <<Val2:b\d+>> StaticFieldGet [{{l\d+}}] field_name:StoreLoad.byteField2 +## CHECK-DAG: <<Val3:i\d+>> Add [<<Val>>,<<Val2>>] +## CHECK-DAG: Return [<<Val3>>] + +## CHECK-START: int StoreLoad.test(int) load_store_elimination (after) +## CHECK-NOT: StaticFieldGet + +## CHECK-START: int StoreLoad.test(int) load_store_elimination (after) +## CHECK-DAG: <<Arg:i\d+>> ParameterValue +## CHECK-DAG: StaticFieldSet [{{l\d+}},<<Arg>>] field_name:StoreLoad.byteField +## CHECK-DAG: StaticFieldSet [{{l\d+}},<<Arg>>] field_name:StoreLoad.byteField2 +## CHECK-DAG: <<Conv:b\d+>> TypeConversion [<<Arg>>] +## CHECK-DAG: <<Val3:i\d+>> Add [<<Conv>>,<<Conv>>] +## CHECK-DAG: Return [<<Val3>>] +.method public static test(I)I + .registers 2 + sput-byte v1, LStoreLoad;->byteField:B + sput-byte v1, LStoreLoad;->byteField2:B + sget-byte v0, LStoreLoad;->byteField:B + sget-byte v1, LStoreLoad;->byteField2:B + add-int/2addr v0, v1 + return v0 +.end method + +## CHECK-START: int StoreLoad.test2(int) load_store_elimination (before) +## CHECK-DAG: <<Arg:i\d+>> ParameterValue +## CHECK-DAG: StaticFieldSet [{{l\d+}},<<Arg>>] field_name:StoreLoad.byteField +## CHECK-DAG: Return [<<Arg>>] + +## CHECK-START: int StoreLoad.test2(int) load_store_elimination (after) +## CHECK-NOT: TypeConversion +.method public static test2(I)I + .registers 1 + sput-byte v0, LStoreLoad;->byteField:B + return v0 +.end method + +.field public static byteField:B +.field public static byteField2:B diff --git a/test/530-checker-lse3/src/Main.java b/test/530-checker-lse3/src/Main.java new file mode 100644 index 0000000000..caef0b33cc --- /dev/null +++ b/test/530-checker-lse3/src/Main.java @@ -0,0 +1,48 @@ +/* + * 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. + */ + +import java.lang.reflect.Method; +import java.lang.reflect.Field; + +public class Main { + + // Workaround for b/18051191. + class InnerClass {} + + public static void main(String[] args) throws Exception { + Class<?> c = Class.forName("StoreLoad"); + Method m = c.getMethod("test", int.class); + int result = (Integer)m.invoke(null, 0x12345678); + if (result != (0x78 + 0x78)) { + throw new Error("Expected 240, got " + result); + } + m = c.getMethod("test2", int.class); + result = (Integer)m.invoke(null, 0xdeadbeef); + if (result != 0xdeadbeef) { + throw new Error("Expected 0xdeadbeef, got " + result); + } + Field f = c.getDeclaredField("byteField"); + byte b = f.getByte(null); + if (b != (byte)0xef) { + throw new Error("Expected 0xef, got " + b); + } + f = c.getDeclaredField("byteField2"); + b = f.getByte(null); + if (b != (byte)0x78) { + throw new Error("Expected 0xef, got " + b); + } + } +} diff --git a/test/706-jit-skip-compilation/expected.txt b/test/706-jit-skip-compilation/expected.txt deleted file mode 100644 index 6a5618ebc6..0000000000 --- a/test/706-jit-skip-compilation/expected.txt +++ /dev/null @@ -1 +0,0 @@ -JNI_OnLoad called diff --git a/test/706-jit-skip-compilation/info.txt b/test/706-jit-skip-compilation/info.txt deleted file mode 100644 index e9ef86bfb3..0000000000 --- a/test/706-jit-skip-compilation/info.txt +++ /dev/null @@ -1,4 +0,0 @@ -Regression test for the JIT crashing when compiling a method with invalid -dead dex code. For not compilable methods we don't gather samples and we don't -trigger JIT compilation. However kAccDontBotherCompile is not persisted in the -oat file and so we may end up compiling a method which we shouldn't. diff --git a/test/706-jit-skip-compilation/smali/errclass.smali b/test/706-jit-skip-compilation/smali/errclass.smali deleted file mode 100644 index 410504cb2f..0000000000 --- a/test/706-jit-skip-compilation/smali/errclass.smali +++ /dev/null @@ -1,34 +0,0 @@ -# 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. - - -.class public LErrClass; - -.super Ljava/lang/Object; - -.method public static errMethod()J - .registers 8 - const/4 v0, 0x0 - const/4 v3, 0x0 - aget v1, v0, v3 # v0 is null, this will alays throw and the invalid code - # below will not be verified. - move v3, v4 - move-wide/from16 v6, v2 # should trigger a verification error if verified as - # v3 is a single register but used as a pair here. - return v6 -.end method - -# Add a field to work around demerger bug b/18051191. -# Failure to verify dex file '...': Offset(552) should be zero when size is zero for field-ids. -.field private a:I diff --git a/test/706-jit-skip-compilation/src/Main.java b/test/706-jit-skip-compilation/src/Main.java deleted file mode 100644 index aa847248d6..0000000000 --- a/test/706-jit-skip-compilation/src/Main.java +++ /dev/null @@ -1,57 +0,0 @@ -/* - * 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. - */ - -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; - -public class Main { - public static void main(String[] args) throws Exception { - System.loadLibrary(args[0]); - Class<?> c = Class.forName("ErrClass"); - Method m = c.getMethod("errMethod"); - - // Print the counter before invokes. The golden file expects this to be 0. - int hotnessCounter = getHotnessCounter(c, "errMethod"); - if (hotnessCounter != 0) { - throw new RuntimeException("Unexpected hotnessCounter: " + hotnessCounter); - } - - // Loop enough to make sure the interpreter reports invocations count. - long result = 0; - for (int i = 0; i < 10000; i++) { - try { - result += (Long)m.invoke(null); - hotnessCounter = getHotnessCounter(c, "errMethod"); - if (hotnessCounter != 0) { - throw new RuntimeException( - "Unexpected hotnessCounter: " + hotnessCounter); - } - - } catch (InvocationTargetException e) { - if (!(e.getCause() instanceof NullPointerException)) { - throw e; - } - } - } - - // Not compilable methods should not increase their hotness counter. - if (hotnessCounter != 0) { - throw new RuntimeException("Unexpected hotnessCounter: " + hotnessCounter); - } - } - - public static native int getHotnessCounter(Class cls, String method_name); -} diff --git a/test/711-checker-type-conversion/src/Main.java b/test/711-checker-type-conversion/src/Main.java index 2c9c3a157e..ae58200b1b 100644 --- a/test/711-checker-type-conversion/src/Main.java +++ b/test/711-checker-type-conversion/src/Main.java @@ -22,6 +22,32 @@ public class Main { } } + public static void assertShortEquals(short expected, short result) { + if (expected != result) { + throw new Error("Expected: " + expected + ", found: " + result); + } + } + + public static void assertIntEquals(int expected, int result) { + if (expected != result) { + throw new Error("Expected: " + expected + ", found: " + result); + } + } + + public static void assertLongEquals(long expected, long result) { + if (expected != result) { + throw new Error("Expected: " + expected + ", found: " + result); + } + } + + public static void assertCharEquals(char expected, char result) { + if (expected != result) { + // Values are cast to int to display numeric values instead of + // (UTF-16 encoded) characters. + throw new Error("Expected: " + (int)expected + ", found: " + (int)result); + } + } + /// CHECK-START: byte Main.getByte1() constant_folding (before) /// CHECK: TypeConversion /// CHECK: TypeConversion @@ -69,9 +95,170 @@ public class Main { return (byte)((byte)i + (byte)i); } + static byte byteVal = -1; + static short shortVal = -1; + static char charVal = 0xffff; + static int intVal = -1; + + static byte[] byteArr = { 0 }; + static short[] shortArr = { 0 }; + static char[] charArr = { 0 }; + static int[] intArr = { 0 }; + + static byte $noinline$getByte() { + return byteVal; + } + + static short $noinline$getShort() { + return shortVal; + } + + static char $noinline$getChar() { + return charVal; + } + + static int $noinline$getInt() { + return intVal; + } + + static boolean sFlag = true; + + /// CHECK-START: void Main.byteToShort() instruction_simplifier$before_codegen (after) + /// CHECK-NOT: TypeConversion + private static void byteToShort() { + shortArr[0] = 0; + if (sFlag) { + shortArr[0] = $noinline$getByte(); + } + } + + /// CHECK-START: void Main.byteToChar() instruction_simplifier$before_codegen (after) + /// CHECK: TypeConversion + private static void byteToChar() { + charArr[0] = 0; + if (sFlag) { + charArr[0] = (char)$noinline$getByte(); + } + } + + /// CHECK-START: void Main.byteToInt() instruction_simplifier$before_codegen (after) + /// CHECK-NOT: TypeConversion + private static void byteToInt() { + intArr[0] = 0; + if (sFlag) { + intArr[0] = $noinline$getByte(); + } + } + + /// CHECK-START: void Main.charToByte() instruction_simplifier$before_codegen (after) + /// CHECK-NOT: TypeConversion + private static void charToByte() { + byteArr[0] = 0; + if (sFlag) { + byteArr[0] = (byte)$noinline$getChar(); + } + } + + /// CHECK-START: void Main.charToShort() instruction_simplifier$before_codegen (after) + /// CHECK-NOT: TypeConversion + private static void charToShort() { + shortArr[0] = 0; + if (sFlag) { + shortArr[0] = (short)$noinline$getChar(); + } + } + + /// CHECK-START: void Main.charToInt() instruction_simplifier$before_codegen (after) + /// CHECK-NOT: TypeConversion + private static void charToInt() { + intArr[0] = 0; + if (sFlag) { + intArr[0] = $noinline$getChar(); + } + } + + /// CHECK-START: void Main.shortToByte() instruction_simplifier$before_codegen (after) + /// CHECK-NOT: TypeConversion + private static void shortToByte() { + byteArr[0] = 0; + if (sFlag) { + byteArr[0] = (byte)$noinline$getShort(); + } + } + + /// CHECK-START: void Main.shortToChar() instruction_simplifier$before_codegen (after) + /// CHECK-NOT: TypeConversion + private static void shortToChar() { + charArr[0] = 0; + if (sFlag) { + charArr[0] = (char)$noinline$getShort(); + } + } + + /// CHECK-START: void Main.shortToInt() instruction_simplifier$before_codegen (after) + /// CHECK-NOT: TypeConversion + private static void shortToInt() { + intArr[0] = 0; + if (sFlag) { + intArr[0] = $noinline$getShort(); + } + } + + /// CHECK-START: void Main.intToByte() instruction_simplifier$before_codegen (after) + /// CHECK-NOT: TypeConversion + private static void intToByte() { + byteArr[0] = 0; + if (sFlag) { + byteArr[0] = (byte)$noinline$getInt(); + } + } + + /// CHECK-START: void Main.intToShort() instruction_simplifier$before_codegen (after) + /// CHECK-NOT: TypeConversion + private static void intToShort() { + shortArr[0] = 0; + if (sFlag) { + shortArr[0] = (short)$noinline$getInt(); + } + } + + /// CHECK-START: void Main.intToChar() instruction_simplifier$before_codegen (after) + /// CHECK-NOT: TypeConversion + private static void intToChar() { + charArr[0] = 0; + if (sFlag) { + charArr[0] = (char)$noinline$getInt(); + } + } + public static void main(String[] args) { assertByteEquals(getByte1(), (byte)-5); assertByteEquals(getByte2(), (byte)(-201)); assertByteEquals(getByte3(), (byte)(0xcd + 0xcd)); + + byteToShort(); + assertShortEquals(shortArr[0], (short)-1); + byteToChar(); + assertCharEquals(charArr[0], (char)-1); + byteToInt(); + assertIntEquals(intArr[0], -1); + charToByte(); + assertByteEquals(byteArr[0], (byte)-1); + charToShort(); + assertShortEquals(shortArr[0], (short)-1); + charToInt(); + assertIntEquals(intArr[0], 0xffff); + shortToByte(); + assertByteEquals(byteArr[0], (byte)-1); + shortToChar(); + assertCharEquals(charArr[0], (char)-1); + shortToInt(); + assertIntEquals(intArr[0], -1); + intToByte(); + assertByteEquals(byteArr[0], (byte)-1); + intToShort(); + assertShortEquals(shortArr[0], (short)-1); + intToChar(); + assertCharEquals(charArr[0], (char)-1); } } |