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/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;