From 56a6ff47bca7087db1916b2beeedee283cd1caf2 Mon Sep 17 00:00:00 2001 From: Samuel Huang Date: Tue, 13 Mar 2018 21:35:53 +0000 Subject: [Zucchini] Zucchini-gen: Make number of CreateEquivalenceMap() generations depend on Disassembler. The number of CreateEquivalenceMap() iterations used be constant kNumIteraitons = 2. This CL makes the value depend on architecture. Current assignment: - DisassemblerNoOp: 1, since no pointers are identified (though in this case, CreateEquivalenceMap() should not be called). - DisassemblerWin32: 2. Upcoming DisassemblerDex will use 4. Also applying generic cleanups on headers and comments. Bug: 729154 Change-Id: Ia12d98fcba500e4c81c8a5d356ce4cadf424ffde Reviewed-on: https://chromium-review.googlesource.com/961273 Reviewed-by: agrieve Commit-Queue: Samuel Huang Cr-Commit-Position: refs/heads/master@{#542919} NOKEYCHECK=True GitOrigin-RevId: 55aea0a875b80e614464fdd157d9717471f9d64f --- disassembler.cc | 10 ++++++++-- disassembler.h | 10 ++++++++-- disassembler_no_op.cc | 4 +++- disassembler_win32.cc | 6 ++---- image_index.h | 4 ++-- main_utils.cc | 3 --- rel32_utils_unittest.cc | 1 - test_disassembler.cc | 5 ++++- zucchini_apply_unittest.cc | 10 +--------- zucchini_commands.cc | 1 - zucchini_gen.cc | 22 +++++++++++----------- zucchini_tools.cc | 1 + 12 files changed, 40 insertions(+), 37 deletions(-) diff --git a/disassembler.cc b/disassembler.cc index 18527a7..012e5ef 100644 --- a/disassembler.cc +++ b/disassembler.cc @@ -8,6 +8,8 @@ namespace zucchini { +/******** ReferenceGroup ********/ + std::unique_ptr ReferenceGroup::GetReader( offset_t lower, offset_t upper, @@ -25,12 +27,16 @@ std::unique_ptr ReferenceGroup::GetReader( std::unique_ptr ReferenceGroup::GetWriter( MutableBufferView image, Disassembler* disasm) const { - DCHECK_EQ(image.begin(), disasm->GetImage().begin()); + DCHECK_EQ(image.begin(), disasm->image().begin()); DCHECK_EQ(image.size(), disasm->size()); return (disasm->*writer_factory_)(image); } -Disassembler::Disassembler() = default; +/******** Disassembler ********/ + +Disassembler::Disassembler(int num_equivalence_iterations) + : num_equivalence_iterations_(num_equivalence_iterations) {} + Disassembler::~Disassembler() = default; } // namespace zucchini diff --git a/disassembler.h b/disassembler.h index 8d41eaa..eced3cf 100644 --- a/disassembler.h +++ b/disassembler.h @@ -111,11 +111,13 @@ class Disassembler { // Groups must be aggregated by pool. virtual std::vector MakeReferenceGroups() const = 0; - ConstBufferView GetImage() const { return image_; } + ConstBufferView image() const { return image_; } size_t size() const { return image_.size(); } + int num_equivalence_iterations() const { return num_equivalence_iterations_; } + protected: - Disassembler(); + explicit Disassembler(int num_equivalence_iterations); // Parses |image| and initializes internal states. Returns true on success. // This must be called once and before any other operation. @@ -125,6 +127,10 @@ class Disassembler { // only the portion containing the executable file it recognizes. ConstBufferView image_; + // The number of iterations to run for equivalence map generation. This should + // roughly be the max length of reference indirection chains. + int num_equivalence_iterations_; + DISALLOW_COPY_AND_ASSIGN(Disassembler); }; diff --git a/disassembler_no_op.cc b/disassembler_no_op.cc index 69d0eb8..3bc24d0 100644 --- a/disassembler_no_op.cc +++ b/disassembler_no_op.cc @@ -6,7 +6,9 @@ namespace zucchini { -DisassemblerNoOp::DisassemblerNoOp() = default; +// |num_equivalence_iterations_| = 1 since no pointers are present. +DisassemblerNoOp::DisassemblerNoOp() : Disassembler(1) {} + DisassemblerNoOp::~DisassemblerNoOp() = default; ExecutableType DisassemblerNoOp::GetExeType() const { diff --git a/disassembler_win32.cc b/disassembler_win32.cc index 5bdc503..a932bea 100644 --- a/disassembler_win32.cc +++ b/disassembler_win32.cc @@ -86,8 +86,9 @@ bool DisassemblerWin32::QuickDetect(ConstBufferView image) { return ReadWin32Header(image, &source); } +// |num_equivalence_iterations_| = 2 for reloc -> abs32. template -DisassemblerWin32::DisassemblerWin32() = default; +DisassemblerWin32::DisassemblerWin32() : Disassembler(2) {} template DisassemblerWin32::~DisassemblerWin32() = default; @@ -309,9 +310,6 @@ bool DisassemblerWin32::ParseAndStoreRelocBlocks() { &reloc_block_offsets_); } -// TODO(huangs): Print warning if too few abs32 references are found. -// Empirically, file size / # relocs is < 100, so take 200 as the -// threshold for warning. template bool DisassemblerWin32::ParseAndStoreAbs32() { if (has_parsed_abs32_) diff --git a/image_index.h b/image_index.h index 4f07015..6301b3a 100644 --- a/image_index.h +++ b/image_index.h @@ -43,14 +43,14 @@ class ImageIndex { size_t TypeCount() const { if (reference_sets_.empty()) return 0U; - return reference_sets_.rbegin()->first.value() + 1; + return reference_sets_.rbegin()->first.value() + 1; // Max key + 1. } // Returns the array size needed to accommodate all pool values. size_t PoolCount() const { if (target_pools_.empty()) return 0U; - return target_pools_.rbegin()->first.value() + 1; + return target_pools_.rbegin()->first.value() + 1; // Max key + 1. } // Returns true if |image_[location]| is either: diff --git a/main_utils.cc b/main_utils.cc index b874dd0..ee0332a 100644 --- a/main_utils.cc +++ b/main_utils.cc @@ -25,9 +25,6 @@ namespace { -#if defined(OS_WIN) -#endif - /******** Command ********/ // Specifications for a Zucchini command. diff --git a/rel32_utils_unittest.cc b/rel32_utils_unittest.cc index 80928de..6d90112 100644 --- a/rel32_utils_unittest.cc +++ b/rel32_utils_unittest.cc @@ -9,7 +9,6 @@ #include #include -#include "base/memory/ptr_util.h" #include "base/optional.h" #include "base/test/gtest_util.h" #include "components/zucchini/address_translator.h" diff --git a/test_disassembler.cc b/test_disassembler.cc index 8d59a93..2d6727b 100644 --- a/test_disassembler.cc +++ b/test_disassembler.cc @@ -8,13 +8,16 @@ namespace zucchini { +// |num_equivalence_iterations_| = 2 to cover common case for testing. TestDisassembler::TestDisassembler(const ReferenceTypeTraits& traits1, const std::vector& refs1, const ReferenceTypeTraits& traits2, const std::vector& refs2, const ReferenceTypeTraits& traits3, const std::vector& refs3) - : traits_{traits1, traits2, traits3}, refs_{refs1, refs2, refs3} {} + : Disassembler(2), + traits_{traits1, traits2, traits3}, + refs_{refs1, refs2, refs3} {} TestDisassembler::~TestDisassembler() = default; diff --git a/zucchini_apply_unittest.cc b/zucchini_apply_unittest.cc index 7e26b7b..f1cb853 100644 --- a/zucchini_apply_unittest.cc +++ b/zucchini_apply_unittest.cc @@ -4,19 +4,11 @@ #include "components/zucchini/zucchini_apply.h" -#include - #include "components/zucchini/image_utils.h" #include "testing/gtest/include/gtest/gtest.h" namespace zucchini { -namespace { - -using OffsetVector = std::vector; - -} // namespace - -// TODO(huangs): Add more tests. +// TODO(huangs): Add tests. } // namespace zucchini diff --git a/zucchini_commands.cc b/zucchini_commands.cc index 60b87cb..90208a0 100644 --- a/zucchini_commands.cc +++ b/zucchini_commands.cc @@ -7,7 +7,6 @@ #include #include -#include #include #include diff --git a/zucchini_gen.cc b/zucchini_gen.cc index 4be0b8b..0758e5a 100644 --- a/zucchini_gen.cc +++ b/zucchini_gen.cc @@ -33,7 +33,6 @@ namespace { // Parameters for patch generation. constexpr double kMinEquivalenceSimilarity = 12.0; constexpr double kMinLabelAffinity = 64.0; -constexpr size_t kNumIterations = 2; } // namespace @@ -46,21 +45,21 @@ std::vector FindExtraTargets(const TargetPool& projected_old_targets, return extra_targets; } +// Label matching (between "old" and "new") can guide EquivalenceMap +// construction; but EquivalenceMap induces Label matching. This apparent "chick +// and egg" problem is solved by alternating 2 steps |num_iterations| times: +// - Associate targets based on previous EquivalenceMap. Note on the first +// iteration, EquivalenceMap is empty, resulting in a no-op. +// - Construct refined EquivalenceMap based on new targets associations. EquivalenceMap CreateEquivalenceMap(const ImageIndex& old_image_index, - const ImageIndex& new_image_index) { - // Label matching (between "old" and "new") can guide EquivalenceMap - // construction; but EquivalenceMap induces Label matching. This apparent - // "chick and egg" problem is solved by multiple iterations alternating 2 - // steps: - // - Association of targets based on previous EquivalenceMap. Note that the - // EquivalenceMap is empty on first iteration, so this is a no-op. - // - Construction of refined EquivalenceMap based on new targets associations. + const ImageIndex& new_image_index, + int num_iterations) { size_t pool_count = old_image_index.PoolCount(); // |target_affinities| is outside the loop to reduce allocation. std::vector target_affinities(pool_count); EquivalenceMap equivalence_map; - for (size_t i = 0; i < kNumIterations; ++i) { + for (int i = 0; i < num_iterations; ++i) { EncodedView old_view(old_image_index); EncodedView new_view(new_image_index); @@ -259,7 +258,8 @@ bool GenerateExecutableElement(ExecutableType exe_type, DCHECK_EQ(old_image_index.PoolCount(), new_image_index.PoolCount()); EquivalenceMap equivalences = - CreateEquivalenceMap(old_image_index, new_image_index); + CreateEquivalenceMap(old_image_index, new_image_index, + new_disasm->num_equivalence_iterations()); OffsetMapper offset_mapper(equivalences); ReferenceDeltaSink reference_delta_sink; diff --git a/zucchini_tools.cc b/zucchini_tools.cc index 784e355..16eff2d 100644 --- a/zucchini_tools.cc +++ b/zucchini_tools.cc @@ -13,6 +13,7 @@ #include #include "base/bind.h" +#include "base/logging.h" #include "base/strings/stringprintf.h" #include "components/zucchini/disassembler.h" #include "components/zucchini/element_detection.h" -- cgit v1.2.3