Several fixes for proper creation and use of vmap tables
Change-Id: I7696115af4263df18ede0777ae14de7a3a7ada3b
diff --git a/src/class_linker.cc b/src/class_linker.cc
index 64d4025..0232ee3 100644
--- a/src/class_linker.cc
+++ b/src/class_linker.cc
@@ -214,7 +214,9 @@
Split(class_path, ':', parsed);
for (size_t i = 0; i < parsed.size(); ++i) {
const DexFile* dex_file = DexFile::Open(parsed[i], Runtime::Current()->GetHostPrefix());
- if (dex_file != NULL) {
+ if (dex_file == NULL) {
+ LOG(WARNING) << "Failed to open dex file " << parsed[i];
+ } else {
class_path_vector.push_back(dex_file);
}
}
@@ -223,7 +225,7 @@
void ClassLinker::Init(const std::string& boot_class_path) {
const Runtime* runtime = Runtime::Current();
if (runtime->IsVerboseStartup()) {
- LOG(INFO) << "ClassLinker::InitFrom entering";
+ LOG(INFO) << "ClassLinker::InitFrom entering boot_class_path=" << boot_class_path;
}
CHECK(!init_done_);
@@ -310,6 +312,7 @@
// use AllocObjectArray to create DexCache instances
std::vector<const DexFile*> boot_class_path_vector;
CreateClassPath(boot_class_path, boot_class_path_vector);
+ CHECK_NE(0U, boot_class_path_vector.size());
for (size_t i = 0; i != boot_class_path_vector.size(); ++i) {
const DexFile* dex_file = boot_class_path_vector[i];
CHECK(dex_file != NULL);
diff --git a/src/compiled_method.cc b/src/compiled_method.cc
index 2846ab3..082600f 100644
--- a/src/compiled_method.cc
+++ b/src/compiled_method.cc
@@ -13,6 +13,7 @@
std::vector<uint32_t>& mapping_table,
std::vector<uint16_t>& vmap_table) {
CHECK_NE(short_code.size(), 0U);
+ CHECK_GE(vmap_table.size(), 1U); // should always contain an entry for LR
size_t code_byte_count = short_code.size() * sizeof(short_code[0]);
std::vector<uint8_t> byte_code(code_byte_count);
@@ -31,6 +32,7 @@
vmap_table.begin(),
vmap_table.end());
DCHECK_EQ(vmap_table.size() + 1, length_prefixed_vmap_table.size());
+ DCHECK_EQ(vmap_table.size(), length_prefixed_vmap_table[0]);
instruction_set_ = instruction_set;
code_ = byte_code;
@@ -40,6 +42,8 @@
fp_spill_mask_ = fp_spill_mask;
mapping_table_ = length_prefixed_mapping_table;
vmap_table_ = length_prefixed_vmap_table;
+
+ DCHECK_EQ(vmap_table_[0], static_cast<uint32_t>(__builtin_popcount(core_spill_mask) + __builtin_popcount(fp_spill_mask)));
}
CompiledMethod::CompiledMethod(InstructionSet instruction_set,
diff --git a/src/compiler/Frontend.cc b/src/compiler/Frontend.cc
index 95bff2b..a0e7713 100644
--- a/src/compiler/Frontend.cc
+++ b/src/compiler/Frontend.cc
@@ -895,20 +895,19 @@
vmapTable.push_back(cUnit->coreVmapTable[i]);
}
// Add a marker to take place of lr
- cUnit->coreVmapTable.push_back(INVALID_VREG);
+ vmapTable.push_back(INVALID_VREG);
// Combine vmap tables - core regs, then fp regs
for (uint32_t i = 0; i < cUnit->fpVmapTable.size(); i++) {
- cUnit->coreVmapTable.push_back(cUnit->fpVmapTable[i]);
- }
- DCHECK(cUnit->coreVmapTable.size() == (uint32_t)
- (__builtin_popcount(cUnit->coreSpillMask) + __builtin_popcount(cUnit->fpSpillMask)));
- vmapTable.push_back(-1);
- for (size_t i = 0; i < cUnit->fpVmapTable.size(); i++) {
vmapTable.push_back(cUnit->fpVmapTable[i]);
}
+ DCHECK_EQ(vmapTable.size(),
+ static_cast<uint32_t>(__builtin_popcount(cUnit->coreSpillMask)
+ + __builtin_popcount(cUnit->fpSpillMask)));
+ DCHECK_GE(vmapTable.size(), 1U); // should always at least one INVALID_VREG for lr
+
CompiledMethod* result = new CompiledMethod(art::kThumb2,
cUnit->codeBuffer, cUnit->frameSize, cUnit->frameSize - sizeof(intptr_t),
- cUnit->coreSpillMask, cUnit->fpSpillMask, cUnit->mappingTable, cUnit->coreVmapTable);
+ cUnit->coreSpillMask, cUnit->fpSpillMask, cUnit->mappingTable, vmapTable);
if (compiler.IsVerbose()) {
LOG(INFO) << "Compiled " << PrettyMethod(method)
diff --git a/src/dex_file.cc b/src/dex_file.cc
index fea8009..c9e2f62 100644
--- a/src/dex_file.cc
+++ b/src/dex_file.cc
@@ -60,12 +60,11 @@
const std::string& strip_location_prefix) {
if (IsValidZipFilename(filename)) {
return DexFile::OpenZip(filename, strip_location_prefix);
- } else {
- if (!IsValidDexFilename(filename)) {
- LOG(WARNING) << "Attempting to open dex file with unknown extension '" << filename << "'";
- }
- return DexFile::OpenFile(filename, filename, strip_location_prefix);
}
+ if (!IsValidDexFilename(filename)) {
+ LOG(WARNING) << "Attempting to open dex file with unknown extension '" << filename << "'";
+ }
+ return DexFile::OpenFile(filename, filename, strip_location_prefix);
}
void DexFile::ChangePermissions(int prot) const {
@@ -243,6 +242,7 @@
UniquePtr<LockedFd> fd(LockedFd::CreateAndLock(cache_path_tmp, 0644));
state_changer.reset(NULL);
if (fd.get() == NULL) {
+ PLOG(ERROR) << "Failed to lock file '" << cache_path_tmp << "'";
return NULL;
}
@@ -270,10 +270,12 @@
TmpFile tmp_file(cache_path_tmp);
UniquePtr<File> file(OS::FileFromFd(cache_path_tmp.c_str(), fd->GetFd()));
if (file.get() == NULL) {
+ LOG(ERROR) << "Failed to create file for '" << cache_path_tmp << "'";
return NULL;
}
bool success = zip_entry->Extract(*file);
if (!success) {
+ LOG(ERROR) << "Failed to extract classes.dex to '" << cache_path_tmp << "'";
return NULL;
}
@@ -282,11 +284,13 @@
// Compute checksum and compare to zip. If things look okay, rename from tmp.
off_t lseek_result = lseek(fd->GetFd(), 0, SEEK_SET);
if (lseek_result == -1) {
+ PLOG(ERROR) << "Failed to seek to start of '" << cache_path_tmp << "'";
return NULL;
}
const size_t kBufSize = 32768;
UniquePtr<uint8_t[]> buf(new uint8_t[kBufSize]);
if (buf.get() == NULL) {
+ LOG(ERROR) << "Failed to allocate buffer to checksum '" << cache_path_tmp << "'";
return NULL;
}
uint32_t computed_crc = crc32(0L, Z_NULL, 0);
@@ -302,6 +306,7 @@
computed_crc = crc32(computed_crc, buf.get(), bytes_read);
}
if (computed_crc != zip_entry->GetCrc32()) {
+ LOG(ERROR) << "Failed to validate checksum for '" << cache_path_tmp << "'";
return NULL;
}
int rename_result = rename(cache_path_tmp.c_str(), cache_path.c_str());
diff --git a/src/oat_file.cc b/src/oat_file.cc
index 1a99820..8af22b0 100644
--- a/src/oat_file.cc
+++ b/src/oat_file.cc
@@ -190,15 +190,28 @@
const uint32_t fp_spill_mask,
const uint32_t* mapping_table,
const uint16_t* vmap_table,
- const Method::InvokeStub* invoke_stub) :
- code_(code),
- frame_size_in_bytes_(frame_size_in_bytes),
- return_pc_offset_in_bytes_(return_pc_offset_in_bytes),
- core_spill_mask_(core_spill_mask),
- fp_spill_mask_(fp_spill_mask),
- mapping_table_(mapping_table),
- vmap_table_(vmap_table),
- invoke_stub_(invoke_stub) {}
+ const Method::InvokeStub* invoke_stub)
+ : code_(code),
+ frame_size_in_bytes_(frame_size_in_bytes),
+ return_pc_offset_in_bytes_(return_pc_offset_in_bytes),
+ core_spill_mask_(core_spill_mask),
+ fp_spill_mask_(fp_spill_mask),
+ mapping_table_(mapping_table),
+ vmap_table_(vmap_table),
+ invoke_stub_(invoke_stub) {
+
+#ifndef NDEBUG
+ if (mapping_table != NULL) { // implies non-native, non-stub code
+ if (vmap_table_ == NULL) {
+ DCHECK_EQ(0U, static_cast<uint32_t>(__builtin_popcount(core_spill_mask_) + __builtin_popcount(fp_spill_mask_)));
+ } else {
+ DCHECK_EQ(vmap_table_[0], static_cast<uint32_t>(__builtin_popcount(core_spill_mask_) + __builtin_popcount(fp_spill_mask_)));
+ }
+ } else {
+ DCHECK(vmap_table_ == NULL);
+ }
+#endif
+}
OatFile::OatMethod::~OatMethod() {}
diff --git a/src/oatdump.cc b/src/oatdump.cc
index 02aecfc..61f352e 100644
--- a/src/oatdump.cc
+++ b/src/oatdump.cc
@@ -84,10 +84,10 @@
os << StringPrintf("%08x\n\n", oat_header.GetExecutableOffset());
os << "BASE:\n";
- os << oat_file.GetBase() << "\n\n";
+ os << reinterpret_cast<const void*>(oat_file.GetBase()) << "\n\n";
os << "LIMIT:\n";
- os << oat_file.GetLimit() << "\n\n";
+ os << reinterpret_cast<const void*>(oat_file.GetLimit()) << "\n\n";
os << std::flush;
@@ -121,7 +121,7 @@
for (size_t class_def_index = 0; class_def_index < dex_file->NumClassDefs(); class_def_index++) {
const DexFile::ClassDef& class_def = dex_file->GetClassDef(class_def_index);
const char* descriptor = dex_file->GetClassDescriptor(class_def);
- os << StringPrintf("%d: %s (type_ide=%d)\n", class_def_index, descriptor, class_def.class_idx_);
+ os << StringPrintf("%d: %s (type_idx=%d)\n", class_def_index, descriptor, class_def.class_idx_);
UniquePtr<const OatFile::OatClass> oat_class(oat_dex_file.GetOatClass(class_def_index));
CHECK(oat_class.get() != NULL);
DumpOatClass(os, oat_file, *oat_class.get(), *dex_file, class_def);
diff --git a/src/thread.cc b/src/thread.cc
index b8dbd6b..e45da0d 100644
--- a/src/thread.cc
+++ b/src/thread.cc
@@ -1359,7 +1359,8 @@
Method* m = frame.GetMethod();
if (false) {
LOG(INFO) << "Visiting stack roots in " << PrettyMethod(m, false)
- << "@ PC: " << m->ToDexPC(pc);
+ << StringPrintf("@ PC:%04x", m->ToDexPC(pc));
+
}
// Process register map (which native and callee save methods don't have)
if (!m->IsNative() && !m->IsCalleeSaveMethod()) {
@@ -1367,8 +1368,9 @@
const uint8_t* reg_bitmap = map.FindBitMap(m->ToDexPC(pc));
CHECK(reg_bitmap != NULL);
const uint16_t* vmap = m->GetVmapTable();
- // For all dex registers
- for (int reg = 0; reg < m->NumRegisters(); ++reg) {
+ // For all dex registers in the bitmap
+ size_t num_regs = std::min(map.RegWidth() * 8, static_cast<size_t>(m->NumRegisters()));
+ for (size_t reg = 0; reg < num_regs; ++reg) {
// Does this register hold a reference?
if (TestBitmap(reg, reg_bitmap)) {
// Is the reference in the context or on the stack?
@@ -1376,6 +1378,10 @@
uint32_t vmap_offset = 0xEBAD0FF5;
// TODO: take advantage of the registers being ordered
for (int i = 0; i < m->GetVmapTableLength(); i++) {
+ // stop if we find what we are are looking for or the INVALID_VREG that marks lr
+ if (vmap[i] == 0xffff) {
+ break;
+ }
if (vmap[i] == reg) {
in_context = true;
vmap_offset = i;
@@ -1386,10 +1392,11 @@
if (in_context) {
// Compute the register we need to load from the context
uint32_t spill_mask = m->GetCoreSpillMask();
+ CHECK_LT(vmap_offset, static_cast<uint32_t>(__builtin_popcount(spill_mask)));
uint32_t matches = 0;
uint32_t spill_shifts = 0;
while (matches != (vmap_offset + 1)) {
- CHECK_NE(spill_mask, 0u);
+ DCHECK_NE(spill_mask, 0u);
matches += spill_mask & 1; // Add 1 if the low bit is set
spill_mask >>= 1;
spill_shifts++;
diff --git a/src/zip_archive.cc b/src/zip_archive.cc
index 550d04a..69bc85b 100644
--- a/src/zip_archive.cc
+++ b/src/zip_archive.cc
@@ -126,9 +126,11 @@
size_t bytes_to_read = (count > kBufSize) ? kBufSize : count;
ssize_t actual = TEMP_FAILURE_RETRY(read(in, &buf[0], bytes_to_read));
if (actual != static_cast<ssize_t>(bytes_to_read)) {
+ PLOG(WARNING) << "Zip: short read writing to file " << file.name();
return false;
}
if (!file.WriteFully(&buf[0], bytes_to_read)) {
+ PLOG(WARNING) << "Zip: failed to write to file " << file.name();
return false;
}
count -= bytes_to_read;
@@ -166,6 +168,7 @@
UniquePtr<uint8_t[]> read_buf(new uint8_t[kBufSize]);
UniquePtr<uint8_t[]> write_buf(new uint8_t[kBufSize]);
if (read_buf.get() == NULL || write_buf.get() == NULL) {
+ LOG(WARNING) << "Zip: failed to alloctate buffer to inflate to file " << out.name();
return false;
}
@@ -216,6 +219,7 @@
(zerr == Z_STREAM_END && zstream->Get().avail_out != kBufSize)) {
size_t bytes_to_write = zstream->Get().next_out - write_buf.get();
if (!out.WriteFully(write_buf.get(), bytes_to_write)) {
+ PLOG(WARNING) << "Zip: failed to write to file " << out.name();
return false;
}
zstream->Get().next_out = write_buf.get();
@@ -238,6 +242,7 @@
bool ZipEntry::Extract(File& file) {
off_t data_offset = GetDataOffset();
if (data_offset == -1) {
+ LOG(WARNING) << "Zip: data_offset=" << data_offset;
return false;
}
if (lseek(zip_archive_->fd_, data_offset, SEEK_SET) != data_offset) {
@@ -253,6 +258,7 @@
case kCompressDeflated:
return InflateToFile(file, zip_archive_->fd_, GetUncompressedLength(), GetCompressedLength());
default:
+ LOG(WARNING) << "Zip: unknown compression method " << std::hex << GetCompressionMethod();
return false;
}
}