Do not execute OAT files that require app images that cannot be loaded
This change creates a new requires-image flag in the OAT header, which
is set when the file was created with an image (app image, etc.). If
this flag is set, we will not load the OAT file as executable if the
image could not be loaded.
Going forward, this allows the compiler to assume there will be an app
image when an app image is generated and in some cases generate better
code.
Note that we still must load the OAT file, because there will not always
be another way to access the underlying DEX files.
Revert submission 1298633-revert-155218105
Reason for revert: Fixing tests and relanding
Reverted Changes:
I701c91d5b:Revert "Disable ART run-test 2231-oat-require-app-...
Ic5cda4c75:Revert "Reject OAT file in speed-profile if app im...
Bug: 38313278
Test: m test-art-host-gtest-oat_file_assistant_test64
Test: atest android.server.wm.MultiDisplaySecurityTests#testDisplayHasAccess_UIDCanPresentOnPrivateDisplay android.classloaders.cts.UsesLibraryHostTest#testUsesLibrary_full
Change-Id: I52cca033fa8e2e6de86514c833798c3d99b99477
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index e767224..c0f73c8 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -521,6 +521,9 @@
UsageError("");
UsageError(" --max-image-block-size=<size>: Maximum solid block size for compressed images.");
UsageError("");
+ UsageError(" --compile-individually: Compiles dex files individually, unloading classes in");
+ UsageError(" between compiling each file.");
+ UsageError("");
std::cerr << "See log for usage error information\n";
exit(EXIT_FAILURE);
}
@@ -824,7 +827,8 @@
timings_(timings),
force_determinism_(false),
check_linkage_conditions_(false),
- crash_on_linkage_violation_(false)
+ crash_on_linkage_violation_(false),
+ compile_individually_(false)
{}
~Dex2Oat() {
@@ -1215,6 +1219,7 @@
key_value_store_->Put(OatHeader::kCompilerFilter,
CompilerFilter::NameOfFilter(compiler_options_->GetCompilerFilter()));
key_value_store_->Put(OatHeader::kConcurrentCopying, kUseReadBarrier);
+ key_value_store_->Put(OatHeader::kRequiresImage, compiler_options_->IsGeneratingImage());
if (invocation_file_.get() != -1) {
std::ostringstream oss;
for (int i = 0; i < argc; ++i) {
@@ -1375,6 +1380,7 @@
if (args.Exists(M::ForceDeterminism)) {
force_determinism_ = true;
}
+ AssignTrueIfExists(args, M::CompileIndividually, &compile_individually_);
if (args.Exists(M::Base)) {
ParseBase(*args.Get(M::Base));
@@ -2013,17 +2019,17 @@
}
bool ShouldCompileDexFilesIndividually() const {
- // Compile individually if we are:
- // 1. not building an image,
- // 2. not verifying a vdex file,
- // 3. using multidex,
+ // Compile individually if we are specifically asked to, or
+ // 1. not building an image, and
+ // 2. not verifying a vdex file, and
+ // 3. using multidex, and
// 4. not doing any AOT compilation.
// This means extract, no-vdex verify, and quicken, will use the individual compilation
// mode (to reduce RAM used by the compiler).
- return !IsImage() &&
- !update_input_vdex_ &&
- compiler_options_->dex_files_for_oat_file_.size() > 1 &&
- !CompilerFilter::IsAotCompilationEnabled(compiler_options_->GetCompilerFilter());
+ return compile_individually_ ||
+ (!IsImage() && !update_input_vdex_ &&
+ compiler_options_->dex_files_for_oat_file_.size() > 1 &&
+ !CompilerFilter::IsAotCompilationEnabled(compiler_options_->GetCompilerFilter()));
}
uint32_t GetCombinedChecksums() const {
@@ -3097,6 +3103,9 @@
// The reason for invoking the compiler.
std::string compilation_reason_;
+ // Whether to force individual compilation.
+ bool compile_individually_;
+
DISALLOW_IMPLICIT_CONSTRUCTORS(Dex2Oat);
};
diff --git a/dex2oat/dex2oat_options.cc b/dex2oat/dex2oat_options.cc
index d282c3b..81b9c98 100644
--- a/dex2oat/dex2oat_options.cc
+++ b/dex2oat/dex2oat_options.cc
@@ -262,7 +262,9 @@
.IntoKey(M::RuntimeOptions)
.Define("--compilation-reason=_")
.WithType<std::string>()
- .IntoKey(M::CompilationReason);
+ .IntoKey(M::CompilationReason)
+ .Define("--compile-individually")
+ .IntoKey(M::CompileIndividually);
AddCompilerOptionsArgumentParserOptions<Dex2oatArgumentMap>(*parser_builder);
diff --git a/dex2oat/dex2oat_options.def b/dex2oat/dex2oat_options.def
index 805a13e..8b018bf 100644
--- a/dex2oat/dex2oat_options.def
+++ b/dex2oat/dex2oat_options.def
@@ -94,5 +94,7 @@
DEX2OAT_OPTIONS_KEY (std::string, CompilationReason)
DEX2OAT_OPTIONS_KEY (Unit, CheckLinkageConditions)
DEX2OAT_OPTIONS_KEY (Unit, CrashOnLinkageViolation)
+DEX2OAT_OPTIONS_KEY (Unit, CompileIndividually)
+
#undef DEX2OAT_OPTIONS_KEY
diff --git a/dex2oat/dex2oat_test.cc b/dex2oat/dex2oat_test.cc
index 54d5196..81fd09a 100644
--- a/dex2oat/dex2oat_test.cc
+++ b/dex2oat/dex2oat_test.cc
@@ -1308,7 +1308,6 @@
const std::string unload_vdex_name = out_dir + "/unload.vdex";
const std::string no_unload_oat_name = out_dir + "/nounload.oat";
const std::string no_unload_vdex_name = out_dir + "/nounload.vdex";
- const std::string app_image_name = out_dir + "/unload.art";
std::string error_msg;
const std::vector<gc::space::ImageSpace*>& spaces = runtime->GetHeap()->GetBootImageSpaces();
ASSERT_GT(spaces.size(), 0u);
@@ -1336,7 +1335,7 @@
base_oat_name,
CompilerFilter::Filter::kQuicken,
&error_msg,
- {"--force-determinism", "--avoid-storing-invocation", "--app-image-file=" + app_image_name});
+ {"--force-determinism", "--avoid-storing-invocation", "--compile-individually"});
ASSERT_EQ(res2, 0);
Copy(base_oat_name, no_unload_oat_name);
Copy(base_vdex_name, no_unload_vdex_name);
@@ -1353,10 +1352,6 @@
<< unload_oat_name << " " << no_unload_oat_name;
EXPECT_EQ(unload_vdex->Compare(no_unload_vdex.get()), 0)
<< unload_vdex_name << " " << no_unload_vdex_name;
- // App image file.
- std::unique_ptr<File> app_image_file(OS::OpenFileForReading(app_image_name.c_str()));
- ASSERT_TRUE(app_image_file != nullptr);
- EXPECT_GT(app_image_file->GetLength(), 0u);
}
// Test that dexlayout section info is correctly written to the oat file for profile based
diff --git a/runtime/oat.cc b/runtime/oat.cc
index 723bf49..52e0dd1 100644
--- a/runtime/oat.cc
+++ b/runtime/oat.cc
@@ -396,6 +396,10 @@
return IsKeyEnabled(OatHeader::kNativeDebuggableKey);
}
+bool OatHeader::RequiresImage() const {
+ return IsKeyEnabled(OatHeader::kRequiresImage);
+}
+
CompilerFilter::Filter OatHeader::GetCompilerFilter() const {
CompilerFilter::Filter filter;
const char* key_value = GetStoreValueByKey(kCompilerFilter);
diff --git a/runtime/oat.h b/runtime/oat.h
index c1659e4..0645e4c 100644
--- a/runtime/oat.h
+++ b/runtime/oat.h
@@ -32,8 +32,8 @@
class PACKED(4) OatHeader {
public:
static constexpr std::array<uint8_t, 4> kOatMagic { { 'o', 'a', 't', '\n' } };
- // Last oat version changed reason: Change x86-64 @CriticalNative hidden arg register to RAX.
- static constexpr std::array<uint8_t, 4> kOatVersion { { '1', '8', '1', '\0' } };
+ // Last oat version changed reason: Add requires-image flag.
+ static constexpr std::array<uint8_t, 4> kOatVersion { { '1', '8', '2', '\0' } };
static constexpr const char* kDex2OatCmdLineKey = "dex2oat-cmdline";
static constexpr const char* kDebuggableKey = "debuggable";
@@ -44,6 +44,7 @@
static constexpr const char* kBootClassPathChecksumsKey = "bootclasspath-checksums";
static constexpr const char* kConcurrentCopying = "concurrent-copying";
static constexpr const char* kCompilationReasonKey = "compilation-reason";
+ static constexpr const char* kRequiresImage = "requires-image";
static constexpr const char kTrueValue[] = "true";
static constexpr const char kFalseValue[] = "false";
@@ -102,6 +103,7 @@
bool IsNativeDebuggable() const;
CompilerFilter::Filter GetCompilerFilter() const;
bool IsConcurrentCopying() const;
+ bool RequiresImage() const;
private:
bool KeyHasValue(const char* key, const char* value, size_t value_size) const;
diff --git a/runtime/oat_file.cc b/runtime/oat_file.cc
index 346c112..a7054d3 100644
--- a/runtime/oat_file.cc
+++ b/runtime/oat_file.cc
@@ -2124,6 +2124,8 @@
return oat_dex_file->GetOatClass(class_def_idx);
}
+bool OatFile::RequiresImage() const { return GetOatHeader().RequiresImage(); }
+
static void DCheckIndexToBssMapping(const OatFile* oat_file,
uint32_t number_of_indexes,
size_t slot_size,
diff --git a/runtime/oat_file.h b/runtime/oat_file.h
index dce34d9..ee5aede 100644
--- a/runtime/oat_file.h
+++ b/runtime/oat_file.h
@@ -378,6 +378,9 @@
return external_dex_files_.empty();
}
+ // Returns whether an image (e.g. app image) is required to safely execute this OAT file.
+ bool RequiresImage() const;
+
protected:
OatFile(const std::string& filename, bool executable);
diff --git a/runtime/oat_file_assistant_test.cc b/runtime/oat_file_assistant_test.cc
index ed47ca3..7b1a5eb 100644
--- a/runtime/oat_file_assistant_test.cc
+++ b/runtime/oat_file_assistant_test.cc
@@ -1437,6 +1437,7 @@
TEST_F(OatFileAssistantTest, GetDexLocation) {
std::string dex_location = GetScratchDir() + "/TestDex.jar";
std::string oat_location = GetOdexDir() + "/TestDex.odex";
+ std::string art_location = GetOdexDir() + "/TestDex.art";
// Start the runtime to initialize the system's class loader.
Thread::Current()->TransitionFromSuspendedToRunnable();
@@ -1463,6 +1464,7 @@
args.push_back("--dex-file=" + dex_location);
args.push_back("--dex-location=TestDex.jar");
args.push_back("--oat-file=" + oat_location);
+ args.push_back("--app-image-file=" + art_location);
std::string error_msg;
ASSERT_TRUE(DexoptTest::Dex2Oat(args, &error_msg)) << error_msg;
}
@@ -1490,6 +1492,7 @@
odex_dir = odex_dir + std::string(GetInstructionSetString(kRuntimeISA));
mkdir(odex_dir.c_str(), 0700);
std::string oat_location = odex_dir + "/" + filebase + ".odex";
+ std::string art_location = odex_dir + "/" + filebase + ".art";
// Clean up in case previous run crashed.
remove(oat_location.c_str());
@@ -1527,6 +1530,7 @@
args.push_back("--dex-file=" + dex_location);
args.push_back("--dex-location=" + filebase + ".jar");
args.push_back("--oat-file=" + oat_location);
+ args.push_back("--app-image-file=" + art_location);
std::string error_msg;
ASSERT_TRUE(DexoptTest::Dex2Oat(args, &error_msg)) << error_msg;
}
@@ -1554,6 +1558,41 @@
EXPECT_EQ(0, remove(oat_location.c_str()));
}
+// Make sure OAT files that require app images are not loaded as executable.
+TEST_F(OatFileAssistantTest, LoadOatNoArt) {
+ std::string dex_location = GetScratchDir() + "/TestDex.jar";
+ std::string odex_location = GetOdexDir() + "/TestDex.odex";
+ std::string art_location = GetOdexDir() + "/TestDex.art";
+ Copy(GetDexSrc1(), dex_location);
+ GenerateOdexForTest(dex_location,
+ odex_location,
+ CompilerFilter::kSpeed,
+ "install",
+ {
+ "--app-image-file=" + art_location,
+ });
+
+ unlink(art_location.c_str());
+
+ std::vector<std::string> error_msgs;
+ const OatFile* oat_file = nullptr;
+
+ // Start the runtime to initialize the system's class loader.
+ Thread::Current()->TransitionFromSuspendedToRunnable();
+ runtime_->Start();
+
+ const auto dex_files = Runtime::Current()->GetOatFileManager().OpenDexFilesFromOat(
+ dex_location.c_str(),
+ Runtime::Current()->GetSystemClassLoader(),
+ /*dex_elements=*/nullptr,
+ &oat_file,
+ &error_msgs);
+
+ EXPECT_FALSE(dex_files.empty());
+ EXPECT_NE(oat_file, nullptr);
+ EXPECT_FALSE(oat_file->IsExecutable());
+}
+
// TODO: More Tests:
// * Test class linker falls back to unquickened dex for DexNoOat
// * Test class linker falls back to unquickened dex for MultiDexNoOat
diff --git a/runtime/oat_file_manager.cc b/runtime/oat_file_manager.cc
index ff743d5..14ec631 100644
--- a/runtime/oat_file_manager.cc
+++ b/runtime/oat_file_manager.cc
@@ -492,13 +492,14 @@
const OatFile* source_oat_file = nullptr;
CheckCollisionResult check_collision_result = CheckCollisionResult::kPerformedHasCollisions;
std::string error_msg;
+ bool accept_oat_file = false;
if ((class_loader != nullptr || dex_elements != nullptr) && oat_file != nullptr) {
// Prevent oat files from being loaded if no class_loader or dex_elements are provided.
// This can happen when the deprecated DexFile.<init>(String) is called directly, and it
// could load oat files without checking the classpath, which would be incorrect.
// Take the file only if it has no collisions, or we must take it because of preopting.
check_collision_result = CheckCollision(oat_file.get(), context.get(), /*out*/ &error_msg);
- bool accept_oat_file = AcceptOatFile(check_collision_result);
+ accept_oat_file = AcceptOatFile(check_collision_result);
if (!accept_oat_file) {
// Failed the collision check. Print warning.
if (runtime->IsDexFileFallbackEnabled()) {
@@ -531,30 +532,24 @@
LOG(WARNING) << error_msg;
}
-
- if (accept_oat_file) {
- VLOG(class_linker) << "Registering " << oat_file->GetLocation();
- source_oat_file = RegisterOatFile(std::move(oat_file));
- *out_oat_file = source_oat_file;
- }
}
std::vector<std::unique_ptr<const DexFile>> dex_files;
// Load the dex files from the oat file.
- if (source_oat_file != nullptr) {
- bool added_image_space = false;
- if (source_oat_file->IsExecutable()) {
+ bool added_image_space = false;
+ if (accept_oat_file) {
+ if (oat_file->IsExecutable()) {
ScopedTrace app_image_timing("AppImage:Loading");
// We need to throw away the image space if we are debuggable but the oat-file source of the
// image is not otherwise we might get classes with inlined methods or other such things.
std::unique_ptr<gc::space::ImageSpace> image_space;
if (ShouldLoadAppImage(check_collision_result,
- source_oat_file,
+ oat_file.get(),
context.get(),
&error_msg)) {
- image_space = oat_file_assistant.OpenImageSpace(source_oat_file);
+ image_space = oat_file_assistant.OpenImageSpace(oat_file.get());
}
if (image_space != nullptr) {
ScopedObjectAccess soa(self);
@@ -608,7 +603,18 @@
}
if (!added_image_space) {
DCHECK(dex_files.empty());
- dex_files = oat_file_assistant.LoadDexFiles(*source_oat_file, dex_location);
+
+ if (oat_file->RequiresImage()) {
+ // If we could not load the image, but the OAT file requires it, we have to reload the OAT
+ // file as non-executable.
+ OatFileAssistant nonexecutable_oat_file_assistant(dex_location,
+ kRuntimeISA,
+ /*load_executable=*/false,
+ only_use_system_oat_files_);
+ oat_file.reset(nonexecutable_oat_file_assistant.GetBestOatFile().release());
+ }
+
+ dex_files = oat_file_assistant.LoadDexFiles(*oat_file.get(), dex_location);
// Register for tracking.
for (const auto& dex_file : dex_files) {
@@ -616,13 +622,17 @@
}
}
if (dex_files.empty()) {
- error_msgs->push_back("Failed to open dex files from " + source_oat_file->GetLocation());
+ error_msgs->push_back("Failed to open dex files from " + oat_file->GetLocation());
} else {
// Opened dex files from an oat file, madvise them to their loaded state.
for (const std::unique_ptr<const DexFile>& dex_file : dex_files) {
OatDexFile::MadviseDexFile(*dex_file, MadviseState::kMadviseStateAtLoad);
}
}
+
+ VLOG(class_linker) << "Registering " << oat_file->GetLocation();
+ source_oat_file = RegisterOatFile(std::move(oat_file));
+ *out_oat_file = source_oat_file;
}
// Fall back to running out of the original dex file if we couldn't load any