ART: Check long and double register pairs in invokes
For invokes, ensure that long and double parameters are actually
in registers pairs. We were testing the pair, but skipping the
actual high parameter register.
Bug: 17410612
Change-Id: I8f4c3335ea8b7dc3cf252bee52a5a706ae8905f8
diff --git a/compiler/optimizing/builder.cc b/compiler/optimizing/builder.cc
index f98029d..e5fe6c3 100644
--- a/compiler/optimizing/builder.cc
+++ b/compiler/optimizing/builder.cc
@@ -748,13 +748,11 @@
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 b6b1bb1..b988813 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 @@
kNotCompiledHugeMethod,
kNotCompiledLargeMethodNoBranches,
kNotCompiledNoCodegen,
- kNotCompiledNonSequentialRegPair,
kNotCompiledPathological,
kNotCompiledSpaceFilter,
kNotCompiledUnhandledInstruction,
@@ -84,14 +84,15 @@
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 @@
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 @@
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 4d88227..b86a7ee 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -3372,11 +3372,27 @@
<< " 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 a6b216b..8565637 100644
--- a/test/800-smali/expected.txt
+++ b/test/800-smali/expected.txt
@@ -16,4 +16,5 @@
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 0000000..17718cb
--- /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 3e88364..33df06d 100644
--- a/test/800-smali/src/Main.java
+++ b/test/800-smali/src/Main.java
@@ -81,6 +81,8 @@
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() {