aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTianjie Xu2018-02-22 17:40:39 -0600
committerTianjie Xu2018-02-23 12:44:05 -0600
commit572abbb81cfa12cddf742fa35cd8a4b9eebdc7d1 (patch)
treeacef858937e69879b8d122c2d20834347c095e3f
parentbf52b7e00b63397d9c81b7f7ffa7a8b8799edd4a (diff)
downloadplatform-bootable-recovery-572abbb81cfa12cddf742fa35cd8a4b9eebdc7d1.tar.gz
platform-bootable-recovery-572abbb81cfa12cddf742fa35cd8a4b9eebdc7d1.tar.xz
platform-bootable-recovery-572abbb81cfa12cddf742fa35cd8a4b9eebdc7d1.zip
Remove the assumption of target chunk size in imgdiff
In the split mode of imgdiff, we used to assume that the size of a split target chunk is always greater than the blocksize i.e. 4096. This may lead to the following assertion failure: I0221 04:57:33.451323 818464 common.py:205 imgdiff F 02-21 04:57:33 821203 821203 imgdiff.cpp:999] Check failed: tgt_size >= BLOCK_SIZE (tgt_size=476, BLOCK_SIZE=4096) This CL removes the assumption and handles the edge cases. Test: generate and verify the incremental update for TFs in the bug; unit test passes Bug: 73757557 Bug: 73711365 Change-Id: Iadbb4ee658995f5856cd488f3793980881a59620
-rw-r--r--applypatch/imgdiff.cpp45
-rw-r--r--applypatch/include/applypatch/imgdiff_image.h5
-rw-r--r--tests/component/imgdiff_test.cpp101
3 files changed, 131 insertions, 20 deletions
diff --git a/applypatch/imgdiff.cpp b/applypatch/imgdiff.cpp
index 3dae63d4..674cc2b1 100644
--- a/applypatch/imgdiff.cpp
+++ b/applypatch/imgdiff.cpp
@@ -955,14 +955,17 @@ bool ZipModeImage::SplitZipModeImageWithLimit(const ZipModeImage& tgt_image,
955 tgt->GetRawDataLength()); 955 tgt->GetRawDataLength());
956 } 956 }
957 } else { 957 } else {
958 ZipModeImage::AddSplitImageFromChunkList(tgt_image, src_image, src_ranges, split_tgt_chunks, 958 bool added_image = ZipModeImage::AddSplitImageFromChunkList(
959 split_src_chunks, split_tgt_images, 959 tgt_image, src_image, src_ranges, split_tgt_chunks, split_src_chunks, split_tgt_images,
960 split_src_images); 960 split_src_images);
961 961
962 split_tgt_chunks.clear(); 962 split_tgt_chunks.clear();
963 split_src_chunks.clear(); 963 split_src_chunks.clear();
964 used_src_ranges.Insert(src_ranges); 964 // No need to update the split_src_ranges if we don't update the split source images.
965 split_src_ranges->push_back(std::move(src_ranges)); 965 if (added_image) {
966 used_src_ranges.Insert(src_ranges);
967 split_src_ranges->push_back(std::move(src_ranges));
968 }
966 src_ranges.Clear(); 969 src_ranges.Clear();
967 970
968 // We don't have enough space for the current chunk; start a new split image and handle 971 // We don't have enough space for the current chunk; start a new split image and handle
@@ -973,9 +976,12 @@ bool ZipModeImage::SplitZipModeImageWithLimit(const ZipModeImage& tgt_image,
973 976
974 // TODO Trim it in case the CD exceeds limit too much. 977 // TODO Trim it in case the CD exceeds limit too much.
975 src_ranges.Insert(central_directory->GetStartOffset(), central_directory->DataLengthForPatch()); 978 src_ranges.Insert(central_directory->GetStartOffset(), central_directory->DataLengthForPatch());
976 ZipModeImage::AddSplitImageFromChunkList(tgt_image, src_image, src_ranges, split_tgt_chunks, 979 bool added_image = ZipModeImage::AddSplitImageFromChunkList(tgt_image, src_image, src_ranges,
977 split_src_chunks, split_tgt_images, split_src_images); 980 split_tgt_chunks, split_src_chunks,
978 split_src_ranges->push_back(std::move(src_ranges)); 981 split_tgt_images, split_src_images);
982 if (added_image) {
983 split_src_ranges->push_back(std::move(src_ranges));
984 }
979 985
980 ValidateSplitImages(*split_tgt_images, *split_src_images, *split_src_ranges, 986 ValidateSplitImages(*split_tgt_images, *split_src_images, *split_src_ranges,
981 tgt_image.file_content_.size()); 987 tgt_image.file_content_.size());
@@ -983,7 +989,7 @@ bool ZipModeImage::SplitZipModeImageWithLimit(const ZipModeImage& tgt_image,
983 return true; 989 return true;
984} 990}
985 991
986void ZipModeImage::AddSplitImageFromChunkList(const ZipModeImage& tgt_image, 992bool ZipModeImage::AddSplitImageFromChunkList(const ZipModeImage& tgt_image,
987 const ZipModeImage& src_image, 993 const ZipModeImage& src_image,
988 const SortedRangeSet& split_src_ranges, 994 const SortedRangeSet& split_src_ranges,
989 const std::vector<ImageChunk>& split_tgt_chunks, 995 const std::vector<ImageChunk>& split_tgt_chunks,
@@ -991,12 +997,6 @@ void ZipModeImage::AddSplitImageFromChunkList(const ZipModeImage& tgt_image,
991 std::vector<ZipModeImage>* split_tgt_images, 997 std::vector<ZipModeImage>* split_tgt_images,
992 std::vector<ZipModeImage>* split_src_images) { 998 std::vector<ZipModeImage>* split_src_images) {
993 CHECK(!split_tgt_chunks.empty()); 999 CHECK(!split_tgt_chunks.empty());
994 // Target chunks should occupy at least one block.
995 // TODO put a warning and change the type to raw if it happens in extremely rare cases.
996 size_t tgt_size = split_tgt_chunks.back().GetStartOffset() +
997 split_tgt_chunks.back().DataLengthForPatch() -
998 split_tgt_chunks.front().GetStartOffset();
999 CHECK_GE(tgt_size, BLOCK_SIZE);
1000 1000
1001 std::vector<ImageChunk> aligned_tgt_chunks; 1001 std::vector<ImageChunk> aligned_tgt_chunks;
1002 1002
@@ -1015,7 +1015,12 @@ void ZipModeImage::AddSplitImageFromChunkList(const ZipModeImage& tgt_image,
1015 1015
1016 i++; 1016 i++;
1017 } 1017 }
1018 CHECK_LT(i, split_tgt_chunks.size()); 1018
1019 // Nothing left after alignment in the current split tgt chunks; skip adding the split_tgt_image.
1020 if (i == split_tgt_chunks.size()) {
1021 return false;
1022 }
1023
1019 aligned_tgt_chunks.insert(aligned_tgt_chunks.end(), split_tgt_chunks.begin() + i + 1, 1024 aligned_tgt_chunks.insert(aligned_tgt_chunks.end(), split_tgt_chunks.begin() + i + 1,
1020 split_tgt_chunks.end()); 1025 split_tgt_chunks.end());
1021 CHECK(!aligned_tgt_chunks.empty()); 1026 CHECK(!aligned_tgt_chunks.empty());
@@ -1024,8 +1029,10 @@ void ZipModeImage::AddSplitImageFromChunkList(const ZipModeImage& tgt_image,
1024 size_t end_offset = 1029 size_t end_offset =
1025 aligned_tgt_chunks.back().GetStartOffset() + aligned_tgt_chunks.back().GetRawDataLength(); 1030 aligned_tgt_chunks.back().GetStartOffset() + aligned_tgt_chunks.back().GetRawDataLength();
1026 if (end_offset % BLOCK_SIZE != 0 && end_offset < tgt_image.file_content_.size()) { 1031 if (end_offset % BLOCK_SIZE != 0 && end_offset < tgt_image.file_content_.size()) {
1032 size_t tail_block_length = std::min<size_t>(tgt_image.file_content_.size() - end_offset,
1033 BLOCK_SIZE - (end_offset % BLOCK_SIZE));
1027 aligned_tgt_chunks.emplace_back(CHUNK_NORMAL, end_offset, &tgt_image.file_content_, 1034 aligned_tgt_chunks.emplace_back(CHUNK_NORMAL, end_offset, &tgt_image.file_content_,
1028 BLOCK_SIZE - (end_offset % BLOCK_SIZE)); 1035 tail_block_length);
1029 } 1036 }
1030 1037
1031 ZipModeImage split_tgt_image(false); 1038 ZipModeImage split_tgt_image(false);
@@ -1049,6 +1056,8 @@ void ZipModeImage::AddSplitImageFromChunkList(const ZipModeImage& tgt_image,
1049 1056
1050 split_tgt_images->push_back(std::move(split_tgt_image)); 1057 split_tgt_images->push_back(std::move(split_tgt_image));
1051 split_src_images->push_back(std::move(split_src_image)); 1058 split_src_images->push_back(std::move(split_src_image));
1059
1060 return true;
1052} 1061}
1053 1062
1054void ZipModeImage::ValidateSplitImages(const std::vector<ZipModeImage>& split_tgt_images, 1063void ZipModeImage::ValidateSplitImages(const std::vector<ZipModeImage>& split_tgt_images,
@@ -1536,7 +1545,7 @@ int imgdiff(int argc, const char** argv) {
1536 " patches together and output them into <patch-file>.\n" 1545 " patches together and output them into <patch-file>.\n"
1537 " --split-info, Output the split information (patch_size, tgt_size, src_ranges);\n" 1546 " --split-info, Output the split information (patch_size, tgt_size, src_ranges);\n"
1538 " zip mode with block-limit only.\n" 1547 " zip mode with block-limit only.\n"
1539 " --debug_dir, Debug directory to put the split srcs and patches, zip mode only.\n" 1548 " --debug-dir, Debug directory to put the split srcs and patches, zip mode only.\n"
1540 " -v, --verbose, Enable verbose logging."; 1549 " -v, --verbose, Enable verbose logging.";
1541 return 2; 1550 return 2;
1542 } 1551 }
diff --git a/applypatch/include/applypatch/imgdiff_image.h b/applypatch/include/applypatch/imgdiff_image.h
index 0f74420f..08480723 100644
--- a/applypatch/include/applypatch/imgdiff_image.h
+++ b/applypatch/include/applypatch/imgdiff_image.h
@@ -265,8 +265,9 @@ class ZipModeImage : public Image {
265 std::vector<SortedRangeSet>& split_src_ranges, 265 std::vector<SortedRangeSet>& split_src_ranges,
266 size_t total_tgt_size); 266 size_t total_tgt_size);
267 // Construct the dummy split images based on the chunks info and source ranges; and move them into 267 // Construct the dummy split images based on the chunks info and source ranges; and move them into
268 // the given vectors. 268 // the given vectors. Return true if we add a new split image into |split_tgt_images|, and
269 static void AddSplitImageFromChunkList(const ZipModeImage& tgt_image, 269 // false otherwise.
270 static bool AddSplitImageFromChunkList(const ZipModeImage& tgt_image,
270 const ZipModeImage& src_image, 271 const ZipModeImage& src_image,
271 const SortedRangeSet& split_src_ranges, 272 const SortedRangeSet& split_src_ranges,
272 const std::vector<ImageChunk>& split_tgt_chunks, 273 const std::vector<ImageChunk>& split_tgt_chunks,
diff --git a/tests/component/imgdiff_test.cpp b/tests/component/imgdiff_test.cpp
index 728b6cc7..6c23def0 100644
--- a/tests/component/imgdiff_test.cpp
+++ b/tests/component/imgdiff_test.cpp
@@ -969,3 +969,104 @@ TEST(ImgdiffTest, zip_mode_large_enough_limit) {
969 // Expect 1 piece of patch since limit is larger than the zip file size. 969 // Expect 1 piece of patch since limit is larger than the zip file size.
970 GenerateAndCheckSplitTarget(debug_dir.path, 1, tgt); 970 GenerateAndCheckSplitTarget(debug_dir.path, 1, tgt);
971} 971}
972
973TEST(ImgdiffTest, zip_mode_large_apk_small_target_chunk) {
974 TemporaryFile tgt_file;
975 FILE* tgt_file_ptr = fdopen(tgt_file.release(), "wb");
976 ZipWriter tgt_writer(tgt_file_ptr);
977
978 // The first entry is less than 4096 bytes, followed immediately by an entry that has a very
979 // large counterpart in the source file. Therefore the first entry will be patched separately.
980 std::string small_chunk("a", 2000);
981 ASSERT_EQ(0, tgt_writer.StartEntry("a", 0));
982 ASSERT_EQ(0, tgt_writer.WriteBytes(small_chunk.data(), small_chunk.size()));
983 ASSERT_EQ(0, tgt_writer.FinishEntry());
984 construct_store_entry(
985 {
986 { "b", 12, 'b' }, { "c", 3, 'c' },
987 },
988 &tgt_writer);
989 ASSERT_EQ(0, tgt_writer.Finish());
990 ASSERT_EQ(0, fclose(tgt_file_ptr));
991
992 TemporaryFile src_file;
993 FILE* src_file_ptr = fdopen(src_file.release(), "wb");
994 ZipWriter src_writer(src_file_ptr);
995 construct_store_entry({ { "a", 1, 'a' }, { "b", 13, 'b' }, { "c", 1, 'c' } }, &src_writer);
996 ASSERT_EQ(0, src_writer.Finish());
997 ASSERT_EQ(0, fclose(src_file_ptr));
998
999 // Compute patch.
1000 TemporaryFile patch_file;
1001 TemporaryFile split_info_file;
1002 TemporaryDir debug_dir;
1003 std::string split_info_arg = android::base::StringPrintf("--split-info=%s", split_info_file.path);
1004 std::string debug_dir_arg = android::base::StringPrintf("--debug-dir=%s", debug_dir.path);
1005 std::vector<const char*> args = {
1006 "imgdiff", "-z", "--block-limit=10", split_info_arg.c_str(), debug_dir_arg.c_str(),
1007 src_file.path, tgt_file.path, patch_file.path,
1008 };
1009 ASSERT_EQ(0, imgdiff(args.size(), args.data()));
1010
1011 std::string tgt;
1012 ASSERT_TRUE(android::base::ReadFileToString(tgt_file.path, &tgt));
1013
1014 // Expect three split src images:
1015 // src_piece 0: a 1 blocks
1016 // src_piece 1: b-0 10 blocks
1017 // src_piece 2: b-1 3 blocks, c 1 blocks, CD
1018 GenerateAndCheckSplitTarget(debug_dir.path, 3, tgt);
1019}
1020
1021TEST(ImgdiffTest, zip_mode_large_apk_skipped_small_target_chunk) {
1022 TemporaryFile tgt_file;
1023 FILE* tgt_file_ptr = fdopen(tgt_file.release(), "wb");
1024 ZipWriter tgt_writer(tgt_file_ptr);
1025
1026 construct_store_entry(
1027 {
1028 { "a", 11, 'a' },
1029 },
1030 &tgt_writer);
1031
1032 // Construct a tiny target entry of 1 byte, which will be skipped due to the tail alignment of
1033 // the previous entry.
1034 std::string small_chunk("b", 1);
1035 ASSERT_EQ(0, tgt_writer.StartEntry("b", 0));
1036 ASSERT_EQ(0, tgt_writer.WriteBytes(small_chunk.data(), small_chunk.size()));
1037 ASSERT_EQ(0, tgt_writer.FinishEntry());
1038
1039 ASSERT_EQ(0, tgt_writer.Finish());
1040 ASSERT_EQ(0, fclose(tgt_file_ptr));
1041
1042 TemporaryFile src_file;
1043 FILE* src_file_ptr = fdopen(src_file.release(), "wb");
1044 ZipWriter src_writer(src_file_ptr);
1045 construct_store_entry(
1046 {
1047 { "a", 11, 'a' }, { "b", 11, 'b' },
1048 },
1049 &src_writer);
1050 ASSERT_EQ(0, src_writer.Finish());
1051 ASSERT_EQ(0, fclose(src_file_ptr));
1052
1053 // Compute patch.
1054 TemporaryFile patch_file;
1055 TemporaryFile split_info_file;
1056 TemporaryDir debug_dir;
1057 std::string split_info_arg = android::base::StringPrintf("--split-info=%s", split_info_file.path);
1058 std::string debug_dir_arg = android::base::StringPrintf("--debug-dir=%s", debug_dir.path);
1059 std::vector<const char*> args = {
1060 "imgdiff", "-z", "--block-limit=10", split_info_arg.c_str(), debug_dir_arg.c_str(),
1061 src_file.path, tgt_file.path, patch_file.path,
1062 };
1063 ASSERT_EQ(0, imgdiff(args.size(), args.data()));
1064
1065 std::string tgt;
1066 ASSERT_TRUE(android::base::ReadFileToString(tgt_file.path, &tgt));
1067
1068 // Expect two split src images:
1069 // src_piece 0: a-0 10 blocks
1070 // src_piece 1: a-0 1 block, CD
1071 GenerateAndCheckSplitTarget(debug_dir.path, 2, tgt);
1072}