Merge "Workaround for gcc volatile struct member bug"
diff --git a/runtime/thread-inl.h b/runtime/thread-inl.h
index 8449607..e47fd37 100644
--- a/runtime/thread-inl.h
+++ b/runtime/thread-inl.h
@@ -50,7 +50,8 @@
// old_state_and_flags.suspend_request is true.
DCHECK_NE(new_state, kRunnable);
DCHECK_EQ(this, Thread::Current());
- union StateAndFlags old_state_and_flags = state_and_flags_;
+ union StateAndFlags old_state_and_flags;
+ old_state_and_flags.as_int = state_and_flags_.as_int;
state_and_flags_.as_struct.state = new_state;
return static_cast<ThreadState>(old_state_and_flags.as_struct.state);
}
@@ -87,7 +88,7 @@
union StateAndFlags old_state_and_flags;
union StateAndFlags new_state_and_flags;
do {
- old_state_and_flags = state_and_flags_;
+ old_state_and_flags.as_int = state_and_flags_.as_int;
if (UNLIKELY((old_state_and_flags.as_struct.flags & kCheckpointRequest) != 0)) {
RunCheckpointFunction();
continue;
@@ -104,22 +105,23 @@
inline ThreadState Thread::TransitionFromSuspendedToRunnable() {
bool done = false;
- union StateAndFlags old_state_and_flags = state_and_flags_;
+ union StateAndFlags old_state_and_flags;
+ old_state_and_flags.as_int = state_and_flags_.as_int;
int16_t old_state = old_state_and_flags.as_struct.state;
DCHECK_NE(static_cast<ThreadState>(old_state), kRunnable);
do {
Locks::mutator_lock_->AssertNotHeld(this); // Otherwise we starve GC..
- old_state_and_flags = state_and_flags_;
+ old_state_and_flags.as_int = state_and_flags_.as_int;
DCHECK_EQ(old_state_and_flags.as_struct.state, old_state);
if (UNLIKELY((old_state_and_flags.as_struct.flags & kSuspendRequest) != 0)) {
// Wait while our suspend count is non-zero.
MutexLock mu(this, *Locks::thread_suspend_count_lock_);
- old_state_and_flags = state_and_flags_;
+ old_state_and_flags.as_int = state_and_flags_.as_int;
DCHECK_EQ(old_state_and_flags.as_struct.state, old_state);
while ((old_state_and_flags.as_struct.flags & kSuspendRequest) != 0) {
// Re-check when Thread::resume_cond_ is notified.
Thread::resume_cond_->Wait(this);
- old_state_and_flags = state_and_flags_;
+ old_state_and_flags.as_int = state_and_flags_.as_int;
DCHECK_EQ(old_state_and_flags.as_struct.state, old_state);
}
DCHECK_EQ(GetSuspendCount(), 0);
@@ -127,10 +129,11 @@
// Re-acquire shared mutator_lock_ access.
Locks::mutator_lock_->SharedLock(this);
// Atomically change from suspended to runnable if no suspend request pending.
- old_state_and_flags = state_and_flags_;
+ old_state_and_flags.as_int = state_and_flags_.as_int;
DCHECK_EQ(old_state_and_flags.as_struct.state, old_state);
if (LIKELY((old_state_and_flags.as_struct.flags & kSuspendRequest) == 0)) {
- union StateAndFlags new_state_and_flags = old_state_and_flags;
+ union StateAndFlags new_state_and_flags;
+ new_state_and_flags.as_int = old_state_and_flags.as_int;
new_state_and_flags.as_struct.state = kRunnable;
// CAS the value without a memory barrier, that occurred in the lock above.
done = android_atomic_cas(old_state_and_flags.as_int, new_state_and_flags.as_int,
diff --git a/runtime/thread.cc b/runtime/thread.cc
index 297fa45..fa49faa 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -579,7 +579,8 @@
}
bool Thread::RequestCheckpoint(Closure* function) {
- union StateAndFlags old_state_and_flags = state_and_flags_;
+ union StateAndFlags old_state_and_flags;
+ old_state_and_flags.as_int = state_and_flags_.as_int;
if (old_state_and_flags.as_struct.state != kRunnable) {
return false; // Fail, thread is suspended and so can't run a checkpoint.
}
@@ -591,7 +592,8 @@
// Checkpoint function installed now install flag bit.
// We must be runnable to request a checkpoint.
DCHECK_EQ(old_state_and_flags.as_struct.state, kRunnable);
- union StateAndFlags new_state_and_flags = old_state_and_flags;
+ union StateAndFlags new_state_and_flags;
+ new_state_and_flags.as_int = old_state_and_flags.as_int;
new_state_and_flags.as_struct.flags |= kCheckpointRequest;
int succeeded = android_atomic_cmpxchg(old_state_and_flags.as_int, new_state_and_flags.as_int,
&state_and_flags_.as_int);
diff --git a/runtime/thread.h b/runtime/thread.h
index db2f7b4..44b2186 100644
--- a/runtime/thread.h
+++ b/runtime/thread.h
@@ -147,7 +147,8 @@
}
bool IsSuspended() const {
- union StateAndFlags state_and_flags = state_and_flags_;
+ union StateAndFlags state_and_flags;
+ state_and_flags.as_int = state_and_flags_.as_int;
return state_and_flags.as_struct.state != kRunnable &&
(state_and_flags.as_struct.flags & kSuspendRequest) != 0;
}
@@ -638,7 +639,8 @@
// 32 bits of atomically changed state and flags. Keeping as 32 bits allows and atomic CAS to
// change from being Suspended to Runnable without a suspend request occurring.
- union StateAndFlags {
+ union PACKED(4) StateAndFlags {
+ StateAndFlags() {}
struct PACKED(4) {
// Bitfield of flag values. Must be changed atomically so that flag values aren't lost. See
// ThreadFlags for bit field meanings.
@@ -650,6 +652,11 @@
volatile uint16_t state;
} as_struct;
volatile int32_t as_int;
+
+ private:
+ // gcc does not handle struct with volatile member assignments correctly.
+ // See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47409
+ DISALLOW_COPY_AND_ASSIGN(StateAndFlags);
};
union StateAndFlags state_and_flags_;
COMPILE_ASSERT(sizeof(union StateAndFlags) == sizeof(int32_t),