Fix the double-OOME case again.
The key ingredient to this change is that the NULL check on the NewStringUTF
call in ClassLinker::FindClass was accidentally checking the input to the
function (which we know isn't null) rather than the output (which will be
if allocation fails).
I've also fixed a bunch of places where we were missing null checks on
allocation, but there's a lot more to do.
I've stopped -Xcheck:jni from trying to dump pending OutOfMemoryError stacks
because that's highly likely to go horribly wrong and just gets in the way
of debugging the actual problem. (We could perhaps do better, so I've added
a TODO.)
In DexFile, we were always allocating java.lang.Strings for the names of
each local variable, even if we were only parsing the debug info for the
purposes of translating a line number. This was less of a problem in Dalvik
where the names were just const char*s, but since we go straight to Strings,
this was causing yet more OOME carnage (and is a lot of wasted effort).
This fixes the regression we were seeing in 061.
Change-Id: I6241602756db3070b214ccb35e3760b37e1e3d10
diff --git a/src/check_jni.cc b/src/check_jni.cc
index 26068fd..fa91100 100644
--- a/src/check_jni.cc
+++ b/src/check_jni.cc
@@ -726,10 +726,13 @@
* make any JNI calls other than the Exception* methods.
*/
if ((flags & kFlag_ExcepOkay) == 0 && self->IsExceptionPending()) {
- LOG(ERROR) << "JNI ERROR: JNI " << function_name_ << " called with "
- << PrettyTypeOf(self->GetException()) << " pending";
- LOG(ERROR) << "Pending exception is:";
- LOG(ERROR) << jniGetStackTrace(env_);
+ std::string type(PrettyTypeOf(self->GetException()));
+ LOG(ERROR) << "JNI ERROR: JNI " << function_name_ << " called with " << type << " pending";
+ // TODO: write native code that doesn't require allocation for dumping an exception.
+ if (type != "java.lang.OutOfMemoryError") {
+ LOG(ERROR) << "Pending exception is: ";
+ LOG(ERROR) << jniGetStackTrace(env_);
+ }
JniAbort();
return;
}
diff --git a/src/class_linker.cc b/src/class_linker.cc
index 99b5b0e..f610f31 100644
--- a/src/class_linker.cc
+++ b/src/class_linker.cc
@@ -224,9 +224,8 @@
CHECK(!init_done_);
- // java_lang_Class comes first, its needed for AllocClass
- Class* java_lang_Class = down_cast<Class*>(
- Heap::AllocObject(NULL, sizeof(ClassClass)));
+ // java_lang_Class comes first, it's needed for AllocClass
+ Class* java_lang_Class = down_cast<Class*>(Heap::AllocObject(NULL, sizeof(ClassClass)));
CHECK(java_lang_Class != NULL);
java_lang_Class->SetClass(java_lang_Class);
java_lang_Class->SetClassSize(sizeof(ClassClass));
@@ -272,6 +271,7 @@
// Create storage for root classes, save away our work so far (requires
// descriptors)
class_roots_ = ObjectArray<Class>::Alloc(object_array_class, kClassRootsMax);
+ CHECK(class_roots_ != NULL);
SetClassRoot(kJavaLangClass, java_lang_Class);
SetClassRoot(kJavaLangObject, java_lang_Object);
SetClassRoot(kClassArrayClass, class_array_class);
@@ -750,8 +750,16 @@
}
DexCache* ClassLinker::AllocDexCache(const DexFile& dex_file) {
+ String* location = intern_table_->InternStrong(dex_file.GetLocation().c_str());
+ if (location == NULL) {
+ return NULL;
+ }
DexCache* dex_cache = down_cast<DexCache*>(AllocObjectArray<Object>(DexCache::LengthAsArray()));
- dex_cache->Init(intern_table_->InternStrong(dex_file.GetLocation().c_str()),
+ if (dex_cache == NULL) {
+ return NULL;
+ }
+ // TODO: lots of missing null checks hidden in this call...
+ dex_cache->Init(location,
AllocObjectArray<String>(dex_file.NumStringIds()),
AllocClassArray(dex_file.NumTypeIds()),
AllocObjectArray<Method>(dex_file.NumMethodIds()),
@@ -875,7 +883,7 @@
jmethodID mid = env->GetMethodID(c.get(), "loadClass", "(Ljava/lang/String;)Ljava/lang/Class;");
CHECK(mid != NULL);
ScopedLocalRef<jobject> class_name_object(env, env->NewStringUTF(class_name_string.c_str()));
- if (class_name_string == NULL) {
+ if (class_name_object.get() == NULL) {
return NULL;
}
ScopedLocalRef<jobject> class_loader_object(env, AddLocalReference<jobject>(env, class_loader));
@@ -1030,6 +1038,9 @@
DCHECK(klass->GetDescriptor()->Equals(descriptor));
} else {
klass->SetDescriptor(intern_table_->InternStrong(descriptor));
+ if (klass->GetDescriptor() == NULL) {
+ return;
+ }
}
uint32_t access_flags = dex_class_def.access_flags_;
// Make sure that none of our runtime-only flags are set.
@@ -1048,7 +1059,11 @@
const char* source_file = dex_file.dexGetSourceFile(dex_class_def);
if (source_file != NULL) {
- klass->SetSourceFile(intern_table_->InternStrong(source_file));
+ String* source_file_string = intern_table_->InternStrong(source_file);
+ if (source_file_string == NULL) {
+ return;
+ }
+ klass->SetSourceFile(source_file_string);
}
// Load class interfaces.
@@ -1174,6 +1189,9 @@
dst->SetDeclaringClass(klass);
String* method_name = ResolveString(dex_file, method_id.name_idx_, klass->GetDexCache());
+ if (method_name == NULL) {
+ return;
+ }
dst->SetName(method_name);
if (method_name->Equals("<init>")) {
dst->SetClass(GetClassRoot(kJavaLangReflectConstructor));
@@ -1181,7 +1199,11 @@
int32_t utf16_length;
std::string signature(dex_file.CreateMethodDescriptor(method_id.proto_idx_, &utf16_length));
- dst->SetSignature(intern_table_->InternStrong(utf16_length, signature.c_str()));
+ String* signature_string = intern_table_->InternStrong(utf16_length, signature.c_str());
+ if (signature_string == NULL) {
+ return;
+ }
+ dst->SetSignature(signature_string);
if (method_name->Equals("finalize") && signature == "()V") {
/*
@@ -1201,7 +1223,11 @@
dst->SetProtoIdx(method_id.proto_idx_);
dst->SetCodeItemOffset(src.code_off_);
const char* shorty = dex_file.GetShorty(method_id.proto_idx_);
- dst->SetShorty(intern_table_->InternStrong(shorty));
+ String* shorty_string = intern_table_->InternStrong(shorty);
+ dst->SetShorty(shorty_string);
+ if (shorty_string == NULL) {
+ return;
+ }
dst->SetAccessFlags(src.access_flags_);
dst->SetReturnTypeIdx(dex_file.GetProtoId(method_id.proto_idx_).return_type_idx_);
@@ -1304,6 +1330,7 @@
CHECK(primitive_class != NULL);
primitive_class->SetAccessFlags(kAccPublic | kAccFinal | kAccAbstract);
primitive_class->SetDescriptor(intern_table_->InternStrong(descriptor));
+ CHECK(primitive_class->GetDescriptor() != NULL);
primitive_class->SetPrimitiveType(type);
primitive_class->SetStatus(Class::kStatusInitialized);
bool success = InsertClass(descriptor, primitive_class);
@@ -1393,6 +1420,9 @@
DCHECK(new_class->GetDescriptor()->Equals(descriptor));
} else {
new_class->SetDescriptor(intern_table_->InternStrong(descriptor.c_str()));
+ if (new_class->GetDescriptor() == NULL) {
+ return NULL;
+ }
}
Class* java_lang_Object = GetClassRoot(kJavaLangObject);
new_class->SetSuperClass(java_lang_Object);
diff --git a/src/dex_file.cc b/src/dex_file.cc
index fa6d9d5..235c734 100644
--- a/src/dex_file.cc
+++ b/src/dex_file.cc
@@ -627,32 +627,37 @@
uint32_t parameters_size = DecodeUnsignedLeb128(&stream);
uint16_t arg_reg = code_item->registers_size_ - code_item->ins_size_;
uint32_t address = 0;
+ bool need_locals = (local_cb != NULL);
if (!method->IsStatic()) {
- local_in_reg[arg_reg].name_ = String::AllocFromModifiedUtf8("this");
- local_in_reg[arg_reg].descriptor_ = method->GetDeclaringClass()->GetDescriptor();
- local_in_reg[arg_reg].signature_ = NULL;
- local_in_reg[arg_reg].start_address_ = 0;
- local_in_reg[arg_reg].is_live_ = true;
+ if (need_locals) {
+ local_in_reg[arg_reg].name_ = String::AllocFromModifiedUtf8("this");
+ local_in_reg[arg_reg].descriptor_ = method->GetDeclaringClass()->GetDescriptor();
+ local_in_reg[arg_reg].signature_ = NULL;
+ local_in_reg[arg_reg].start_address_ = 0;
+ local_in_reg[arg_reg].is_live_ = true;
+ }
arg_reg++;
}
- ParameterIterator *it = GetParameterIterator(GetProtoId(method->GetProtoIdx()));
+ ParameterIterator* it = GetParameterIterator(GetProtoId(method->GetProtoIdx()));
for (uint32_t i = 0; i < parameters_size && it->HasNext(); ++i, it->Next()) {
if (arg_reg >= code_item->registers_size_) {
LOG(FATAL) << "invalid stream";
return;
}
-
- String* descriptor = String::AllocFromModifiedUtf8(it->GetDescriptor());
- String* name = dexArtStringById(DecodeUnsignedLeb128P1(&stream));
-
- local_in_reg[arg_reg].name_ = name;
- local_in_reg[arg_reg].descriptor_ = descriptor;
- local_in_reg[arg_reg].signature_ = NULL;
- local_in_reg[arg_reg].start_address_ = address;
- local_in_reg[arg_reg].is_live_ = true;
- switch (descriptor->CharAt(0)) {
+ int32_t id = DecodeUnsignedLeb128P1(&stream);
+ const char* descriptor_utf8 = it->GetDescriptor();
+ if (need_locals) {
+ String* descriptor = String::AllocFromModifiedUtf8(descriptor_utf8);
+ String* name = dexArtStringById(id);
+ local_in_reg[arg_reg].name_ = name;
+ local_in_reg[arg_reg].descriptor_ = descriptor;
+ local_in_reg[arg_reg].signature_ = NULL;
+ local_in_reg[arg_reg].start_address_ = address;
+ local_in_reg[arg_reg].is_live_ = true;
+ }
+ switch (*descriptor_utf8) {
case 'D':
case 'J':
arg_reg += 2;
@@ -695,17 +700,19 @@
}
// Emit what was previously there, if anything
- InvokeLocalCbIfLive(cnxt, reg, address, local_in_reg, local_cb);
+ if (need_locals) {
+ InvokeLocalCbIfLive(cnxt, reg, address, local_in_reg, local_cb);
- local_in_reg[reg].name_ = dexArtStringById(DecodeUnsignedLeb128P1(&stream));
- local_in_reg[reg].descriptor_ = dexArtStringByTypeIdx(DecodeUnsignedLeb128P1(&stream));
- if (opcode == DBG_START_LOCAL_EXTENDED) {
- local_in_reg[reg].signature_ = dexArtStringById(DecodeUnsignedLeb128P1(&stream));
- } else {
- local_in_reg[reg].signature_ = NULL;
+ local_in_reg[reg].name_ = dexArtStringById(DecodeUnsignedLeb128P1(&stream));
+ local_in_reg[reg].descriptor_ = dexArtStringByTypeIdx(DecodeUnsignedLeb128P1(&stream));
+ if (opcode == DBG_START_LOCAL_EXTENDED) {
+ local_in_reg[reg].signature_ = dexArtStringById(DecodeUnsignedLeb128P1(&stream));
+ } else {
+ local_in_reg[reg].signature_ = NULL;
+ }
+ local_in_reg[reg].start_address_ = address;
+ local_in_reg[reg].is_live_ = true;
}
- local_in_reg[reg].start_address_ = address;
- local_in_reg[reg].is_live_ = true;
break;
case DBG_END_LOCAL:
@@ -715,8 +722,10 @@
return;
}
- InvokeLocalCbIfLive(cnxt, reg, address, local_in_reg, local_cb);
- local_in_reg[reg].is_live_ = false;
+ if (need_locals) {
+ InvokeLocalCbIfLive(cnxt, reg, address, local_in_reg, local_cb);
+ local_in_reg[reg].is_live_ = false;
+ }
break;
case DBG_RESTART_LOCAL:
@@ -726,17 +735,18 @@
return;
}
- if (local_in_reg[reg].name_ == NULL
- || local_in_reg[reg].descriptor_ == NULL) {
- LOG(FATAL) << "invalid stream";
- return;
- }
+ if (need_locals) {
+ if (local_in_reg[reg].name_ == NULL || local_in_reg[reg].descriptor_ == NULL) {
+ LOG(FATAL) << "invalid stream";
+ return;
+ }
- // If the register is live, the "restart" is superfluous,
- // and we don't want to mess with the existing start address.
- if (!local_in_reg[reg].is_live_) {
- local_in_reg[reg].start_address_ = address;
- local_in_reg[reg].is_live_ = true;
+ // If the register is live, the "restart" is superfluous,
+ // and we don't want to mess with the existing start address.
+ if (!local_in_reg[reg].is_live_) {
+ local_in_reg[reg].start_address_ = address;
+ local_in_reg[reg].is_live_ = true;
+ }
}
break;
diff --git a/src/dex_file.h b/src/dex_file.h
index 468ce4d..77189f8 100644
--- a/src/dex_file.h
+++ b/src/dex_file.h
@@ -801,7 +801,7 @@
DexDebugNewPositionCb posCb, DexDebugNewLocalCb local_cb,
void* cnxt, const byte* stream, LocalInfo* local_in_reg) const;
- void dexDecodeDebugInfo(const CodeItem* code_item, const art::Method *method,
+ void dexDecodeDebugInfo(const CodeItem* code_item, const art::Method* method,
DexDebugNewPositionCb posCb, DexDebugNewLocalCb local_cb,
void* cnxt) const {
const byte* stream = dexGetDebugInfoStream(code_item);
diff --git a/src/jni_internal.cc b/src/jni_internal.cc
index 9c7a63d..7e75dff 100644
--- a/src/jni_internal.cc
+++ b/src/jni_internal.cc
@@ -909,6 +909,9 @@
return NULL;
}
Object* result = c->AllocObject();
+ if (result == NULL) {
+ return NULL;
+ }
jobject local_result = AddLocalReference<jobject>(env, result);
CallNonvirtualVoidMethodV(env, local_result, java_class, mid, args);
return local_result;
@@ -921,6 +924,9 @@
return NULL;
}
Object* result = c->AllocObject();
+ if (result == NULL) {
+ return NULL;
+ }
jobject local_result = AddLocalReference<jobjectArray>(env, result);
CallNonvirtualVoidMethodA(env, local_result, java_class, mid, args);
return local_result;
diff --git a/src/runtime.cc b/src/runtime.cc
index 946c4cf..dc9d6e7 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -704,7 +704,9 @@
name = "$$$refs_and_args_callee_save_method$$$";
}
method->SetName(intern_table_->InternStrong(name));
+ CHECK(method->GetName() != NULL);
method->SetSignature(intern_table_->InternStrong("()V"));
+ CHECK(method->GetSignature() != NULL);
method->SetCode(NULL);
if ((insns == kThumb2) || (insns == kArm)) {
uint32_t ref_spills = (1 << art::arm::R5) | (1 << art::arm::R6) | (1 << art::arm::R7) |
diff --git a/src/thread.cc b/src/thread.cc
index 3c19fe4..62b3290 100644
--- a/src/thread.cc
+++ b/src/thread.cc
@@ -1076,6 +1076,9 @@
} else {
// Create java_trace array and place in local reference table
java_traces = class_linker->AllocStackTraceElementArray(depth);
+ if (java_traces == NULL) {
+ return NULL;
+ }
result = AddLocalReference<jobjectArray>(ts.Env(), java_traces);
}
@@ -1096,8 +1099,10 @@
StackTraceElement::Alloc(String::AllocFromModifiedUtf8(class_name.c_str()),
method->GetName(),
klass->GetSourceFile(),
- dex_file.GetLineNumFromPC(method,
- method->ToDexPC(native_pc)));
+ dex_file.GetLineNumFromPC(method, method->ToDexPC(native_pc)));
+ if (obj == NULL) {
+ return NULL;
+ }
#ifdef MOVING_GARBAGE_COLLECTOR
// Re-read after potential GC
java_traces = Decode<ObjectArray<Object>*>(ts.Env(), result);
@@ -1131,14 +1136,25 @@
JNIEnv* env = GetJniEnv();
ScopedLocalRef<jclass> exception_class(env, env->FindClass(descriptor.c_str()));
- CHECK(exception_class.get() != NULL) << "descriptor=\"" << descriptor << "\"";
+ if (exception_class.get() == NULL) {
+ LOG(ERROR) << "Couldn't throw new " << descriptor << " because JNI FindClass failed: "
+ << PrettyTypeOf(GetException());
+ CHECK(IsExceptionPending());
+ return;
+ }
int rc = env->ThrowNew(exception_class.get(), msg);
- CHECK_EQ(rc, JNI_OK);
+ if (rc != JNI_OK) {
+ LOG(ERROR) << "Couldn't throw new " << descriptor << " because JNI ThrowNew failed: "
+ << PrettyTypeOf(GetException());
+ CHECK(IsExceptionPending());
+ return;
+ }
}
void Thread::ThrowOutOfMemoryError(Class* c, size_t byte_count) {
- LOG(ERROR) << "Failed to allocate a " << PrettyDescriptor(c->GetDescriptor())
- << " (" << byte_count << " bytes)" << (throwing_OutOfMemoryError_ ? " recursive case" : "");
+ LOG(ERROR) << "Failed to allocate a " << byte_count << "-byte "
+ << PrettyDescriptor(c->GetDescriptor())
+ << (throwing_OutOfMemoryError_ ? " (recursive case)" : "");
if (!throwing_OutOfMemoryError_) {
throwing_OutOfMemoryError_ = true;
ThrowNewException("Ljava/lang/OutOfMemoryError;", NULL);
