Revert "Load app images"
Fails when a method is duplicated (see test 097-duplicate-method)
Bug: 22858531
This reverts commit f7fd970244f143b1abb956e29794c446e4d57f46.
Change-Id: Ib30ae5be00cc568e799290be6b3c8f29cbbe4c20
diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc
index 818d50a..d021525 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -730,8 +730,8 @@
bool CompilerDriver::IsImageClass(const char* descriptor) const {
if (!IsBootImage()) {
- // NOTE: Currently only reachable from InitImageMethodVisitor for the app image case.
- return true;
+ // NOTE: Currently unreachable, all callers check IsImage().
+ return false;
} else {
return image_classes_->find(descriptor) != image_classes_->end();
}
diff --git a/compiler/image_test.cc b/compiler/image_test.cc
index b841675..12132c0 100644
--- a/compiler/image_test.cc
+++ b/compiler/image_test.cc
@@ -155,11 +155,7 @@
{
std::vector<const char*> dup_oat_filename(1, dup_oat->GetPath().c_str());
std::vector<const char*> dup_image_filename(1, image_file.GetFilename().c_str());
- bool success_image = writer->Write(kInvalidFd,
- dup_image_filename,
- kInvalidFd,
- dup_oat_filename,
- dup_oat_filename[0]);
+ bool success_image = writer->Write(kInvalidImageFd, dup_image_filename, dup_oat_filename);
ASSERT_TRUE(success_image);
bool success_fixup = ElfWriter::Fixup(dup_oat.get(),
writer->GetOatDataBegin(dup_oat_filename[0]));
@@ -296,17 +292,11 @@
oat_data_begin,
oat_data_end,
oat_file_end,
- /*boot_image_begin*/0U,
- /*boot_image_size*/0U,
- /*boot_oat_begin*/0U,
- /*boot_oat_size_*/0U,
sizeof(void*),
/*compile_pic*/false,
- /*is_pic*/false,
ImageHeader::kDefaultStorageMode,
/*data_size*/0u);
ASSERT_TRUE(image_header.IsValid());
- ASSERT_TRUE(!image_header.IsAppImage());
char* magic = const_cast<char*>(image_header.GetMagic());
strcpy(magic, ""); // bad magic
diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc
index 72c615e..d0bb201 100644
--- a/compiler/image_writer.cc
+++ b/compiler/image_writer.cc
@@ -159,45 +159,17 @@
bool ImageWriter::Write(int image_fd,
const std::vector<const char*>& image_filenames,
- int oat_fd,
- const std::vector<const char*>& oat_filenames,
- const std::string& oat_location) {
- // If image_fd or oat_fd are not kInvalidFd then we may have empty strings in image_filenames or
- // oat_filenames.
+ const std::vector<const char*>& oat_filenames) {
CHECK(!image_filenames.empty());
- if (image_fd != kInvalidFd) {
- CHECK_EQ(image_filenames.size(), 1u);
- }
CHECK(!oat_filenames.empty());
- if (oat_fd != kInvalidFd) {
- CHECK_EQ(oat_filenames.size(), 1u);
- }
CHECK_EQ(image_filenames.size(), oat_filenames.size());
size_t oat_file_offset = 0;
for (size_t i = 0; i < oat_filenames.size(); ++i) {
const char* oat_filename = oat_filenames[i];
- std::unique_ptr<File> oat_file;
-
- if (oat_fd != -1) {
- if (strlen(oat_filename) == 0u) {
- oat_file.reset(new File(oat_fd, false));
- } else {
- oat_file.reset(new File(oat_fd, oat_filename, false));
- }
- int length = oat_file->GetLength();
- if (length < 0) {
- PLOG(ERROR) << "Oat file has negative length " << length;
- return false;
- } else {
- // Leave the fd open since dex2oat still needs to write out the oat file with the fd.
- oat_file->DisableAutoClose();
- }
- } else {
- oat_file.reset(OS::OpenFileReadWrite(oat_filename));
- }
- if (oat_file == nullptr) {
+ std::unique_ptr<File> oat_file(OS::OpenFileReadWrite(oat_filename));
+ if (oat_file.get() == nullptr) {
PLOG(ERROR) << "Failed to open oat file " << oat_filename;
return false;
}
@@ -209,7 +181,7 @@
return false;
}
Runtime::Current()->GetOatFileManager().RegisterOatFile(
- std::unique_ptr<const OatFile>(oat_file_));
+ std::unique_ptr<const OatFile>(oat_file_));
const OatHeader& oat_header = oat_file_->GetOatHeader();
ImageInfo& image_info = GetImageInfo(oat_filename);
@@ -248,15 +220,8 @@
SetOatChecksumFromElfFile(oat_file.get());
- if (oat_fd != -1) {
- // Leave fd open for caller.
- if (oat_file->Flush() != 0) {
- LOG(ERROR) << "Failed to flush oat file " << oat_filename << " for " << oat_location;
- return false;
- }
- } else if (oat_file->FlushCloseOrErase() != 0) {
- LOG(ERROR) << "Failed to flush and close oat file " << oat_filename
- << " for " << oat_location;
+ if (oat_file->FlushCloseOrErase() != 0) {
+ LOG(ERROR) << "Failed to flush and close oat file " << oat_filename;
return false;
}
}
@@ -273,22 +238,16 @@
const char* oat_filename = oat_filenames[i];
ImageInfo& image_info = GetImageInfo(oat_filename);
std::unique_ptr<File> image_file;
- if (image_fd != kInvalidFd) {
- if (strlen(image_filename) == 0u) {
- image_file.reset(new File(image_fd, unix_file::kCheckSafeUsage));
- } else {
- LOG(ERROR) << "image fd " << image_fd << " name " << image_filename;
- }
+ if (image_fd != kInvalidImageFd) {
+ image_file.reset(new File(image_fd, image_filename, unix_file::kCheckSafeUsage));
} else {
image_file.reset(OS::CreateEmptyFile(image_filename));
}
-
if (image_file == nullptr) {
LOG(ERROR) << "Failed to open image file " << image_filename;
return false;
}
-
- if (!compile_app_image_ && fchmod(image_file->Fd(), 0644) != 0) {
+ if (fchmod(image_file->Fd(), 0644) != 0) {
PLOG(ERROR) << "Failed to make image file world readable: " << image_filename;
image_file->Erase();
return EXIT_FAILURE;
@@ -742,7 +701,6 @@
std::unordered_set<mirror::Class*>* visited) {
DCHECK(early_exit != nullptr);
DCHECK(visited != nullptr);
- DCHECK(compile_app_image_);
if (klass == nullptr) {
return false;
}
@@ -759,13 +717,6 @@
visited->emplace(klass);
bool result = IsBootClassLoaderNonImageClass(klass);
bool my_early_exit = false; // Only for ourselves, ignore caller.
- // Remove classes that failed to verify since we don't want to have java.lang.VerifyError in the
- // app image.
- if (klass->GetStatus() == mirror::Class::kStatusError) {
- result = true;
- } else {
- CHECK(klass->GetVerifyError() == nullptr) << PrettyClass(klass);
- }
if (!result) {
// Check interfaces since these wont be visited through VisitReferences.)
mirror::IfTable* if_table = klass->GetIfTable();
@@ -776,12 +727,6 @@
visited);
}
}
- if (klass->IsObjectArrayClass()) {
- result = result || ContainsBootClassLoaderNonImageClassInternal(
- klass->GetComponentType(),
- &my_early_exit,
- visited);
- }
// Check static fields and their classes.
size_t num_static_fields = klass->NumReferenceStaticFields();
if (num_static_fields != 0 && klass->IsResolved()) {
@@ -835,9 +780,7 @@
if (compile_app_image_) {
// For app images, we need to prune boot loader classes that are not in the boot image since
// these may have already been loaded when the app image is loaded.
- // Keep classes in the boot image space since we don't want to re-resolve these.
- return Runtime::Current()->GetHeap()->ObjectIsInBootImageSpace(klass) ||
- !ContainsBootClassLoaderNonImageClass(klass);
+ return !ContainsBootClassLoaderNonImageClass(klass);
}
std::string temp;
return compiler_driver_.IsImageClass(klass->GetDescriptor(&temp));
@@ -900,25 +843,25 @@
for (size_t i = 0, num = dex_cache->NumResolvedMethods(); i != num; ++i) {
ArtMethod* method =
mirror::DexCache::GetElementPtrSize(resolved_methods, i, target_ptr_size_);
- DCHECK(method != nullptr) << "Expected resolution method instead of null method";
- mirror::Class* declaring_class = method->GetDeclaringClass();
- // Miranda methods may be held live by a class which was not an image class but have a
- // declaring class which is an image class. Set it to the resolution method to be safe and
- // prevent dangling pointers.
- if (method->IsMiranda() || !KeepClass(declaring_class)) {
- mirror::DexCache::SetElementPtrSize(resolved_methods,
- i,
- resolution_method,
- target_ptr_size_);
- } else {
- // Check that the class is still in the classes table.
- DCHECK(class_linker->ClassInClassTable(declaring_class)) << "Class "
- << PrettyClass(declaring_class) << " not in class linker table";
+ if (method != nullptr) {
+ auto* declaring_class = method->GetDeclaringClass();
+ // Miranda methods may be held live by a class which was not an image class but have a
+ // declaring class which is an image class. Set it to the resolution method to be safe and
+ // prevent dangling pointers.
+ if (method->IsMiranda() || !KeepClass(declaring_class)) {
+ mirror::DexCache::SetElementPtrSize(resolved_methods,
+ i,
+ resolution_method,
+ target_ptr_size_);
+ } else {
+ // Check that the class is still in the classes table.
+ DCHECK(class_linker->ClassInClassTable(declaring_class)) << "Class "
+ << PrettyClass(declaring_class) << " not in class linker table";
+ }
}
}
- ArtField** resolved_fields = dex_cache->GetResolvedFields();
for (size_t i = 0; i < dex_cache->NumResolvedFields(); i++) {
- ArtField* field = mirror::DexCache::GetElementPtrSize(resolved_fields, i, target_ptr_size_);
+ ArtField* field = dex_cache->GetResolvedField(i, target_ptr_size_);
if (field != nullptr && !KeepClass(field->GetDeclaringClass())) {
dex_cache->SetResolvedField(i, nullptr, target_ptr_size_);
}
@@ -963,32 +906,6 @@
}
}
-mirror::String* ImageWriter::FindInternedString(mirror::String* string) {
- Thread* const self = Thread::Current();
- for (auto& pair : image_info_map_) {
- const ImageInfo& image_info = pair.second;
- mirror::String* const found = image_info.intern_table_->LookupStrong(self, string);
- DCHECK(image_info.intern_table_->LookupWeak(self, string) == nullptr)
- << string->ToModifiedUtf8();
- if (found != nullptr) {
- return found;
- }
- }
- if (compile_app_image_) {
- Runtime* const runtime = Runtime::Current();
- mirror::String* found = runtime->GetInternTable()->LookupStrong(self, string);
- // If we found it in the runtime intern table it could either be in the boot image or interned
- // during app image compilation. If it was in the boot image return that, otherwise return null
- // since it belongs to another image space.
- if (found != nullptr && runtime->GetHeap()->ObjectIsInBootImageSpace(found)) {
- return found;
- }
- DCHECK(runtime->GetInternTable()->LookupWeak(self, string) == nullptr)
- << string->ToModifiedUtf8();
- }
- return nullptr;
-}
-
void ImageWriter::CalculateObjectBinSlots(Object* obj) {
DCHECK(obj != nullptr);
// if it is a string, we want to intern it if its not interned.
@@ -998,16 +915,13 @@
// we must be an interned string that was forward referenced and already assigned
if (IsImageBinSlotAssigned(obj)) {
- DCHECK_EQ(obj, FindInternedString(obj->AsString()));
+ DCHECK_EQ(obj, image_info.intern_table_->InternStrongImageString(obj->AsString()));
return;
}
- // Need to check if the string is already interned in another image info so that we don't have
- // the intern tables of two different images contain the same string.
- mirror::String* interned = FindInternedString(obj->AsString());
- if (interned == nullptr) {
- // Not in another image space, insert to our table.
- interned = image_info.intern_table_->InternStrongImageString(obj->AsString());
- }
+ // InternImageString allows us to intern while holding the heap bitmap lock. This is safe since
+ // we are guaranteed to not have GC during image writing.
+ mirror::String* const interned = image_info.intern_table_->InternStrongImageString(
+ obj->AsString());
if (obj != interned) {
if (!IsImageBinSlotAssigned(interned)) {
// interned obj is after us, allocate its location early
@@ -1152,11 +1066,6 @@
// Visit and assign offsets for fields and field arrays.
auto* as_klass = h_obj->AsClass();
mirror::DexCache* dex_cache = as_klass->GetDexCache();
- DCHECK_NE(klass->GetStatus(), mirror::Class::kStatusError);
- if (compile_app_image_) {
- // Extra sanity, no boot loader classes should be left!
- CHECK(!IsBootClassLoaderClass(as_klass)) << PrettyClass(as_klass);
- }
LengthPrefixedArray<ArtField>* fields[] = {
as_klass->GetSFieldsPtr(), as_klass->GetIFieldsPtr(),
};
@@ -1496,13 +1405,6 @@
<< " Oat data end=" << reinterpret_cast<uintptr_t>(oat_data_end)
<< " Oat file end=" << reinterpret_cast<uintptr_t>(oat_file_end);
}
- // Store boot image info for app image so that we can relocate.
- uint32_t boot_image_begin = 0;
- uint32_t boot_image_end = 0;
- uint32_t boot_oat_begin = 0;
- uint32_t boot_oat_end = 0;
- gc::Heap* const heap = Runtime::Current()->GetHeap();
- heap->GetBootImagesSize(&boot_image_begin, &boot_image_end, &boot_oat_begin, &boot_oat_end);
// Create the header, leave 0 for data size since we will fill this in as we are writing the
// image.
@@ -1515,13 +1417,8 @@
PointerToLowMemUInt32(image_info.oat_data_begin_),
PointerToLowMemUInt32(oat_data_end),
PointerToLowMemUInt32(oat_file_end),
- boot_image_begin,
- boot_image_end - boot_image_begin,
- boot_oat_begin,
- boot_oat_end - boot_oat_begin,
target_ptr_size_,
compile_pic_,
- /*is_pic*/compile_app_image_,
image_storage_mode_,
/*data_size*/0u);
}
@@ -1908,14 +1805,13 @@
if (klass == class_linker->GetClassRoot(ClassLinker::kJavaLangDexCache)) {
FixupDexCache(down_cast<mirror::DexCache*>(orig), down_cast<mirror::DexCache*>(copy));
} else if (klass->IsClassLoaderClass()) {
- mirror::ClassLoader* copy_loader = down_cast<mirror::ClassLoader*>(copy);
// If src is a ClassLoader, set the class table to null so that it gets recreated by the
// ClassLoader.
- copy_loader->SetClassTable(nullptr);
+ down_cast<mirror::ClassLoader*>(copy)->SetClassTable(nullptr);
// Also set allocator to null to be safe. The allocator is created when we create the class
// table. We also never expect to unload things in the image since they are held live as
// roots.
- copy_loader->SetAllocator(nullptr);
+ down_cast<mirror::ClassLoader*>(copy)->SetAllocator(nullptr);
}
}
FixupVisitor visitor(this, copy);
@@ -2000,7 +1896,7 @@
// If we are compiling an app image, we need to use the stubs of the boot image.
if (compile_app_image_) {
// Use the current image pointers.
- const std::vector<gc::space::ImageSpace*>& image_spaces =
+ std::vector<gc::space::ImageSpace*> image_spaces =
Runtime::Current()->GetHeap()->GetBootImageSpaces();
DCHECK(!image_spaces.empty());
const OatFile* oat_file = image_spaces[0]->GetOatFile();
diff --git a/compiler/image_writer.h b/compiler/image_writer.h
index 622eb19..ad69038 100644
--- a/compiler/image_writer.h
+++ b/compiler/image_writer.h
@@ -49,7 +49,7 @@
class ClassTable;
-static constexpr int kInvalidFd = -1;
+static constexpr int kInvalidImageFd = -1;
// Write a Space built during compilation for use during execution.
class ImageWriter FINAL {
@@ -103,15 +103,11 @@
uint8_t* GetOatFileBegin(const char* oat_filename) const;
- // If image_fd is not kInvalidFd, then we use that for the image file. Otherwise we open
+ // If image_fd is not kInvalidImageFd, then we use that for the file. Otherwise we open
// the names in image_filenames.
- // If oat_fd is not kInvalidFd, then we use that for the oat file. Otherwise we open
- // the names in oat_filenames.
bool Write(int image_fd,
const std::vector<const char*>& image_filenames,
- int oat_fd,
- const std::vector<const char*>& oat_filenames,
- const std::string& oat_location)
+ const std::vector<const char*>& oat_filenames)
REQUIRES(!Locks::mutator_lock_);
uintptr_t GetOatDataBegin(const char* oat_filename) {
@@ -451,10 +447,6 @@
const ImageInfo& GetConstImageInfo(const char* oat_filename) const;
const ImageInfo& GetImageInfo(size_t index) const;
- // Find an already strong interned string in the other images or in the boot image. Used to
- // remove duplicates in the multi image and app image case.
- mirror::String* FindInternedString(mirror::String* string) SHARED_REQUIRES(Locks::mutator_lock_);
-
const CompilerDriver& compiler_driver_;
// Beginning target image address for the first image.
diff --git a/compiler/oat_writer.cc b/compiler/oat_writer.cc
index 1e405db..c74c41f 100644
--- a/compiler/oat_writer.cc
+++ b/compiler/oat_writer.cc
@@ -958,40 +958,30 @@
}
ClassLinker* linker = Runtime::Current()->GetClassLinker();
+ InvokeType invoke_type = it.GetMethodInvokeType(dex_file_->GetClassDef(class_def_index_));
// Unchecked as we hold mutator_lock_ on entry.
ScopedObjectAccessUnchecked soa(Thread::Current());
StackHandleScope<1> hs(soa.Self());
Handle<mirror::DexCache> dex_cache(hs.NewHandle(linker->FindDexCache(
Thread::Current(), *dex_file_)));
- ArtMethod* method;
- if (writer_->HasBootImage()) {
- const InvokeType invoke_type = it.GetMethodInvokeType(
- dex_file_->GetClassDef(class_def_index_));
- method = linker->ResolveMethod<ClassLinker::kNoICCECheckForCache>(
- *dex_file_,
- it.GetMemberIndex(),
- dex_cache,
- ScopedNullHandle<mirror::ClassLoader>(),
- nullptr,
- invoke_type);
- if (method == nullptr) {
- LOG(INTERNAL_FATAL) << "Unexpected failure to resolve a method: "
- << PrettyMethod(it.GetMemberIndex(), *dex_file_, true);
- soa.Self()->AssertPendingException();
- mirror::Throwable* exc = soa.Self()->GetException();
- std::string dump = exc->Dump();
- LOG(FATAL) << dump;
- UNREACHABLE();
- }
- } else {
- // Should already have been resolved by the compiler, just peek into the dex cache.
- // It may not be resolved if the class failed to verify, in this case, don't set the
- // entrypoint. This is not fatal since the dex cache will contain a resolution method.
- method = dex_cache->GetResolvedMethod(it.GetMemberIndex(), linker->GetImagePointerSize());
+ ArtMethod* method = linker->ResolveMethod<ClassLinker::kNoICCECheckForCache>(
+ *dex_file_,
+ it.GetMemberIndex(),
+ dex_cache,
+ ScopedNullHandle<mirror::ClassLoader>(),
+ nullptr,
+ invoke_type);
+ if (method == nullptr) {
+ LOG(INTERNAL_FATAL) << "Unexpected failure to resolve a method: "
+ << PrettyMethod(it.GetMemberIndex(), *dex_file_, true);
+ soa.Self()->AssertPendingException();
+ mirror::Throwable* exc = soa.Self()->GetException();
+ std::string dump = exc->Dump();
+ LOG(FATAL) << dump;
+ UNREACHABLE();
}
- if (method != nullptr &&
- compiled_method != nullptr &&
- compiled_method->GetQuickCode().size() != 0) {
+
+ if (compiled_method != nullptr && compiled_method->GetQuickCode().size() != 0) {
method->SetEntryPointFromQuickCompiledCodePtrSize(
reinterpret_cast<void*>(offsets.code_offset_), pointer_size_);
}
@@ -1477,7 +1467,7 @@
} while (false)
VISIT(InitCodeMethodVisitor);
- if (HasImage()) {
+ if (compiler_driver_->IsBootImage()) {
VISIT(InitImageMethodVisitor);
}