Skip to content

Commit

Permalink
Fix handling for invalid or missing app images
Browse files Browse the repository at this point in the history
Avoid crashing when there is an invalid app image path passed to
oatdump. Added regression test.

Bug: 137724009
Test: test-art-host-gtest-oatdump_app_test

Change-Id: I27470d0c1d844de5b9f3f3bf960e925cd8977d50
  • Loading branch information
Mathieu Chartier committed Jul 19, 2019
1 parent d84794d commit 6b689ce
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 4 deletions.
2 changes: 2 additions & 0 deletions oatdump/oatdump.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2782,12 +2782,14 @@ static int DumpImages(Runtime* runtime, OatDumperOptions* options, std::ostream*
if (space == nullptr) {
LOG(ERROR) << "Failed to open app image " << options->app_image_ << " with error "
<< error_msg;
return EXIT_FAILURE;
}
// Open dex files for the image.
std::vector<std::unique_ptr<const DexFile>> dex_files;
if (!runtime->GetClassLinker()->OpenImageDexFiles(space.get(), &dex_files, &error_msg)) {
LOG(ERROR) << "Failed to open app image dex files " << options->app_image_ << " with error "
<< error_msg;
return EXIT_FAILURE;
}
// Dump the actual image.
int result = DumpImage(space.get(), options, os);
Expand Down
9 changes: 9 additions & 0 deletions oatdump/oatdump_app_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,13 @@ TEST_F(OatDumpTest, TestAppImageWithBootImageStatic) {
ASSERT_TRUE(Exec(kStatic, kModeAppImage, {}, kListAndCode));
}

TEST_F(OatDumpTest, TestAppImageInvalidPath) {
TEST_DISABLED_WITHOUT_BAKER_READ_BARRIERS(); // GC bug, b/126305867
TEST_DISABLED_FOR_NON_STATIC_HOST_BUILDS();
const std::string app_image_arg = "--app-image-file=" + GetAppImageName();
ASSERT_TRUE(GenerateAppOdexFile(kStatic, {"--runtime-arg", "-Xmx64M", app_image_arg}));
SetAppImageName("missing_app_image.art");
ASSERT_TRUE(Exec(kStatic, kModeAppImage, {}, kListAndCode, /*expect_failure=*/true));
}

} // namespace art
25 changes: 21 additions & 4 deletions oatdump/oatdump_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,15 @@ class OatDumpTest : public CommonRuntimeTest {
return "ProfileTestMultiDex";
}

void SetAppImageName(const std::string& name) {
app_image_name_ = name;
}

std::string GetAppImageName() {
return tmp_dir_ + "/" + GetAppBaseName() + ".art";
if (app_image_name_.empty()) {
app_image_name_ = tmp_dir_ + "/" + GetAppBaseName() + ".art";
}
return app_image_name_;
}

std::string GetAppOdexName() {
Expand Down Expand Up @@ -158,7 +165,8 @@ class OatDumpTest : public CommonRuntimeTest {
::testing::AssertionResult Exec(Flavor flavor,
Mode mode,
const std::vector<std::string>& args,
Display display) {
Display display,
bool expect_failure = false) {
std::string file_path = GetExecutableFilePath(flavor, "oatdump");

if (!OS::FileExists(file_path.c_str())) {
Expand Down Expand Up @@ -322,8 +330,17 @@ class OatDumpTest : public CommonRuntimeTest {
if (res.stage != ForkAndExecResult::kFinished) {
return ::testing::AssertionFailure() << strerror(errno);
}
error_buf.push_back(0); // Make data a C string.

if (!res.StandardSuccess()) {
return ::testing::AssertionFailure() << "Did not terminate successfully: " << res.status_code;
if (expect_failure && WIFEXITED(res.status_code)) {
// Avoid crash as valid exit.
return ::testing::AssertionSuccess();
}
return ::testing::AssertionFailure() << "Did not terminate successfully: " << res.status_code
<< " " << error_buf.data();
} else if (expect_failure) {
return ::testing::AssertionFailure() << "Expected failure";
}

if (mode == kModeSymbolize) {
Expand All @@ -342,14 +359,14 @@ class OatDumpTest : public CommonRuntimeTest {
}
if (!result) {
oss << "Processed bytes " << total << ":" << std::endl;
error_buf.push_back(0); // Make data a C string.
}

return result ? ::testing::AssertionSuccess()
: (::testing::AssertionFailure() << oss.str() << error_buf.data());
}

std::string tmp_dir_;
std::string app_image_name_;

private:
std::string core_art_location_;
Expand Down

0 comments on commit 6b689ce

Please sign in to comment.