Work around ICE bugs with MIPS GCC and O1.
Also, work around GCC warning bugs where array accesses with explicit
bounds checks are flagged as being out-of-bounds. Significantly, clean-up
the HandleScope so the array accesses don't appear out-of-bounds at compile
time.
Change-Id: I5d66567559cc1f97cd0aa02c0df8575ebadbfe3d
diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
index 054dd46..96903db 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -1275,8 +1275,8 @@
// is at *m = sp. Will update to point to the bottom of the save frame.
//
// Note: assumes ComputeAll() has been run before.
- void LayoutCalleeSaveFrame(StackReference<mirror::ArtMethod>** m, void* sp, HandleScope** table,
- uint32_t* handle_scope_entries)
+ void LayoutCalleeSaveFrame(Thread* self, StackReference<mirror::ArtMethod>** m, void* sp,
+ HandleScope** handle_scope)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
mirror::ArtMethod* method = (*m)->AsMirrorPtr();
@@ -1289,8 +1289,6 @@
sp8 += sizeof(void*); // In the callee-save frame we use a full pointer.
// Under the callee saves put handle scope and new method stack reference.
- *handle_scope_entries = num_handle_scope_references_;
-
size_t handle_scope_size = HandleScope::SizeOf(num_handle_scope_references_);
size_t scope_and_method = handle_scope_size + sizeof(StackReference<mirror::ArtMethod>);
@@ -1300,8 +1298,8 @@
reinterpret_cast<uintptr_t>(sp8), kStackAlignment));
uint8_t* sp8_table = sp8 + sizeof(StackReference<mirror::ArtMethod>);
- *table = reinterpret_cast<HandleScope*>(sp8_table);
- (*table)->SetNumberOfReferences(num_handle_scope_references_);
+ *handle_scope = HandleScope::Create(sp8_table, self->GetTopHandleScope(),
+ num_handle_scope_references_);
// Add a slot for the method pointer, and fill it. Fix the pointer-pointer given to us.
uint8_t* method_pointer = sp8;
@@ -1319,12 +1317,12 @@
// Re-layout the callee-save frame (insert a handle-scope). Then add space for the cookie.
// Returns the new bottom. Note: this may be unaligned.
- uint8_t* LayoutJNISaveFrame(StackReference<mirror::ArtMethod>** m, void* sp, HandleScope** table,
- uint32_t* handle_scope_entries)
+ uint8_t* LayoutJNISaveFrame(Thread* self, StackReference<mirror::ArtMethod>** m, void* sp,
+ HandleScope** handle_scope)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
// First, fix up the layout of the callee-save frame.
// We have to squeeze in the HandleScope, and relocate the method pointer.
- LayoutCalleeSaveFrame(m, sp, table, handle_scope_entries);
+ LayoutCalleeSaveFrame(self, m, sp, handle_scope);
// The bottom of the callee-save frame is now where the method is, *m.
uint8_t* sp8 = reinterpret_cast<uint8_t*>(*m);
@@ -1336,14 +1334,15 @@
}
// WARNING: After this, *sp won't be pointing to the method anymore!
- uint8_t* ComputeLayout(StackReference<mirror::ArtMethod>** m, bool is_static, const char* shorty,
- uint32_t shorty_len, HandleScope** table, uint32_t* handle_scope_entries,
+ uint8_t* ComputeLayout(Thread* self, StackReference<mirror::ArtMethod>** m,
+ bool is_static, const char* shorty, uint32_t shorty_len,
+ HandleScope** handle_scope,
uintptr_t** start_stack, uintptr_t** start_gpr, uint32_t** start_fpr)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
Walk(shorty, shorty_len);
// JNI part.
- uint8_t* sp8 = LayoutJNISaveFrame(m, reinterpret_cast<void*>(*m), table, handle_scope_entries);
+ uint8_t* sp8 = LayoutJNISaveFrame(self, m, reinterpret_cast<void*>(*m), handle_scope);
sp8 = LayoutNativeCall(sp8, start_stack, start_gpr, start_fpr);
@@ -1426,20 +1425,19 @@
// of transitioning into native code.
class BuildGenericJniFrameVisitor FINAL : public QuickArgumentVisitor {
public:
- BuildGenericJniFrameVisitor(StackReference<mirror::ArtMethod>** sp, bool is_static,
- const char* shorty, uint32_t shorty_len, Thread* self)
+ BuildGenericJniFrameVisitor(Thread* self, bool is_static, const char* shorty, uint32_t shorty_len,
+ StackReference<mirror::ArtMethod>** sp)
: QuickArgumentVisitor(*sp, is_static, shorty, shorty_len),
jni_call_(nullptr, nullptr, nullptr, nullptr), sm_(&jni_call_) {
ComputeGenericJniFrameSize fsc;
uintptr_t* start_gpr_reg;
uint32_t* start_fpr_reg;
uintptr_t* start_stack_arg;
- uint32_t handle_scope_entries;
- bottom_of_used_area_ = fsc.ComputeLayout(sp, is_static, shorty, shorty_len, &handle_scope_,
- &handle_scope_entries, &start_stack_arg,
+ bottom_of_used_area_ = fsc.ComputeLayout(self, sp, is_static, shorty, shorty_len,
+ &handle_scope_,
+ &start_stack_arg,
&start_gpr_reg, &start_fpr_reg);
- handle_scope_->SetNumberOfReferences(handle_scope_entries);
jni_call_.Reset(start_gpr_reg, start_fpr_reg, start_stack_arg, handle_scope_);
// jni environment is always first argument
@@ -1611,7 +1609,7 @@
const char* shorty = called->GetShorty(&shorty_len);
// Run the visitor.
- BuildGenericJniFrameVisitor visitor(&sp, called->IsStatic(), shorty, shorty_len, self);
+ BuildGenericJniFrameVisitor visitor(self, called->IsStatic(), shorty, shorty_len, &sp);
visitor.VisitArguments();
visitor.FinalizeHandleScope(self);
diff --git a/runtime/gc/allocator/rosalloc.h b/runtime/gc/allocator/rosalloc.h
index 8374ff7..ad7f901 100644
--- a/runtime/gc/allocator/rosalloc.h
+++ b/runtime/gc/allocator/rosalloc.h
@@ -285,7 +285,7 @@
// Returns the byte size of the bracket size from the index.
static size_t IndexToBracketSize(size_t idx) {
- DCHECK(idx < kNumOfSizeBrackets);
+ DCHECK_LT(idx, kNumOfSizeBrackets);
return bracketSizes[idx];
}
// Returns the index of the size bracket from the bracket size.
diff --git a/runtime/handle_scope-inl.h b/runtime/handle_scope-inl.h
index 7bc811d..2717180 100644
--- a/runtime/handle_scope-inl.h
+++ b/runtime/handle_scope-inl.h
@@ -26,9 +26,10 @@
template<size_t kNumReferences>
inline StackHandleScope<kNumReferences>::StackHandleScope(Thread* self)
- : HandleScope(kNumReferences), self_(self), pos_(0) {
+ : HandleScope(self->GetTopHandleScope(), kNumReferences), self_(self), pos_(0) {
+ COMPILE_ASSERT(kNumReferences >= 1, stack_handle_scope_must_contain_at_least_1_reference);
// TODO: Figure out how to use a compile assert.
- DCHECK_EQ(&references_[0], &references_storage_[0]);
+ CHECK_EQ(&storage_[0], GetReferences());
for (size_t i = 0; i < kNumReferences; ++i) {
SetReference(i, nullptr);
}
diff --git a/runtime/handle_scope.h b/runtime/handle_scope.h
index 99059f9..f795e38 100644
--- a/runtime/handle_scope.h
+++ b/runtime/handle_scope.h
@@ -66,39 +66,28 @@
return link_;
}
- void SetLink(HandleScope* link) {
- DCHECK_NE(this, link);
- link_ = link;
- }
-
- // Sets the number_of_references_ field for constructing tables out of raw memory. Warning: will
- // not resize anything.
- void SetNumberOfReferences(uint32_t num_references) {
- number_of_references_ = num_references;
- }
-
- mirror::Object* GetReference(size_t i) const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
- ALWAYS_INLINE {
+ ALWAYS_INLINE mirror::Object* GetReference(size_t i) const
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
DCHECK_LT(i, number_of_references_);
- return references_[i].AsMirrorPtr();
+ return GetReferences()[i].AsMirrorPtr();
}
- Handle<mirror::Object> GetHandle(size_t i) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
- ALWAYS_INLINE {
+ ALWAYS_INLINE Handle<mirror::Object> GetHandle(size_t i)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
DCHECK_LT(i, number_of_references_);
- return Handle<mirror::Object>(&references_[i]);
+ return Handle<mirror::Object>(&GetReferences()[i]);
}
- MutableHandle<mirror::Object> GetMutableHandle(size_t i)
- SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) ALWAYS_INLINE {
+ ALWAYS_INLINE MutableHandle<mirror::Object> GetMutableHandle(size_t i)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
DCHECK_LT(i, number_of_references_);
- return MutableHandle<mirror::Object>(&references_[i]);
+ return MutableHandle<mirror::Object>(&GetReferences()[i]);
}
- void SetReference(size_t i, mirror::Object* object) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
- ALWAYS_INLINE {
+ ALWAYS_INLINE void SetReference(size_t i, mirror::Object* object)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
DCHECK_LT(i, number_of_references_);
- references_[i].Assign(object);
+ GetReferences()[i].Assign(object);
}
bool Contains(StackReference<mirror::Object>* handle_scope_entry) const {
@@ -106,39 +95,53 @@
// jni_compiler should have a jobject/jclass as a native method is
// passed in a this pointer or a class
DCHECK_GT(number_of_references_, 0U);
- return &references_[0] <= handle_scope_entry &&
- handle_scope_entry <= &references_[number_of_references_ - 1];
+ return &GetReferences()[0] <= handle_scope_entry &&
+ handle_scope_entry <= &GetReferences()[number_of_references_ - 1];
}
- // Offset of link within HandleScope, used by generated code
+ // Offset of link within HandleScope, used by generated code.
static size_t LinkOffset(size_t pointer_size) {
return 0;
}
- // Offset of length within handle scope, used by generated code
+ // Offset of length within handle scope, used by generated code.
static size_t NumberOfReferencesOffset(size_t pointer_size) {
return pointer_size;
}
- // Offset of link within handle scope, used by generated code
+ // Offset of link within handle scope, used by generated code.
static size_t ReferencesOffset(size_t pointer_size) {
return pointer_size + sizeof(number_of_references_);
}
- protected:
- explicit HandleScope(size_t number_of_references) :
- link_(nullptr), number_of_references_(number_of_references) {
+ // Placement new creation.
+ static HandleScope* Create(void* storage, HandleScope* link, uint32_t num_references)
+ WARN_UNUSED {
+ return new (storage) HandleScope(link, num_references);
}
- HandleScope* link_;
- uint32_t number_of_references_;
+ protected:
+ // Return backing storage used for references.
+ ALWAYS_INLINE StackReference<mirror::Object>* GetReferences() const {
+ uintptr_t address = reinterpret_cast<uintptr_t>(this) + ReferencesOffset(sizeof(void*));
+ return reinterpret_cast<StackReference<mirror::Object>*>(address);
+ }
- // number_of_references_ are available if this is allocated and filled in by jni_compiler.
- StackReference<mirror::Object> references_[0];
+ // Semi-hidden constructor. Construction expected by generated code and StackHandleScope.
+ explicit HandleScope(HandleScope* link, uint32_t num_references) :
+ link_(link), number_of_references_(num_references) {
+ }
+
+ // Link-list of handle scopes. The root is held by a Thread.
+ HandleScope* const link_;
+
+ // Number of handlerized references.
+ const uint32_t number_of_references_;
+
+ // Storage for references.
+ // StackReference<mirror::Object> references_[number_of_references_]
private:
- template<size_t kNumReferences> friend class StackHandleScope;
-
DISALLOW_COPY_AND_ASSIGN(HandleScope);
};
@@ -169,22 +172,22 @@
// Currently unused, using this GetReference instead of the one in HandleScope is preferred to
// avoid compiler optimizations incorrectly optimizing out of bound array accesses.
// TODO: Remove this when it is un-necessary.
- mirror::Object* GetReference(size_t i) const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
- ALWAYS_INLINE {
- DCHECK_LT(i, number_of_references_);
- return references_storage_[i].AsMirrorPtr();
+ ALWAYS_INLINE mirror::Object* GetReference(size_t i) const
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
+ DCHECK_LT(i, kNumReferences);
+ return GetReferences()[i].AsMirrorPtr();
}
- MutableHandle<mirror::Object> GetHandle(size_t i) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
- ALWAYS_INLINE {
- DCHECK_LT(i, number_of_references_);
- return MutableHandle<mirror::Object>(&references_storage_[i]);
+ ALWAYS_INLINE MutableHandle<mirror::Object> GetHandle(size_t i)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
+ DCHECK_LT(i, kNumReferences);
+ return MutableHandle<mirror::Object>(&GetReferences()[i]);
}
- void SetReference(size_t i, mirror::Object* object) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
- ALWAYS_INLINE {
- DCHECK_LT(i, number_of_references_);
- references_storage_[i].Assign(object);
+ ALWAYS_INLINE void SetReference(size_t i, mirror::Object* object)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
+ DCHECK_LT(i, kNumReferences);
+ GetReferences()[i].Assign(object);
}
template<class T>
@@ -204,9 +207,8 @@
}
private:
- // References_storage_ needs to be first so that it appears in the same location as
- // HandleScope::references_.
- StackReference<mirror::Object> references_storage_[kNumReferences];
+ // Reference storage needs to be first as expected by the HandleScope layout.
+ StackReference<mirror::Object> storage_[kNumReferences];
// The thread that the stack handle scope is a linked list upon. The stack handle scope will
// push and pop itself from this thread.
diff --git a/runtime/handle_scope_test.cc b/runtime/handle_scope_test.cc
index 7afd279..dc99987 100644
--- a/runtime/handle_scope_test.cc
+++ b/runtime/handle_scope_test.cc
@@ -25,7 +25,7 @@
template<size_t kNumReferences>
class NoThreadStackHandleScope : public HandleScope {
public:
- explicit NoThreadStackHandleScope() : HandleScope(kNumReferences) {
+ explicit NoThreadStackHandleScope(HandleScope* link) : HandleScope(link, kNumReferences) {
}
~NoThreadStackHandleScope() {
}
@@ -41,10 +41,8 @@
TEST(HandleScopeTest, Offsets) NO_THREAD_SAFETY_ANALYSIS {
// As the members of HandleScope are private, we cannot use OFFSETOF_MEMBER
// here. So do the inverse: set some data, and access it through pointers created from the offsets.
- NoThreadStackHandleScope<1> test_table;
+ NoThreadStackHandleScope<0x9ABC> test_table(reinterpret_cast<HandleScope*>(0x5678));
test_table.SetReference(0, reinterpret_cast<mirror::Object*>(0x1234));
- test_table.SetLink(reinterpret_cast<HandleScope*>(0x5678));
- test_table.SetNumberOfReferences(0x9ABC);
uint8_t* table_base_ptr = reinterpret_cast<uint8_t*>(&test_table);
diff --git a/runtime/thread.h b/runtime/thread.h
index 998e472..b0be841 100644
--- a/runtime/thread.h
+++ b/runtime/thread.h
@@ -692,7 +692,7 @@
}
void PushHandleScope(HandleScope* handle_scope) {
- handle_scope->SetLink(tlsPtr_.top_handle_scope);
+ DCHECK_EQ(handle_scope->GetLink(), tlsPtr_.top_handle_scope);
tlsPtr_.top_handle_scope = handle_scope;
}