summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--compiler/dex/quick/arm64/utility_arm64.cc6
-rw-r--r--runtime/base/mutex.cc5
-rw-r--r--runtime/base/mutex.h6
-rw-r--r--runtime/debugger.cc4
-rw-r--r--runtime/gc/heap.cc2
-rw-r--r--runtime/instrumentation.cc53
-rw-r--r--runtime/instrumentation.h14
-rw-r--r--runtime/native/dalvik_system_VMDebug.cc4
-rw-r--r--runtime/runtime.cc14
-rw-r--r--runtime/runtime.h3
-rw-r--r--runtime/trace.cc27
-rw-r--r--test/083-compiler-regressions/expected.txt1
-rw-r--r--test/083-compiler-regressions/src/Main.java13
13 files changed, 99 insertions, 53 deletions
diff --git a/compiler/dex/quick/arm64/utility_arm64.cc b/compiler/dex/quick/arm64/utility_arm64.cc
index 5326e74e16..f58f83070b 100644
--- a/compiler/dex/quick/arm64/utility_arm64.cc
+++ b/compiler/dex/quick/arm64/utility_arm64.cc
@@ -345,7 +345,7 @@ bool Arm64Mir2Lir::InexpensiveConstantInt(int32_t value, Instruction::Code opcod
case Instruction::SUB_INT_2ADDR:
// The code below is consistent with the implementation of OpRegRegImm().
{
- int32_t abs_value = std::abs(value);
+ uint32_t abs_value = (value == INT_MIN) ? value : std::abs(value);
if (abs_value < 0x1000) {
return true;
} else if ((abs_value & UINT64_C(0xfff)) == 0 && ((abs_value >> 12) < 0x1000)) {
@@ -809,7 +809,7 @@ LIR* Arm64Mir2Lir::OpRegRegImm(OpKind op, RegStorage r_dest, RegStorage r_src1,
LIR* Arm64Mir2Lir::OpRegRegImm64(OpKind op, RegStorage r_dest, RegStorage r_src1, int64_t value) {
LIR* res;
bool neg = (value < 0);
- int64_t abs_value = (neg) ? -value : value;
+ uint64_t abs_value = (neg & !(value == LLONG_MIN)) ? -value : value;
ArmOpcode opcode = kA64Brk1d;
ArmOpcode alt_opcode = kA64Brk1d;
bool is_logical = false;
@@ -942,7 +942,7 @@ LIR* Arm64Mir2Lir::OpRegImm64(OpKind op, RegStorage r_dest_src1, int64_t value)
ArmOpcode neg_opcode = kA64Brk1d;
bool shift;
bool neg = (value < 0);
- uint64_t abs_value = (neg) ? -value : value;
+ uint64_t abs_value = (neg & !(value == LLONG_MIN)) ? -value : value;
if (LIKELY(abs_value < 0x1000)) {
// abs_value is a 12-bit immediate.
diff --git a/runtime/base/mutex.cc b/runtime/base/mutex.cc
index fae8ba972c..9b9741121c 100644
--- a/runtime/base/mutex.cc
+++ b/runtime/base/mutex.cc
@@ -37,6 +37,7 @@ ReaderWriterMutex* Locks::breakpoint_lock_ = nullptr;
ReaderWriterMutex* Locks::classlinker_classes_lock_ = nullptr;
Mutex* Locks::deoptimization_lock_ = nullptr;
ReaderWriterMutex* Locks::heap_bitmap_lock_ = nullptr;
+Mutex* Locks::instrument_entrypoints_lock_ = nullptr;
Mutex* Locks::intern_table_lock_ = nullptr;
Mutex* Locks::logging_lock_ = nullptr;
Mutex* Locks::mem_maps_lock_ = nullptr;
@@ -869,6 +870,10 @@ void Locks::Init() {
} \
current_lock_level = new_level;
+ UPDATE_CURRENT_LOCK_LEVEL(kInstrumentEntrypointsLock);
+ DCHECK(instrument_entrypoints_lock_ == nullptr);
+ instrument_entrypoints_lock_ = new Mutex("instrument entrypoint lock", current_lock_level);
+
UPDATE_CURRENT_LOCK_LEVEL(kMutatorLock);
DCHECK(mutator_lock_ == nullptr);
mutator_lock_ = new ReaderWriterMutex("mutator lock", current_lock_level);
diff --git a/runtime/base/mutex.h b/runtime/base/mutex.h
index 013c078fff..1847f6b7f2 100644
--- a/runtime/base/mutex.h
+++ b/runtime/base/mutex.h
@@ -100,6 +100,7 @@ enum LockLevel {
kTraceLock,
kHeapBitmapLock,
kMutatorLock,
+ kInstrumentEntrypointsLock,
kThreadListSuspendThreadLock,
kZygoteCreationLock,
@@ -491,6 +492,9 @@ class Locks {
// potential deadlock cycle.
static Mutex* thread_list_suspend_thread_lock_;
+ // Guards allocation entrypoint instrumenting.
+ static Mutex* instrument_entrypoints_lock_ ACQUIRED_AFTER(thread_list_suspend_thread_lock_);
+
// The mutator_lock_ is used to allow mutators to execute in a shared (reader) mode or to block
// mutators by having an exclusive (writer) owner. In normal execution each mutator thread holds
// a share on the mutator_lock_. The garbage collector may also execute with shared access but
@@ -549,7 +553,7 @@ class Locks {
// else | .. running ..
// Goto x | .. running ..
// .. running .. | .. running ..
- static ReaderWriterMutex* mutator_lock_ ACQUIRED_AFTER(thread_list_suspend_thread_lock_);
+ static ReaderWriterMutex* mutator_lock_ ACQUIRED_AFTER(instrument_entrypoints_lock_);
// Allow reader-writer mutual exclusion on the mark and live bitmaps of the heap.
static ReaderWriterMutex* heap_bitmap_lock_ ACQUIRED_AFTER(mutator_lock_);
diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index ce3172c2ff..7fb199c0eb 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -4339,7 +4339,7 @@ void Dbg::SetAllocTrackingEnabled(bool enable) {
recent_allocation_records_ = new AllocRecord[alloc_record_max_];
CHECK(recent_allocation_records_ != NULL);
}
- Runtime::Current()->GetInstrumentation()->InstrumentQuickAllocEntryPoints(false);
+ Runtime::Current()->GetInstrumentation()->InstrumentQuickAllocEntryPoints();
} else {
{
ScopedObjectAccess soa(self); // For type_cache_.Clear();
@@ -4355,7 +4355,7 @@ void Dbg::SetAllocTrackingEnabled(bool enable) {
type_cache_.Clear();
}
// If an allocation comes in before we uninstrument, we will safely drop it on the floor.
- Runtime::Current()->GetInstrumentation()->UninstrumentQuickAllocEntryPoints(false);
+ Runtime::Current()->GetInstrumentation()->UninstrumentQuickAllocEntryPoints();
}
}
diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index 9604c77b67..6af98cf14e 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -423,7 +423,7 @@ Heap::Heap(size_t initial_size, size_t growth_limit, size_t min_free, size_t max
}
}
if (running_on_valgrind_) {
- Runtime::Current()->GetInstrumentation()->InstrumentQuickAllocEntryPoints(false);
+ Runtime::Current()->GetInstrumentation()->InstrumentQuickAllocEntryPoints();
}
if (VLOG_IS_ON(heap) || VLOG_IS_ON(startup)) {
LOG(INFO) << "Heap() exiting";
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 88a57f2a6e..652d29bf05 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -620,45 +620,52 @@ static void ResetQuickAllocEntryPointsForThread(Thread* thread, void* arg) {
thread->ResetQuickAllocEntryPointsForThread();
}
-void Instrumentation::SetEntrypointsInstrumented(bool instrumented, bool suspended) {
+void Instrumentation::SetEntrypointsInstrumented(bool instrumented) {
+ Thread* self = Thread::Current();
Runtime* runtime = Runtime::Current();
ThreadList* tl = runtime->GetThreadList();
- if (suspended) {
- Locks::mutator_lock_->AssertExclusiveHeld(Thread::Current());
- }
- if (runtime->IsStarted() && !suspended) {
+ Locks::mutator_lock_->AssertNotHeld(self);
+ Locks::instrument_entrypoints_lock_->AssertHeld(self);
+ if (runtime->IsStarted()) {
tl->SuspendAll();
}
{
- MutexLock mu(Thread::Current(), *Locks::runtime_shutdown_lock_);
+ MutexLock mu(self, *Locks::runtime_shutdown_lock_);
SetQuickAllocEntryPointsInstrumented(instrumented);
ResetQuickAllocEntryPoints();
}
- if (runtime->IsStarted() && !suspended) {
+ if (runtime->IsStarted()) {
tl->ResumeAll();
}
}
-void Instrumentation::InstrumentQuickAllocEntryPoints(bool suspended) {
- // TODO: the read of quick_alloc_entry_points_instrumentation_counter_ is racey and this code
- // should be guarded by a lock.
- DCHECK_GE(quick_alloc_entry_points_instrumentation_counter_.LoadSequentiallyConsistent(), 0);
- const bool enable_instrumentation =
- quick_alloc_entry_points_instrumentation_counter_.FetchAndAddSequentiallyConsistent(1) == 0;
- if (enable_instrumentation) {
- SetEntrypointsInstrumented(true, suspended);
+void Instrumentation::InstrumentQuickAllocEntryPoints() {
+ MutexLock mu(Thread::Current(), *Locks::instrument_entrypoints_lock_);
+ InstrumentQuickAllocEntryPointsLocked();
+}
+
+void Instrumentation::UninstrumentQuickAllocEntryPoints() {
+ MutexLock mu(Thread::Current(), *Locks::instrument_entrypoints_lock_);
+ UninstrumentQuickAllocEntryPointsLocked();
+}
+
+void Instrumentation::InstrumentQuickAllocEntryPointsLocked() {
+ Locks::instrument_entrypoints_lock_->AssertHeld(Thread::Current());
+ if (quick_alloc_entry_points_instrumentation_counter_ == 0) {
+ SetEntrypointsInstrumented(true);
}
+ ++quick_alloc_entry_points_instrumentation_counter_;
+ LOG(INFO) << "Counter: " << quick_alloc_entry_points_instrumentation_counter_;
}
-void Instrumentation::UninstrumentQuickAllocEntryPoints(bool suspended) {
- // TODO: the read of quick_alloc_entry_points_instrumentation_counter_ is racey and this code
- // should be guarded by a lock.
- DCHECK_GT(quick_alloc_entry_points_instrumentation_counter_.LoadSequentiallyConsistent(), 0);
- const bool disable_instrumentation =
- quick_alloc_entry_points_instrumentation_counter_.FetchAndSubSequentiallyConsistent(1) == 1;
- if (disable_instrumentation) {
- SetEntrypointsInstrumented(false, suspended);
+void Instrumentation::UninstrumentQuickAllocEntryPointsLocked() {
+ Locks::instrument_entrypoints_lock_->AssertHeld(Thread::Current());
+ CHECK_GT(quick_alloc_entry_points_instrumentation_counter_, 0U);
+ --quick_alloc_entry_points_instrumentation_counter_;
+ if (quick_alloc_entry_points_instrumentation_counter_ == 0) {
+ SetEntrypointsInstrumented(false);
}
+ LOG(INFO) << "Counter: " << quick_alloc_entry_points_instrumentation_counter_;
}
void Instrumentation::ResetQuickAllocEntryPoints() {
diff --git a/runtime/instrumentation.h b/runtime/instrumentation.h
index 3c1c756992..3017bf6a38 100644
--- a/runtime/instrumentation.h
+++ b/runtime/instrumentation.h
@@ -182,9 +182,13 @@ class Instrumentation {
return interpreter_handler_table_;
}
- void InstrumentQuickAllocEntryPoints(bool suspended)
+ void InstrumentQuickAllocEntryPoints() LOCKS_EXCLUDED(Locks::instrument_entrypoints_lock_);
+ void UninstrumentQuickAllocEntryPoints() LOCKS_EXCLUDED(Locks::instrument_entrypoints_lock_);
+ void InstrumentQuickAllocEntryPointsLocked()
+ EXCLUSIVE_LOCKS_REQUIRED(Locks::instrument_entrypoints_lock_)
LOCKS_EXCLUDED(Locks::thread_list_lock_, Locks::runtime_shutdown_lock_);
- void UninstrumentQuickAllocEntryPoints(bool suspended)
+ void UninstrumentQuickAllocEntryPointsLocked()
+ EXCLUSIVE_LOCKS_REQUIRED(Locks::instrument_entrypoints_lock_)
LOCKS_EXCLUDED(Locks::thread_list_lock_, Locks::runtime_shutdown_lock_);
void ResetQuickAllocEntryPoints() EXCLUSIVE_LOCKS_REQUIRED(Locks::runtime_shutdown_lock_);
@@ -350,7 +354,7 @@ class Instrumentation {
// No thread safety analysis to get around SetQuickAllocEntryPointsInstrumented requiring
// exclusive access to mutator lock which you can't get if the runtime isn't started.
- void SetEntrypointsInstrumented(bool instrumented, bool suspended) NO_THREAD_SAFETY_ANALYSIS;
+ void SetEntrypointsInstrumented(bool instrumented) NO_THREAD_SAFETY_ANALYSIS;
void MethodEnterEventImpl(Thread* thread, mirror::Object* this_object,
mirror::ArtMethod* method, uint32_t dex_pc) const
@@ -455,8 +459,8 @@ class Instrumentation {
InterpreterHandlerTable interpreter_handler_table_ GUARDED_BY(Locks::mutator_lock_);
// Greater than 0 if quick alloc entry points instrumented.
- // TODO: The access and changes to this is racy and should be guarded by a lock.
- AtomicInteger quick_alloc_entry_points_instrumentation_counter_;
+ size_t quick_alloc_entry_points_instrumentation_counter_
+ GUARDED_BY(Locks::instrument_entrypoints_lock_);
DISALLOW_COPY_AND_ASSIGN(Instrumentation);
};
diff --git a/runtime/native/dalvik_system_VMDebug.cc b/runtime/native/dalvik_system_VMDebug.cc
index d8a537f948..ceff2065ba 100644
--- a/runtime/native/dalvik_system_VMDebug.cc
+++ b/runtime/native/dalvik_system_VMDebug.cc
@@ -60,11 +60,11 @@ static jobjectArray VMDebug_getVmFeatureList(JNIEnv* env, jclass) {
}
static void VMDebug_startAllocCounting(JNIEnv*, jclass) {
- Runtime::Current()->SetStatsEnabled(true, false);
+ Runtime::Current()->SetStatsEnabled(true);
}
static void VMDebug_stopAllocCounting(JNIEnv*, jclass) {
- Runtime::Current()->SetStatsEnabled(false, false);
+ Runtime::Current()->SetStatsEnabled(false);
}
static jint VMDebug_getAllocCount(JNIEnv*, jclass, jint kind) {
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 6839b5174f..5b24a1d594 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -1000,14 +1000,18 @@ void Runtime::DumpLockHolders(std::ostream& os) {
}
}
-void Runtime::SetStatsEnabled(bool new_state, bool suspended) {
+void Runtime::SetStatsEnabled(bool new_state) {
+ Thread* self = Thread::Current();
+ MutexLock mu(self, *Locks::instrument_entrypoints_lock_);
if (new_state == true) {
GetStats()->Clear(~0);
// TODO: wouldn't it make more sense to clear _all_ threads' stats?
- Thread::Current()->GetStats()->Clear(~0);
- GetInstrumentation()->InstrumentQuickAllocEntryPoints(suspended);
- } else {
- GetInstrumentation()->UninstrumentQuickAllocEntryPoints(suspended);
+ self->GetStats()->Clear(~0);
+ if (stats_enabled_ != new_state) {
+ GetInstrumentation()->InstrumentQuickAllocEntryPointsLocked();
+ }
+ } else if (stats_enabled_ != new_state) {
+ GetInstrumentation()->UninstrumentQuickAllocEntryPointsLocked();
}
stats_enabled_ = new_state;
}
diff --git a/runtime/runtime.h b/runtime/runtime.h
index 96924eca37..4bf78a2885 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -385,7 +385,8 @@ class Runtime {
void ResetStats(int kinds);
- void SetStatsEnabled(bool new_state, bool suspended);
+ void SetStatsEnabled(bool new_state) LOCKS_EXCLUDED(Locks::instrument_entrypoints_lock_,
+ Locks::mutator_lock_);
enum class NativeBridgeAction { // private
kUnload,
diff --git a/runtime/trace.cc b/runtime/trace.cc
index b32e0429b1..4bb388f3ee 100644
--- a/runtime/trace.cc
+++ b/runtime/trace.cc
@@ -361,6 +361,10 @@ void Trace::Start(const char* trace_filename, int trace_fd, int buffer_size, int
}
Runtime* runtime = Runtime::Current();
+
+ // Enable count of allocs if specified in the flags.
+ bool enable_stats = false;
+
runtime->GetThreadList()->SuspendAll();
// Create Trace object.
@@ -369,13 +373,8 @@ void Trace::Start(const char* trace_filename, int trace_fd, int buffer_size, int
if (the_trace_ != NULL) {
LOG(ERROR) << "Trace already in progress, ignoring this request";
} else {
+ enable_stats = (flags && kTraceCountAllocs) != 0;
the_trace_ = new Trace(trace_file.release(), buffer_size, flags, sampling_enabled);
-
- // Enable count of allocs if specified in the flags.
- if ((flags && kTraceCountAllocs) != 0) {
- runtime->SetStatsEnabled(true, true);
- }
-
if (sampling_enabled) {
CHECK_PTHREAD_CALL(pthread_create, (&sampling_pthread_, NULL, &RunSamplingThread,
reinterpret_cast<void*>(interval_us)),
@@ -391,9 +390,15 @@ void Trace::Start(const char* trace_filename, int trace_fd, int buffer_size, int
}
runtime->GetThreadList()->ResumeAll();
+
+ // Can't call this when holding the mutator lock.
+ if (enable_stats) {
+ runtime->SetStatsEnabled(true);
+ }
}
void Trace::Stop() {
+ bool stop_alloc_counting = false;
Runtime* runtime = Runtime::Current();
runtime->GetThreadList()->SuspendAll();
Trace* the_trace = NULL;
@@ -409,6 +414,7 @@ void Trace::Stop() {
}
}
if (the_trace != NULL) {
+ stop_alloc_counting = (the_trace->flags_ & kTraceCountAllocs) != 0;
the_trace->FinishTracing();
if (the_trace->sampling_enabled_) {
@@ -425,6 +431,11 @@ void Trace::Stop() {
}
runtime->GetThreadList()->ResumeAll();
+ if (stop_alloc_counting) {
+ // Can be racy since SetStatsEnabled is not guarded by any locks.
+ Runtime::Current()->SetStatsEnabled(false);
+ }
+
if (sampling_pthread != 0U) {
CHECK_PTHREAD_CALL(pthread_join, (sampling_pthread, NULL), "sampling thread shutdown");
sampling_pthread_ = 0U;
@@ -489,10 +500,6 @@ void Trace::FinishTracing() {
size_t final_offset = cur_offset_.LoadRelaxed();
- if ((flags_ & kTraceCountAllocs) != 0) {
- Runtime::Current()->SetStatsEnabled(false, true);
- }
-
std::set<mirror::ArtMethod*> visited_methods;
GetVisitedMethods(final_offset, &visited_methods);
diff --git a/test/083-compiler-regressions/expected.txt b/test/083-compiler-regressions/expected.txt
index e907fd1d58..5251c17335 100644
--- a/test/083-compiler-regressions/expected.txt
+++ b/test/083-compiler-regressions/expected.txt
@@ -1,3 +1,4 @@
+b17630605 passes
b17411468 passes
b2296099 passes
b2302318 passes
diff --git a/test/083-compiler-regressions/src/Main.java b/test/083-compiler-regressions/src/Main.java
index 8d7bf01192..8010711725 100644
--- a/test/083-compiler-regressions/src/Main.java
+++ b/test/083-compiler-regressions/src/Main.java
@@ -30,6 +30,7 @@ public class Main {
}
public static void main(String args[]) throws Exception {
+ b17630605();
b17411468();
b2296099Test();
b2302318Test();
@@ -62,6 +63,18 @@ public class Main {
minDoubleWith3ConstsTest();
}
+ public static void b17630605() {
+ // b/17630605 - failure to properly handle min long immediates.
+ long a1 = 40455547223404749L;
+ long a2 = Long.MIN_VALUE;
+ long answer = a1 + a2;
+ if (answer == -9182916489631371059L) {
+ System.out.println("b17630605 passes");
+ } else {
+ System.out.println("b17630605 fails: " + answer);
+ }
+ }
+
public static void b17411468() {
// b/17411468 - inline Math.round failure.
double d1 = 1.0;
generated by cgit v1.2.3-59-g8ed1b (git 2.39.0) at 2025-10-21 12:35:56 +0000