Use PwriteFully and write image header last
Prevent corrupted images if dex2oat gets killed or if the image
writer is interrupted during writing.
Bug: 22858531
Bug: 27561308
(cherry picked from commit dba5a70977da0a28cec2bfc8261d52a177738477)
Change-Id: If4f2c43bcc3cf918b5d2780f1709225b5a4ce116
diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc
index 871435b..b1b971f 100644
--- a/compiler/image_writer.cc
+++ b/compiler/image_writer.cc
@@ -266,17 +266,9 @@
<< PrettyDuration(NanoTime() - compress_start_time);
}
- // Write header first, as uncompressed.
- image_header->data_size_ = data_size;
- if (!image_file->WriteFully(image_info.image_->Begin(), sizeof(ImageHeader))) {
- PLOG(ERROR) << "Failed to write image file header " << image_filename;
- image_file->Erase();
- return false;
- }
-
// Write out the image + fields + methods.
const bool is_compressed = compressed_data != nullptr;
- if (!image_file->WriteFully(image_data_to_write, data_size)) {
+ if (!image_file->PwriteFully(image_data_to_write, data_size, sizeof(ImageHeader))) {
PLOG(ERROR) << "Failed to write image file data " << image_filename;
image_file->Erase();
return false;
@@ -291,13 +283,33 @@
if (!is_compressed) {
CHECK_EQ(bitmap_position_in_file, bitmap_section.Offset());
}
- if (!image_file->Write(reinterpret_cast<char*>(image_info.image_bitmap_->Begin()),
- bitmap_section.Size(),
- bitmap_position_in_file)) {
+ if (!image_file->PwriteFully(reinterpret_cast<char*>(image_info.image_bitmap_->Begin()),
+ bitmap_section.Size(),
+ bitmap_position_in_file)) {
PLOG(ERROR) << "Failed to write image file " << image_filename;
image_file->Erase();
return false;
}
+
+ int err = image_file->Flush();
+ if (err < 0) {
+ PLOG(ERROR) << "Failed to flush image file " << image_filename << " with result " << err;
+ image_file->Erase();
+ return false;
+ }
+
+ // Write header last in case the compiler gets killed in the middle of image writing.
+ // We do not want to have a corrupted image with a valid header.
+ // The header is uncompressed since it contains whether the image is compressed or not.
+ image_header->data_size_ = data_size;
+ if (!image_file->PwriteFully(reinterpret_cast<char*>(image_info.image_->Begin()),
+ sizeof(ImageHeader),
+ 0)) {
+ PLOG(ERROR) << "Failed to write image file header " << image_filename;
+ image_file->Erase();
+ return false;
+ }
+
CHECK_EQ(bitmap_position_in_file + bitmap_section.Size(),
static_cast<size_t>(image_file->GetLength()));
if (image_file->FlushCloseOrErase() != 0) {
diff --git a/runtime/base/unix_file/fd_file.cc b/runtime/base/unix_file/fd_file.cc
index 4672948..e4097dd 100644
--- a/runtime/base/unix_file/fd_file.cc
+++ b/runtime/base/unix_file/fd_file.cc
@@ -234,21 +234,34 @@
return ReadFullyGeneric<pread>(fd_, buffer, byte_count, offset);
}
-bool FdFile::WriteFully(const void* buffer, size_t byte_count) {
+template <bool kUseOffset>
+bool FdFile::WriteFullyGeneric(const void* buffer, size_t byte_count, size_t offset) {
DCHECK(!read_only_mode_);
- const char* ptr = static_cast<const char*>(buffer);
moveTo(GuardState::kBase, GuardState::kClosed, "Writing into closed file.");
+ DCHECK(kUseOffset || offset == 0u);
+ const char* ptr = static_cast<const char*>(buffer);
while (byte_count > 0) {
- ssize_t bytes_written = TEMP_FAILURE_RETRY(write(fd_, ptr, byte_count));
+ ssize_t bytes_written = kUseOffset
+ ? TEMP_FAILURE_RETRY(pwrite(fd_, ptr, byte_count, offset))
+ : TEMP_FAILURE_RETRY(write(fd_, ptr, byte_count));
if (bytes_written == -1) {
return false;
}
byte_count -= bytes_written; // Reduce the number of remaining bytes.
ptr += bytes_written; // Move the buffer forward.
+ offset += static_cast<size_t>(bytes_written);
}
return true;
}
+bool FdFile::PwriteFully(const void* buffer, size_t byte_count, size_t offset) {
+ return WriteFullyGeneric<true>(buffer, byte_count, offset);
+}
+
+bool FdFile::WriteFully(const void* buffer, size_t byte_count) {
+ return WriteFullyGeneric<false>(buffer, byte_count, 0u);
+}
+
bool FdFile::Copy(FdFile* input_file, int64_t offset, int64_t size) {
DCHECK(!read_only_mode_);
off_t off = static_cast<off_t>(offset);
diff --git a/runtime/base/unix_file/fd_file.h b/runtime/base/unix_file/fd_file.h
index 8040afe..16cd44f 100644
--- a/runtime/base/unix_file/fd_file.h
+++ b/runtime/base/unix_file/fd_file.h
@@ -79,6 +79,7 @@
bool ReadFully(void* buffer, size_t byte_count) WARN_UNUSED;
bool PreadFully(void* buffer, size_t byte_count, size_t offset) WARN_UNUSED;
bool WriteFully(const void* buffer, size_t byte_count) WARN_UNUSED;
+ bool PwriteFully(const void* buffer, size_t byte_count, size_t offset) WARN_UNUSED;
// Copy data from another file.
bool Copy(FdFile* input_file, int64_t offset, int64_t size);
@@ -119,6 +120,9 @@
GuardState guard_state_;
private:
+ template <bool kUseOffset>
+ bool WriteFullyGeneric(const void* buffer, size_t byte_count, size_t offset);
+
int fd_;
std::string file_path_;
bool auto_close_;
diff --git a/runtime/base/unix_file/fd_file_test.cc b/runtime/base/unix_file/fd_file_test.cc
index ecf607c..9bc87e5 100644
--- a/runtime/base/unix_file/fd_file_test.cc
+++ b/runtime/base/unix_file/fd_file_test.cc
@@ -110,6 +110,34 @@
ASSERT_EQ(file.Close(), 0);
}
+TEST_F(FdFileTest, ReadWriteFullyWithOffset) {
+ // New scratch file, zero-length.
+ art::ScratchFile tmp;
+ FdFile file;
+ ASSERT_TRUE(file.Open(tmp.GetFilename(), O_RDWR));
+ EXPECT_GE(file.Fd(), 0);
+ EXPECT_TRUE(file.IsOpened());
+
+ const char* test_string = "This is a test string";
+ size_t length = strlen(test_string) + 1;
+ const size_t offset = 12;
+ std::unique_ptr<char[]> offset_read_string(new char[length]);
+ std::unique_ptr<char[]> read_string(new char[length]);
+
+ // Write scratch data to file that we can read back into.
+ EXPECT_TRUE(file.PwriteFully(test_string, length, offset));
+ ASSERT_EQ(file.Flush(), 0);
+
+ // Test reading both the offsets.
+ EXPECT_TRUE(file.PreadFully(&offset_read_string[0], length, offset));
+ EXPECT_STREQ(test_string, &offset_read_string[0]);
+
+ EXPECT_TRUE(file.PreadFully(&read_string[0], length, 0u));
+ EXPECT_NE(memcmp(&read_string[0], test_string, length), 0);
+
+ ASSERT_EQ(file.Close(), 0);
+}
+
TEST_F(FdFileTest, Copy) {
art::ScratchFile src_tmp;
FdFile src;