summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--compiler/dex/mir_method_info.cc6
-rw-r--r--compiler/optimizing/code_generator_arm64.cc3
-rw-r--r--compiler/optimizing/nodes.h12
-rw-r--r--compiler/optimizing/ssa_builder.cc15
-rw-r--r--runtime/hprof/hprof.cc39
-rw-r--r--runtime/jit/profile_saver.cc10
-rw-r--r--test/563-checker-fakestring/smali/TestCase.smali56
-rw-r--r--test/563-checker-fakestring/src/Main.java15
8 files changed, 116 insertions, 40 deletions
diff --git a/compiler/dex/mir_method_info.cc b/compiler/dex/mir_method_info.cc
index 658e7d67a0..c250bd9fd2 100644
--- a/compiler/dex/mir_method_info.cc
+++ b/compiler/dex/mir_method_info.cc
@@ -100,8 +100,12 @@ void MirMethodLoweringInfo::Resolve(CompilerDriver* compiler_driver,
} else {
// The method index is actually the dex PC in this case.
// Calculate the proper dex file and target method idx.
+
+ // We must be in JIT mode if we get here.
CHECK(use_jit);
- CHECK_EQ(invoke_type, kVirtual);
+
+ // The invoke type better be virtual, except for the string init special case above.
+ CHECK_EQ(invoke_type, string_init ? kDirect : kVirtual);
// Don't devirt if we are in a different dex file since we can't have direct invokes in
// another dex file unless we always put a direct / patch pointer.
devirt_target = nullptr;
diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc
index 2cb2741b17..3d65e9c53c 100644
--- a/compiler/optimizing/code_generator_arm64.cc
+++ b/compiler/optimizing/code_generator_arm64.cc
@@ -2820,7 +2820,8 @@ void InstructionCodeGeneratorARM64::GenerateTestAndBranch(HInstruction* instruct
non_fallthrough_target = true_target;
}
- if ((arm64_cond != gt && arm64_cond != le) && rhs.IsImmediate() && (rhs.immediate() == 0)) {
+ if ((arm64_cond == eq || arm64_cond == ne || arm64_cond == lt || arm64_cond == ge) &&
+ rhs.IsImmediate() && (rhs.immediate() == 0)) {
switch (arm64_cond) {
case eq:
__ Cbz(lhs, non_fallthrough_target);
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index e222ef7260..019be5d494 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -3689,19 +3689,13 @@ class HInvokeStaticOrDirect : public HInvoke {
DCHECK(!IsStaticWithExplicitClinitCheck());
}
- HNewInstance* GetThisArgumentOfStringInit() const {
+ HInstruction* GetAndRemoveThisArgumentOfStringInit() {
DCHECK(IsStringInit());
size_t index = InputCount() - 1;
- DCHECK(InputAt(index)->IsNewInstance());
- return InputAt(index)->AsNewInstance();
- }
-
- void RemoveThisArgumentOfStringInit() {
- DCHECK(IsStringInit());
- size_t index = InputCount() - 1;
- DCHECK(InputAt(index)->IsNewInstance());
+ HInstruction* input = InputAt(index);
RemoveAsUserOfInput(index);
inputs_.pop_back();
+ return input;
}
// Is this a call to a static method whose declaring class has an
diff --git a/compiler/optimizing/ssa_builder.cc b/compiler/optimizing/ssa_builder.cc
index c0011cde8d..165d09d1a5 100644
--- a/compiler/optimizing/ssa_builder.cc
+++ b/compiler/optimizing/ssa_builder.cc
@@ -927,16 +927,21 @@ void SsaBuilder::VisitInvokeStaticOrDirect(HInvokeStaticOrDirect* invoke) {
if (invoke->IsStringInit()) {
// This is a StringFactory call which acts as a String constructor. Its
// result replaces the empty String pre-allocated by NewInstance.
- HNewInstance* new_instance = invoke->GetThisArgumentOfStringInit();
- invoke->RemoveThisArgumentOfStringInit();
+ HInstruction* arg_this = invoke->GetAndRemoveThisArgumentOfStringInit();
// Replacing the NewInstance might render it redundant. Keep a list of these
// to be visited once it is clear whether it is has remaining uses.
- uninitialized_strings_.push_back(new_instance);
+ if (arg_this->IsNewInstance()) {
+ uninitialized_strings_.push_back(arg_this->AsNewInstance());
+ } else {
+ DCHECK(arg_this->IsPhi());
+ // NewInstance is not the direct input of the StringFactory call. It might
+ // be redundant but optimizing this case is not worth the effort.
+ }
- // Walk over all vregs and replace any occurrence of `new_instance` with `invoke`.
+ // Walk over all vregs and replace any occurrence of `arg_this` with `invoke`.
for (size_t vreg = 0, e = current_locals_->size(); vreg < e; ++vreg) {
- if ((*current_locals_)[vreg] == new_instance) {
+ if ((*current_locals_)[vreg] == arg_this) {
(*current_locals_)[vreg] = invoke;
}
}
diff --git a/runtime/hprof/hprof.cc b/runtime/hprof/hprof.cc
index dfc1f5fe71..bb35ec73d9 100644
--- a/runtime/hprof/hprof.cc
+++ b/runtime/hprof/hprof.cc
@@ -419,18 +419,13 @@ class Hprof : public SingleRootVisitor {
Hprof(const char* output_filename, int fd, bool direct_to_ddms)
: filename_(output_filename),
fd_(fd),
- direct_to_ddms_(direct_to_ddms),
- start_ns_(NanoTime()),
- output_(nullptr),
- current_heap_(HPROF_HEAP_DEFAULT),
- objects_in_segment_(0),
- next_string_id_(0x400000),
- next_class_serial_number_(1) {
+ direct_to_ddms_(direct_to_ddms) {
LOG(INFO) << "hprof: heap dump \"" << filename_ << "\" starting...";
}
void Dump()
- REQUIRES(Locks::mutator_lock_, !Locks::heap_bitmap_lock_, !Locks::alloc_tracker_lock_) {
+ REQUIRES(Locks::mutator_lock_)
+ REQUIRES(!Locks::heap_bitmap_lock_, !Locks::alloc_tracker_lock_) {
{
MutexLock mu(Thread::Current(), *Locks::alloc_tracker_lock_);
if (Runtime::Current()->GetHeap()->IsAllocTrackingEnabled()) {
@@ -462,10 +457,11 @@ class Hprof : public SingleRootVisitor {
}
if (okay) {
- uint64_t duration = NanoTime() - start_ns_;
- LOG(INFO) << "hprof: heap dump completed ("
- << PrettySize(RoundUp(overall_size, 1024))
- << ") in " << PrettyDuration(duration);
+ const uint64_t duration = NanoTime() - start_ns_;
+ LOG(INFO) << "hprof: heap dump completed (" << PrettySize(RoundUp(overall_size, KB))
+ << ") in " << PrettyDuration(duration)
+ << " objects " << total_objects_
+ << " objects with stack traces " << total_objects_with_stack_trace_;
}
}
@@ -855,7 +851,7 @@ class Hprof : public SingleRootVisitor {
}
CHECK_EQ(traces_.size(), next_trace_sn - kHprofNullStackTrace - 1);
CHECK_EQ(frames_.size(), next_frame_id);
- VLOG(heap) << "hprof: found " << count << " objects with allocation stack traces";
+ total_objects_with_stack_trace_ = count;
}
// If direct_to_ddms_ is set, "filename_" and "fd" will be ignored.
@@ -865,16 +861,19 @@ class Hprof : public SingleRootVisitor {
int fd_;
bool direct_to_ddms_;
- uint64_t start_ns_;
+ uint64_t start_ns_ = NanoTime();
- EndianOutput* output_;
+ EndianOutput* output_ = nullptr;
- HprofHeapId current_heap_; // Which heap we're currently dumping.
- size_t objects_in_segment_;
+ HprofHeapId current_heap_ = HPROF_HEAP_DEFAULT; // Which heap we're currently dumping.
+ size_t objects_in_segment_ = 0;
- HprofStringId next_string_id_;
+ size_t total_objects_ = 0u;
+ size_t total_objects_with_stack_trace_ = 0u;
+
+ HprofStringId next_string_id_ = 0x400000;
SafeMap<std::string, HprofStringId> strings_;
- HprofClassSerialNumber next_class_serial_number_;
+ HprofClassSerialNumber next_class_serial_number_ = 1;
SafeMap<mirror::Class*, HprofClassSerialNumber> classes_;
std::unordered_map<const gc::AllocRecordStackTrace*, HprofStackTraceSerialNumber,
@@ -1064,6 +1063,8 @@ void Hprof::DumpHeapObject(mirror::Object* obj) {
return;
}
+ ++total_objects_;
+
GcRootVisitor visitor(this);
obj->VisitReferences(visitor, VoidFunctor());
diff --git a/runtime/jit/profile_saver.cc b/runtime/jit/profile_saver.cc
index fc257c0441..f3f5f95fb2 100644
--- a/runtime/jit/profile_saver.cc
+++ b/runtime/jit/profile_saver.cc
@@ -22,16 +22,16 @@
namespace art {
-// An arbitrary value to throttle save requests. Set to 500ms for now.
+// An arbitrary value to throttle save requests. Set to 2s for now.
static constexpr const uint64_t kMilisecondsToNano = 1000000;
-static constexpr const uint64_t kMinimumTimeBetweenCodeCacheUpdatesNs = 500 * kMilisecondsToNano;
+static constexpr const uint64_t kMinimumTimeBetweenCodeCacheUpdatesNs = 2000 * kMilisecondsToNano;
// TODO: read the constants from ProfileOptions,
// Add a random delay each time we go to sleep so that we don't hammer the CPU
// with all profile savers running at the same time.
-static constexpr const uint64_t kRandomDelayMaxMs = 10 * 1000; // 10 seconds
-static constexpr const uint64_t kMaxBackoffMs = 4 * 60 * 1000; // 4 minutes
-static constexpr const uint64_t kSavePeriodMs = 4 * 1000; // 4 seconds
+static constexpr const uint64_t kRandomDelayMaxMs = 20 * 1000; // 20 seconds
+static constexpr const uint64_t kMaxBackoffMs = 5 * 60 * 1000; // 5 minutes
+static constexpr const uint64_t kSavePeriodMs = 10 * 1000; // 10 seconds
static constexpr const double kBackoffCoef = 1.5;
static constexpr const uint32_t kMinimumNrOrMethodsToSave = 10;
diff --git a/test/563-checker-fakestring/smali/TestCase.smali b/test/563-checker-fakestring/smali/TestCase.smali
index 4bd804da26..54312a43d0 100644
--- a/test/563-checker-fakestring/smali/TestCase.smali
+++ b/test/563-checker-fakestring/smali/TestCase.smali
@@ -124,3 +124,59 @@
return-object v0
.end method
+
+# Test that the compiler does not assume that the first argument of String.<init>
+# is a NewInstance by inserting an irreducible loop between them (b/26676472).
+
+# We verify the type of the input instruction (Phi) in debuggable mode, because
+# it is eliminated by later stages of SsaBuilder otherwise.
+
+## CHECK-START-DEBUGGABLE: java.lang.String TestCase.thisNotNewInstance1(byte[], boolean) register (after)
+## CHECK-DAG: InvokeStaticOrDirect env:[[<<Phi:l\d+>>,{{.*]]}}
+## CHECK-DAG: <<Phi>> Phi
+
+.method public static thisNotNewInstance1([BZ)Ljava/lang/String;
+ .registers 5
+
+ new-instance v0, Ljava/lang/String;
+
+ # Irreducible loop
+ if-eqz p1, :loop_entry
+ :loop_header
+ const v1, 0x1
+ xor-int p1, p1, v1
+ :loop_entry
+ if-eqz p1, :string_init
+ goto :loop_header
+
+ :string_init
+ const-string v1, "UTF8"
+ invoke-direct {v0, p0, v1}, Ljava/lang/String;-><init>([BLjava/lang/String;)V
+ return-object v0
+
+.end method
+
+## CHECK-START-DEBUGGABLE: java.lang.String TestCase.thisNotNewInstance2(byte[], boolean) register (after)
+## CHECK-DAG: InvokeStaticOrDirect env:[[<<Phi:l\d+>>,{{.*]]}}
+## CHECK-DAG: <<Phi>> Phi
+
+.method public static thisNotNewInstance2([BZ)Ljava/lang/String;
+ .registers 5
+
+ new-instance v0, Ljava/lang/String;
+
+ # Irreducible loop
+ if-eqz p1, :loop_entry
+ :loop_header
+ if-eqz p1, :string_init
+ :loop_entry
+ const v1, 0x1
+ xor-int p1, p1, v1
+ goto :loop_header
+
+ :string_init
+ const-string v1, "UTF8"
+ invoke-direct {v0, p0, v1}, Ljava/lang/String;-><init>([BLjava/lang/String;)V
+ return-object v0
+
+.end method
diff --git a/test/563-checker-fakestring/src/Main.java b/test/563-checker-fakestring/src/Main.java
index 04df0f637c..1ac8a5bfcf 100644
--- a/test/563-checker-fakestring/src/Main.java
+++ b/test/563-checker-fakestring/src/Main.java
@@ -63,5 +63,20 @@ public class Main {
String result = (String) m.invoke(null, new Object[] { testData });
assertEqual(testString, result);
}
+
+ {
+ Method m = c.getMethod("thisNotNewInstance1", byte[].class, boolean.class);
+ String result = (String) m.invoke(null, new Object[] { testData, true });
+ assertEqual(testString, result);
+ result = (String) m.invoke(null, new Object[] { testData, false });
+ assertEqual(testString, result);
+ }
+ {
+ Method m = c.getMethod("thisNotNewInstance2", byte[].class, boolean.class);
+ String result = (String) m.invoke(null, new Object[] { testData, true });
+ assertEqual(testString, result);
+ result = (String) m.invoke(null, new Object[] { testData, false });
+ assertEqual(testString, result);
+ }
}
}
generated by cgit v1.2.3-59-g8ed1b (git 2.39.0) at 2025-12-28 00:11:31 +0000