diff options
| -rw-r--r-- | compiler/optimizing/builder.cc | 12 | ||||
| -rw-r--r-- | compiler/optimizing/optimizing_compiler_stats.h | 15 | ||||
| -rw-r--r-- | runtime/verifier/method_verifier.cc | 26 | ||||
| -rw-r--r-- | test/800-smali/expected.txt | 1 | ||||
| -rw-r--r-- | test/800-smali/smali/b_17410612.smali | 14 | ||||
| -rw-r--r-- | test/800-smali/src/Main.java | 2 |
6 files changed, 52 insertions, 18 deletions
diff --git a/compiler/optimizing/builder.cc b/compiler/optimizing/builder.cc index cbd042901d..e4680ff2fa 100644 --- a/compiler/optimizing/builder.cc +++ b/compiler/optimizing/builder.cc @@ -748,13 +748,11 @@ bool HGraphBuilder::BuildInvoke(const Instruction& instruction, for (size_t i = start_index; i < number_of_vreg_arguments; i++, argument_index++) { Primitive::Type type = Primitive::GetType(descriptor[descriptor_index++]); bool is_wide = (type == Primitive::kPrimLong) || (type == Primitive::kPrimDouble); - if (!is_range && is_wide && args[i] + 1 != args[i + 1]) { - LOG(WARNING) << "Non sequential register pair in " << dex_compilation_unit_->GetSymbol() - << " at " << dex_pc; - // We do not implement non sequential register pair. - MaybeRecordStat(MethodCompilationStat::kNotCompiledNonSequentialRegPair); - return false; - } + // Longs and doubles should be in pairs, that is, sequential registers. The verifier should + // reject any class where this is violated. + DCHECK(is_range || !is_wide || (args[i] + 1 == args[i + 1])) + << "Non sequential register pair in " << dex_compilation_unit_->GetSymbol() + << " at " << dex_pc; HInstruction* arg = LoadLocal(is_range ? register_index + i : args[i], type); invoke->SetArgumentAt(argument_index, arg); if (is_wide) { diff --git a/compiler/optimizing/optimizing_compiler_stats.h b/compiler/optimizing/optimizing_compiler_stats.h index b6b1bb1cad..b988813f75 100644 --- a/compiler/optimizing/optimizing_compiler_stats.h +++ b/compiler/optimizing/optimizing_compiler_stats.h @@ -19,6 +19,7 @@ #include <sstream> #include <string> +#include <type_traits> #include "atomic.h" @@ -38,7 +39,6 @@ enum MethodCompilationStat { kNotCompiledHugeMethod, kNotCompiledLargeMethodNoBranches, kNotCompiledNoCodegen, - kNotCompiledNonSequentialRegPair, kNotCompiledPathological, kNotCompiledSpaceFilter, kNotCompiledUnhandledInstruction, @@ -84,14 +84,15 @@ class OptimizingCompilerStats { for (int i = 0; i < kLastStat; i++) { if (compile_stats_[i] != 0) { - LOG(INFO) << PrintMethodCompilationStat(i) << ": " << compile_stats_[i]; + LOG(INFO) << PrintMethodCompilationStat(static_cast<MethodCompilationStat>(i)) << ": " + << compile_stats_[i]; } } } } private: - std::string PrintMethodCompilationStat(int stat) const { + std::string PrintMethodCompilationStat(MethodCompilationStat stat) const { switch (stat) { case kAttemptCompilation : return "kAttemptCompilation"; case kCompiledBaseline : return "kCompiledBaseline"; @@ -106,7 +107,6 @@ class OptimizingCompilerStats { case kNotCompiledHugeMethod : return "kNotCompiledHugeMethod"; case kNotCompiledLargeMethodNoBranches : return "kNotCompiledLargeMethodNoBranches"; case kNotCompiledNoCodegen : return "kNotCompiledNoCodegen"; - case kNotCompiledNonSequentialRegPair : return "kNotCompiledNonSequentialRegPair"; case kNotCompiledPathological : return "kNotCompiledPathological"; case kNotCompiledSpaceFilter : return "kNotCompiledSpaceFilter"; case kNotCompiledUnhandledInstruction : return "kNotCompiledUnhandledInstruction"; @@ -120,9 +120,12 @@ class OptimizingCompilerStats { case kRemovedCheckedCast: return "kRemovedCheckedCast"; case kRemovedDeadInstruction: return "kRemovedDeadInstruction"; case kRemovedNullCheck: return "kRemovedNullCheck"; - default: LOG(FATAL) << "invalid stat"; + + case kLastStat: break; // Invalid to print out. } - return ""; + LOG(FATAL) << "invalid stat " + << static_cast<std::underlying_type<MethodCompilationStat>::type>(stat); + UNREACHABLE(); } AtomicInteger compile_stats_[kLastStat]; diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc index 4d88227956..b86a7ee966 100644 --- a/runtime/verifier/method_verifier.cc +++ b/runtime/verifier/method_verifier.cc @@ -3372,11 +3372,27 @@ ArtMethod* MethodVerifier::VerifyInvocationArgsFromIterator( << " but expected " << reg_type; return nullptr; } - } else if (!work_line_->VerifyRegisterType(this, get_reg, reg_type)) { - // Continue on soft failures. We need to find possible hard failures to avoid problems in the - // compiler. - if (have_pending_hard_failure_) { - return nullptr; + } else { + if (!work_line_->VerifyRegisterType(this, get_reg, reg_type)) { + // Continue on soft failures. We need to find possible hard failures to avoid problems in + // the compiler. + if (have_pending_hard_failure_) { + return nullptr; + } + } else if (reg_type.IsLongOrDoubleTypes()) { + // Check that registers are consecutive (for non-range invokes). Invokes are the only + // instructions not specifying register pairs by the first component, but require them + // nonetheless. Only check when there's an actual register in the parameters. If there's + // none, this will fail below. + if (!is_range && sig_registers + 1 < expected_args) { + uint32_t second_reg = arg[sig_registers + 1]; + if (second_reg != get_reg + 1) { + Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "Rejecting invocation, long or double parameter " + "at index " << sig_registers << " is not a pair: " << get_reg << " + " + << second_reg << "."; + return nullptr; + } + } } } sig_registers += reg_type.IsLongOrDoubleTypes() ? 2 : 1; diff --git a/test/800-smali/expected.txt b/test/800-smali/expected.txt index a6b216bf3a..85656374c5 100644 --- a/test/800-smali/expected.txt +++ b/test/800-smali/expected.txt @@ -16,4 +16,5 @@ MoveExc MoveExceptionOnEntry EmptySparseSwitch b/20224106 +b/17410612 Done! diff --git a/test/800-smali/smali/b_17410612.smali b/test/800-smali/smali/b_17410612.smali new file mode 100644 index 0000000000..17718cbf60 --- /dev/null +++ b/test/800-smali/smali/b_17410612.smali @@ -0,0 +1,14 @@ +.class public LB17410612; + +# Test that an invoke with a long parameter has the long parameter in +# a pair. This should fail in the verifier and not an abort in the compiler. + +.super Ljava/lang/Object; + +.method public static run()V + .registers 4 + const-wide v0, 0 # Make (v0, v1) a long + const-wide v2, 0 # Make (v2, v3) a long + invoke-static {v0, v3}, Ljava/lang/Long;->valueOf(J)Ljava/lang/Long; + return-void +.end method diff --git a/test/800-smali/src/Main.java b/test/800-smali/src/Main.java index 3e88364089..33df06d87a 100644 --- a/test/800-smali/src/Main.java +++ b/test/800-smali/src/Main.java @@ -81,6 +81,8 @@ public class Main { null)); testCases.add(new TestCase("b/20224106", "B20224106", "run", null, new VerifyError(), 0)); + testCases.add(new TestCase("b/17410612", "B17410612", "run", null, new VerifyError(), + 0)); } public void runTests() { |