[Zucchini] Introduce image_index.

This CL adds ImageIndex, which stores annotation of an image file for
quick access to raw data and embedded references.
Note that some functionality from EncodedView from original implementation
have been moved into ImageIndex.

Bug: 729154
Change-Id: I1d5353fc8093543f2be0400361affa8b57333924
Reviewed-on: https://chromium-review.googlesource.com/589809
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491835}

chrome/installer/zucchini/BUILD.gn

@@ -19,6 +19,8 @@ static_library("zucchini_lib") {
     "disassembler.h",
     "disassembler_no_op.cc",
     "disassembler_no_op.h",
+    "image_index.cc",
+    "image_index.h",
     "image_utils.h",
     "io_utils.cc",
     "io_utils.h",
@@ -70,12 +72,15 @@ test("zucchini_unittests") {
     "buffer_source_unittest.cc",
     "buffer_view_unittest.cc",
     "crc32_unittest.cc",
+    "image_index_unittest.cc",
     "image_utils_unittest.cc",
     "io_utils_unittest.cc",
     "label_manager_unittest.cc",
     "patch_utils_unittest.cc",
     "rel32_utils_unittest.cc",
     "suffix_array_unittest.cc",
+    "test_reference_reader.cc",
+    "test_reference_reader.h",
     "typed_value_unittest.cc",
   ]
 

chrome/installer/zucchini/disassembler.h

@@ -9,7 +9,6 @@
 #include <string>
 #include <vector>
 
-#include "base/optional.h"
 #include "chrome/installer/zucchini/buffer_view.h"
 #include "chrome/installer/zucchini/image_utils.h"
 
@@ -17,30 +16,6 @@ namespace zucchini {
 
 class Disassembler;
 
-// Interface for extracting References through member function GetNext().
-// This is used by Disassemblers to extract references from an image file.
-// Typycally, a Reader lazily extracts values and does not hold any storage.
-class ReferenceReader {
- public:
-  virtual ~ReferenceReader() = default;
-
-  // Returns the next available Reference, or nullopt_t if exhausted.
-  // Extracted References must be ordered by their location in the image.
-  virtual base::Optional<Reference> GetNext() = 0;
-};
-
-// Interface for writing References through member function
-// PutNext(reference). This is used by Disassemblers to write new References
-// in the image file.
-class ReferenceWriter {
- public:
-  virtual ~ReferenceWriter() = default;
-
-  // Writes |reference| in the underlying image file. This operation always
-  // succeeds.
-  virtual void PutNext(Reference reference) = 0;
-};
-
 // A ReferenceGroup is associated with a specific |type| and has convenience
 // methods to obtain readers and writers for that type. A ReferenceGroup does
 // not store references; it is a lightweight class that communicates with the

chrome/installer/zucchini/image_index.cc

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/installer/zucchini/image_index.h"
+
+#include <algorithm>
+
+namespace zucchini {
+
+ImageIndex::ImageIndex(ConstBufferView image,
+                       std::vector<ReferenceTypeTraits>&& traits_map)
+    : raw_image_(image),
+      type_tags_(image.size(), kNoTypeTag),
+      types_(traits_map.size()) {
+  int pool_count = 0;
+  for (const auto& traits : traits_map) {
+    DCHECK_LT(traits.type_tag.value(), types_.size());
+    types_[traits.type_tag.value()].traits = traits;
+
+    DCHECK(kNoPoolTag != traits.pool_tag);
+    pool_count = std::max(pool_count, traits.pool_tag.value() + 1);
+  }
+  pools_.resize(pool_count);
+  for (const auto& traits : traits_map) {
+    pools_[traits.pool_tag.value()].types.push_back(traits.type_tag);
+  }
+}
+
+ImageIndex::~ImageIndex() = default;
+
+bool ImageIndex::InsertReferences(TypeTag type, ReferenceReader&& ref_reader) {
+  const ReferenceTypeTraits& traits = GetTraits(type);
+  for (base::Optional<Reference> ref = ref_reader.GetNext(); ref.has_value();
+       ref = ref_reader.GetNext()) {
+    DCHECK_LE(ref->location + traits.width, size());
+
+    // Check for overlap with existing reference. If found, then invalidate.
+    if (std::any_of(type_tags_.begin() + ref->location,
+                    type_tags_.begin() + ref->location + traits.width,
+                    [](TypeTag type) { return type != kNoTypeTag; }))
+      return false;
+    std::fill(type_tags_.begin() + ref->location,
+              type_tags_.begin() + ref->location + traits.width,
+              traits.type_tag);
+    types_[traits.type_tag.value()].references.push_back(*ref);
+  }
+  return true;
+}
+
+Reference ImageIndex::FindReference(TypeTag type, offset_t location) const {
+  DCHECK_LE(location, size());
+  DCHECK_LT(type.value(), types_.size());
+  auto pos = std::upper_bound(
+      types_[type.value()].references.begin(),
+      types_[type.value()].references.end(), location,
+      [](offset_t a, const Reference& ref) { return a < ref.location; });
+
+  DCHECK(pos != types_[type.value()].references.begin());
+  --pos;
+  DCHECK_LT(location, pos->location + GetTraits(type).width);
+  return *pos;
+}
+
+bool ImageIndex::IsToken(offset_t location) const {
+  TypeTag type = GetType(location);
+
+  // |location| points into raw data.
+  if (type == kNoTypeTag)
+    return true;
+
+  // |location| points into a Reference.
+  Reference reference = FindReference(type, location);
+  // Only the first byte of a reference is a token.
+  return location == reference.location;
+}
+
+void ImageIndex::LabelTargets(PoolTag pool,
+                              const BaseLabelManager& label_manager) {
+  for (const TypeTag& type : pools_[pool.value()].types)
+    for (auto& ref : types_[type.value()].references)
+      ref.target = label_manager.MarkedIndexFromOffset(ref.target);
+  pools_[pool.value()].label_bound = label_manager.size();
+}
+
+void ImageIndex::UnlabelTargets(PoolTag pool,
+                                const BaseLabelManager& label_manager) {
+  for (const TypeTag& type : pools_[pool.value()].types)
+    for (auto& ref : types_[type.value()].references) {
+      ref.target = label_manager.OffsetFromMarkedIndex(ref.target);
+      DCHECK(!IsMarked(ref.target));  // Expected to be represented as offset.
+    }
+  pools_[pool.value()].label_bound = 0;
+}
+
+void ImageIndex::LabelAssociatedTargets(
+    PoolTag pool,
+    const BaseLabelManager& label_manager,
+    const BaseLabelManager& reference_label_manager) {
+  // Convert to marked indexes.
+  for (const auto& type : pools_[pool.value()].types) {
+    for (auto& ref : types_[type.value()].references) {
+      // Represent Label as marked index iff the index is also in
+      // |reference_label_manager|.
+      DCHECK(!IsMarked(ref.target));  // Expected to be represented as offset.
+      offset_t index = label_manager.IndexOfOffset(ref.target);
+      DCHECK_NE(kUnusedIndex, index);  // Target is expected to have a label.
+      if (reference_label_manager.IsIndexStored(index))
+        ref.target = MarkIndex(index);
+    }
+  }
+  pools_[pool.value()].label_bound = label_manager.size();
+}
+
+ImageIndex::TypeInfo::TypeInfo() = default;
+ImageIndex::TypeInfo::TypeInfo(TypeInfo&&) = default;
+ImageIndex::TypeInfo::~TypeInfo() = default;
+
+ImageIndex::PoolInfo::PoolInfo() = default;
+ImageIndex::PoolInfo::PoolInfo(PoolInfo&&) = default;
+ImageIndex::PoolInfo::~PoolInfo() = default;
+
+}  // namespace zucchini

chrome/installer/zucchini/image_index.h

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_INSTALLER_ZUCCHINI_IMAGE_INDEX_H_
+#define CHROME_INSTALLER_ZUCCHINI_IMAGE_INDEX_H_
+
+#include <stdint.h>
+
+#include <vector>
+
+#include "base/logging.h"
+#include "base/macros.h"
+#include "base/optional.h"
+#include "chrome/installer/zucchini/buffer_view.h"
+#include "chrome/installer/zucchini/image_utils.h"
+#include "chrome/installer/zucchini/label_manager.h"
+
+namespace zucchini {
+
+// A class that holds annotations of an image, allowing quick access to its
+// raw and reference content. The memory overhead of storing all references is
+// relatively high, so this is only used during patch generation.
+class ImageIndex {
+ public:
+  ImageIndex(ConstBufferView image,
+             std::vector<ReferenceTypeTraits>&& traits_map);
+  ~ImageIndex();
+
+  // Inserts all references of type |type_tag| read from |ref_reader| to this
+  // index. This should be called exactly once for each reference type. If
+  // overlap between any two references of any type is encountered, returns
+  // false and leaves the object in an invalid state. Otherwise, returns true.
+  bool InsertReferences(TypeTag type_tag, ReferenceReader&& ref_reader);
+
+  // Returns the number of reference type the index holds.
+  size_t TypeCount() const { return types_.size(); }
+
+  // Returns the number of target pool discovered.
+  size_t PoolCount() const { return pools_.size(); }
+
+  size_t LabelBound(PoolTag pool) const {
+    DCHECK_LT(pool.value(), pools_.size());
+    return pools_[pool.value()].label_bound;
+  }
+
+  // Returns traits describing references of type |type|.
+  const ReferenceTypeTraits& GetTraits(TypeTag type) const {
+    DCHECK_LT(type.value(), types_.size());
+    return types_[type.value()].traits;
+  }
+
+  PoolTag GetPoolTag(TypeTag type) const { return GetTraits(type).pool_tag; }
+
+  // Returns true if |raw_image_[location]| is either:
+  // - A raw value.
+  // - The first byte of a reference.
+  bool IsToken(offset_t location) const;
+
+  // Returns true if |raw_image_[location]| is part of a reference.
+  bool IsReference(offset_t location) const {
+    return GetType(location) != kNoTypeTag;
+  }
+
+  // Returns the type tag of the reference covering |location|, or kNoTypeTag if
+  // |location| is not part of a reference.
+  TypeTag GetType(offset_t location) const {
+    DCHECK_EQ(type_tags_.size(), raw_image_.size());  // Sanity check.
+    DCHECK_LT(location, size());
+    return type_tags_[location];
+  }
+
+  // Returns the raw value at |location|.
+  uint8_t GetRawValue(offset_t location) const {
+    DCHECK_LT(location, size());
+    return raw_image_[location];
+  }
+
+  // Returns the reference of type |type| covering |location|, which is expected
+  // to exist.
+  Reference FindReference(TypeTag type, offset_t location) const;
+
+  // Returns a vector of references of type |type|, where references are sorted
+  // by their location.
+  const std::vector<Reference>& GetReferences(TypeTag type) const {
+    DCHECK_LT(type.value(), types_.size());
+    return types_[type.value()].references;
+  }
+
+  // Returns the size of the image.
+  size_t size() const { return raw_image_.size(); }
+
+  // Replaces every target represented as offset whose Label is in
+  // |label_manager| by the index of this Label, and updates the Label bound
+  // associated with |pool|.
+  void LabelTargets(PoolTag pool, const BaseLabelManager& label_manager);
+
+  // Replaces every associated target represented as offset whose Label is in
+  // |label_manager| by the index of this Label, and updates the Label bound
+  // associated with |pool|. A target is associated iff its Label index is also
+  // used in |reference_label_manager|. All targets must have a Label in
+  // |label_manager|, and must be represented as offset when calling this
+  // function, implying it can only be called once for each |pool|, until
+  // UnlabelTargets() (below) is called.
+  void LabelAssociatedTargets(PoolTag pool,
+                              const BaseLabelManager& label_manager,
+                              const BaseLabelManager& reference_label_manager);
+
+  // Replaces every target represented as a Label index by its original offset,
+  // assuming that |label_manager| still holds the same Labels referered to by
+  // target indices. Resets Label bound associated with |pool| to 0.
+  void UnlabelTargets(PoolTag pool, const BaseLabelManager& label_manager);
+
+ private:
+  // Information stored for every reference type.
+  struct TypeInfo {
+    TypeInfo();
+    TypeInfo(TypeInfo&&);
+    ~TypeInfo();
+
+    ReferenceTypeTraits traits;
+    // List of Reference instances sorted by location (file offset).
+    std::vector<Reference> references;
+  };
+  // Information stored for every reference pool.
+  struct PoolInfo {
+    PoolInfo();
+    PoolInfo(PoolInfo&&);
+    ~PoolInfo();
+
+    std::vector<TypeTag> types;  // Enumerates type_tag for this pool.
+    size_t label_bound = 0;      // Upper bound on Label indices for this pool.
+  };
+
+  const ConstBufferView raw_image_;
+
+  // Used for random access lookup of reference type, for each byte in
+  // |raw_image_|.
+  std::vector<TypeTag> type_tags_;
+
+  std::vector<TypeInfo> types_;
+  std::vector<PoolInfo> pools_;
+
+  DISALLOW_COPY_AND_ASSIGN(ImageIndex);
+};
+
+}  // namespace zucchini
+
+#endif  // CHROME_INSTALLER_ZUCCHINI_IMAGE_INDEX_H_

chrome/installer/zucchini/image_index_unittest.cc

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/installer/zucchini/image_index.h"
+
+#include <stddef.h>
+
+#include <numeric>
+#include <vector>
+
+#include "base/test/gtest_util.h"
+#include "chrome/installer/zucchini/image_utils.h"
+#include "chrome/installer/zucchini/test_reference_reader.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace zucchini {
+
+class ImageIndexTest : public testing::Test {
+ protected:
+  ImageIndexTest()
+      : buffer_(20),
+        image_index_(ConstBufferView(buffer_.data(), buffer_.size()),
+                     {ReferenceTypeTraits{2, TypeTag(0), PoolTag(0)},
+                      ReferenceTypeTraits{4, TypeTag(1), PoolTag(0)},
+                      ReferenceTypeTraits{3, TypeTag(2), PoolTag(1)}}) {
+    std::iota(buffer_.begin(), buffer_.end(), 0);
+  }
+
+  void InsertReferences0() {
+    EXPECT_TRUE(image_index_.InsertReferences(
+        TypeTag(0), TestReferenceReader({{1, 0}, {8, 1}, {10, 2}})));
+  }
+  void InsertReferences1() {
+    EXPECT_TRUE(image_index_.InsertReferences(TypeTag(1),
+                                              TestReferenceReader({{3, 3}})));
+  }
+  void InsertReferences2() {
+    EXPECT_TRUE(image_index_.InsertReferences(
+        TypeTag(2), TestReferenceReader({{12, 4}, {17, 5}})));
+  }
+  void InsertAllReferences() {
+    InsertReferences0();
+    InsertReferences1();
+    InsertReferences2();
+  }
+
+  std::vector<uint8_t> buffer_;
+  ImageIndex image_index_;
+};
+
+TEST_F(ImageIndexTest, TypeAndPool) {
+  InsertAllReferences();
+
+  EXPECT_EQ(3U, image_index_.TypeCount());
+  EXPECT_EQ(2U, image_index_.PoolCount());
+
+  EXPECT_EQ(PoolTag(0), image_index_.GetPoolTag(TypeTag(0)));
+  EXPECT_EQ(PoolTag(0), image_index_.GetPoolTag(TypeTag(1)));
+  EXPECT_EQ(PoolTag(1), image_index_.GetPoolTag(TypeTag(2)));
+
+  EXPECT_EQ(0U, image_index_.LabelBound(PoolTag(0)));
+  EXPECT_EQ(0U, image_index_.LabelBound(PoolTag(1)));
+}
+
+TEST_F(ImageIndexTest, InvalidInsertReferences1) {
+  // Overlap within the same reader.
+  EXPECT_FALSE(image_index_.InsertReferences(
+      TypeTag(0), TestReferenceReader({{1, 0}, {2, 0}})));
+}
+
+TEST_F(ImageIndexTest, InvalidInsertReferences2) {
+  InsertReferences0();
+  InsertReferences1();
+
+  // Overlap across different readers.
+  EXPECT_FALSE(image_index_.InsertReferences(TypeTag(2),
+                                             TestReferenceReader({{11, 0}})));
+}
+
+TEST_F(ImageIndexTest, GetType) {
+  InsertAllReferences();
+
+  std::vector<int> expected = {
+      -1,            // raw
+      0,  0,         // ref 0
+      1,  1,  1, 1,  // ref 1
+      -1,            // raw
+      0,  0,         // ref 0
+      0,  0,         // ref 0
+      2,  2,  2,     // ref 2
+      -1, -1,        // raw
+      2,  2,  2,     // ref 0
+  };
+
+  for (offset_t i = 0; i < image_index_.size(); ++i)
+    EXPECT_EQ(TypeTag(expected[i]), image_index_.GetType(i));
+}
+
+TEST_F(ImageIndexTest, IsToken) {
+  InsertAllReferences();
+
+  std::vector<bool> expected = {
+      1,           // raw
+      1, 0,        // ref 0
+      1, 0, 0, 0,  // ref 1
+      1,           // raw
+      1, 0,        // ref 0
+      1, 0,        // ref 0
+      1, 0, 0,     // ref 2
+      1, 1,        // raw
+      1, 0, 0,     // ref 0
+  };
+
+  for (offset_t i = 0; i < image_index_.size(); ++i)
+    EXPECT_EQ(expected[i], image_index_.IsToken(i));
+}
+
+TEST_F(ImageIndexTest, IsReference) {
+  InsertAllReferences();
+
+  std::vector<bool> expected = {
+      0,           // raw
+      1, 1,        // ref 0
+      1, 1, 1, 1,  // ref 1
+      0,           // raw
+      1, 1,        // ref 0
+      1, 1,        // ref 0
+      1, 1, 1,     // ref 2
+      0, 0,        // raw
+      1, 1, 1,     // ref 0
+  };
+
+  for (offset_t i = 0; i < image_index_.size(); ++i) {
+    EXPECT_EQ(expected[i], image_index_.IsReference(i));
+  }
+}
+
+TEST_F(ImageIndexTest, FindReference) {
+  InsertAllReferences();
+
+  EXPECT_DCHECK_DEATH(image_index_.FindReference(TypeTag(0), 0));
+  EXPECT_EQ(Reference({1, 0}), image_index_.FindReference(TypeTag(0), 1));
+  EXPECT_EQ(Reference({1, 0}), image_index_.FindReference(TypeTag(0), 2));
+  EXPECT_DCHECK_DEATH(image_index_.FindReference(TypeTag(0), 3));
+  EXPECT_DCHECK_DEATH(image_index_.FindReference(TypeTag(1), 1));
+  EXPECT_DCHECK_DEATH(image_index_.FindReference(TypeTag(1), 2));
+  EXPECT_EQ(Reference({10, 2}), image_index_.FindReference(TypeTag(0), 10));
+  EXPECT_EQ(Reference({10, 2}), image_index_.FindReference(TypeTag(0), 11));
+  EXPECT_DCHECK_DEATH(image_index_.FindReference(TypeTag(0), 12));
+}
+
+TEST_F(ImageIndexTest, LabelTargets) {
+  InsertAllReferences();
+
+  OrderedLabelManager label_manager0;
+  label_manager0.InsertOffsets({0, 2, 3, 4});
+  image_index_.LabelTargets(PoolTag(0), label_manager0);
+  EXPECT_EQ(4U, image_index_.LabelBound(PoolTag(0)));
+  EXPECT_EQ(0U, image_index_.LabelBound(PoolTag(1)));
+
+  EXPECT_EQ(
+      std::vector<Reference>({{1, MarkIndex(0)}, {8, 1}, {10, MarkIndex(1)}}),
+      image_index_.GetReferences(TypeTag(0)));
+  EXPECT_EQ(std::vector<Reference>({{3, MarkIndex(2)}}),
+            image_index_.GetReferences(TypeTag(1)));
+  EXPECT_EQ(std::vector<Reference>({{12, 4}, {17, 5}}),
+            image_index_.GetReferences(TypeTag(2)));
+
+  OrderedLabelManager label_manager1;
+  label_manager1.InsertOffsets({5});
+  image_index_.LabelTargets(PoolTag(1), label_manager1);
+  EXPECT_EQ(4U, image_index_.LabelBound(PoolTag(0)));
+  EXPECT_EQ(1U, image_index_.LabelBound(PoolTag(1)));
+
+  EXPECT_EQ(std::vector<Reference>({{12, 4}, {17, MarkIndex(0)}}),
+            image_index_.GetReferences(TypeTag(2)));
+
+  image_index_.UnlabelTargets(PoolTag(0), label_manager0);
+  EXPECT_EQ(0U, image_index_.LabelBound(PoolTag(0)));
+  EXPECT_EQ(1U, image_index_.LabelBound(PoolTag(1)));
+
+  EXPECT_EQ(std::vector<Reference>({{1, 0}, {8, 1}, {10, 2}}),
+            image_index_.GetReferences(TypeTag(0)));
+  EXPECT_EQ(std::vector<Reference>({{3, 3}}),
+            image_index_.GetReferences(TypeTag(1)));
+  EXPECT_EQ(std::vector<Reference>({{12, 4}, {17, MarkIndex(0)}}),
+            image_index_.GetReferences(TypeTag(2)));
+}
+
+TEST_F(ImageIndexTest, LabelAssociatedTargets) {
+  InsertAllReferences();
+
+  OrderedLabelManager label_manager;
+  label_manager.InsertOffsets({0, 1, 2, 3, 4});
+
+  UnorderedLabelManager reference_label_manager;
+  reference_label_manager.Init({0, kUnusedIndex, 2});
+
+  image_index_.LabelAssociatedTargets(PoolTag(0), label_manager,
+                                      reference_label_manager);
+  EXPECT_EQ(5U, image_index_.LabelBound(PoolTag(0)));
+  EXPECT_EQ(0U, image_index_.LabelBound(PoolTag(1)));
+  EXPECT_EQ(
+      std::vector<Reference>({{1, MarkIndex(0)}, {8, 1}, {10, MarkIndex(2)}}),
+      image_index_.GetReferences(TypeTag(0)));
+  EXPECT_EQ(std::vector<Reference>({{3, 3}}),
+            image_index_.GetReferences(TypeTag(1)));
+  EXPECT_EQ(std::vector<Reference>({{12, 4}, {17, 5}}),
+            image_index_.GetReferences(TypeTag(2)));
+}
+
+}  // namespace zucchini

chrome/installer/zucchini/image_utils.h

@@ -7,6 +7,7 @@
 
 #include <stdint.h>
 
+#include "base/optional.h"
 #include "chrome/installer/zucchini/typed_value.h"
 
 namespace zucchini {
@@ -33,14 +34,15 @@ constexpr PoolTag kNoPoolTag(0xFF);
 
 // Specification of references in an image file.
 struct ReferenceTypeTraits {
+  constexpr ReferenceTypeTraits() = default;
   constexpr ReferenceTypeTraits(offset_t width,
                                 TypeTag type_tag,
                                 PoolTag pool_tag)
       : width(width), type_tag(type_tag), pool_tag(pool_tag) {}
 
   // |width| specifies number of bytes covered by the reference's binary
-  // |encoding.
-  offset_t width;
+  // encoding.
+  offset_t width = 0;
   // |type_tag| identifies the reference type being described.
   TypeTag type_tag = kNoTypeTag;
   // |pool_tag| identifies the pool this type belongs to.
@@ -59,6 +61,30 @@ inline bool operator==(const Reference& a, const Reference& b) {
   return a.location == b.location && a.target == b.target;
 }
 
+// Interface for extracting References through member function GetNext().
+// This is used by Disassemblers to extract references from an image file.
+// Typically, a Reader lazily extracts values and does not hold any storage.
+class ReferenceReader {
+ public:
+  virtual ~ReferenceReader() = default;
+
+  // Returns the next available Reference, or nullopt_t if exhausted.
+  // Extracted References must be ordered by their location in the image.
+  virtual base::Optional<Reference> GetNext() = 0;
+};
+
+// Interface for writing References through member function
+// PutNext(reference). This is used by Disassemblers to write new References
+// in the image file.
+class ReferenceWriter {
+ public:
+  virtual ~ReferenceWriter() = default;
+
+  // Writes |reference| in the underlying image file. This operation always
+  // succeeds.
+  virtual void PutNext(Reference reference) = 0;
+};
+
 // Helper functions to mark an offset_t, so we can distinguish file offsets from
 // Label indexes. Implementation: Marking is flagged by the most significant bit
 // (MSB).

chrome/installer/zucchini/label_manager.cc

@@ -8,7 +8,6 @@
 #include <utility>
 
 #include "base/logging.h"
-#include "chrome/installer/zucchini/disassembler.h"
 
 namespace zucchini {
 
@@ -67,8 +66,8 @@ void OrderedLabelManager::InsertOffsets(const std::vector<offset_t>& offsets) {
   SortAndUniquify(&labels_);
 }
 
-void OrderedLabelManager::InsertTargets(ReferenceReader* reader) {
-  for (auto ref = reader->GetNext(); ref.has_value(); ref = reader->GetNext())
+void OrderedLabelManager::InsertTargets(ReferenceReader&& reader) {
+  for (auto ref = reader.GetNext(); ref.has_value(); ref = reader.GetNext())
     labels_.push_back(ref->target);
   SortAndUniquify(&labels_);
 }

chrome/installer/zucchini/label_manager.h

@@ -14,15 +14,13 @@
 
 namespace zucchini {
 
-class ReferenceReader;
-
 // A LabelManager stores a list of Labels. By definition, all offsets and
 // indices must be distinct. It also provides functions to:
 // - Get the offset of a stored index.
 // - Get the index of a stored offset.
-// - Creates new Labels.
+// - Create new Labels.
 // A LabelManager allows to have a bijection between offsets and indexes.
-// Since not all targets have assocatied labels from LabelManager, we need mixed
+// Since not all targets have associated labels from LabelManager, we need mixed
 // representation of targets as offsets or indexes. Hence, indexes are
 // represented as "marked" values (implemented by setting the MSB), and offsets
 // are "unmarked". So when working with stored targets:
@@ -37,9 +35,7 @@ class ReferenceReader;
 constexpr offset_t kUnusedIndex = offset_t(-1);
 static_assert(IsMarked(kUnusedIndex), "kUnusedIndex must be marked");
 
-// Base class for OrderedLabelManager and UnorderedLabelManager. We're not
-// making common functions virtual, since polymorphism is unused and so we may
-// as well avoid incurring the performance hit.
+// Base class for OrderedLabelManager and UnorderedLabelManager.
 class BaseLabelManager {
  public:
   BaseLabelManager();
@@ -58,13 +54,13 @@ class BaseLabelManager {
            IsMarked(offset_or_marked_index);
   }
 
-  // Returns whether a given (unmarked) |index| is used by a stored label.
+  // Returns whether a given (unmarked) |index| is used by a stored Label.
   bool IsIndexStored(offset_t index) const {
     return index < labels_.size() && labels_[index] != kUnusedIndex;
   }
 
   // Returns the offset of a given (unmarked) |index| if it is associated to a
-  // stored label, or |kUnusedIndex| otherwise.
+  // stored Label, or |kUnusedIndex| otherwise.
   offset_t OffsetOfIndex(offset_t index) const {
     return index < labels_.size() ? labels_[index] : kUnusedIndex;
   }
@@ -73,6 +69,16 @@ class BaseLabelManager {
   // target that is stored either as a marked index, or as an (unmarked) offset.
   offset_t OffsetFromMarkedIndex(offset_t offset_or_marked_index) const;
 
+  // If |offset| has an associated stored Label, returns its index. Otherwise
+  // returns |kUnusedIndex|.
+  virtual offset_t IndexOfOffset(offset_t offset) const = 0;
+
+  // Returns the marked index corresponding to |offset_or_marked_index|, which
+  // is a target that is stored either as a marked index, or as an (unmarked)
+  // offset.
+  virtual offset_t MarkedIndexFromOffset(
+      offset_t offset_or_marked_index) const = 0;
+
   size_t size() const { return labels_.size(); }
 
  protected:
@@ -92,22 +98,18 @@ class OrderedLabelManager : public BaseLabelManager {
   OrderedLabelManager(const OrderedLabelManager&);
   ~OrderedLabelManager() override;
 
-  // If |offset| has an associated stored label, returns its index. Otherwise
-  // returns |kUnusedIndex|.
-  offset_t IndexOfOffset(offset_t offset) const;
-
-  // Returns the marked index corresponding to |offset_or_marked_index|, which
-  // is a target that is stored either as a marked index, or as an (unmarked)
-  // offset.
-  offset_t MarkedIndexFromOffset(offset_t offset_or_marked_index) const;
+  // BaseLabelManager:
+  offset_t IndexOfOffset(offset_t offset) const override;
+  offset_t MarkedIndexFromOffset(
+      offset_t offset_or_marked_index) const override;
 
-  // Creates and stores a new label for each unique offset in |offsets|. This
+  // Creates and stores a new Label for each unique offset in |offsets|. This
   // invalidates all previous Label lookups.
   void InsertOffsets(const std::vector<offset_t>& offsets);
 
-  // For each unique target from |reader|, creates and stores a new label. This
+  // For each unique target from |reader|, creates and stores a new Label. This
   // invalidates all previous Label lookups.
-  void InsertTargets(ReferenceReader* reader);
+  void InsertTargets(ReferenceReader&& reader);
 
   const std::vector<offset_t>& Labels() const { return labels_; }
 };
@@ -124,21 +126,18 @@ class UnorderedLabelManager : public BaseLabelManager {
   UnorderedLabelManager(const UnorderedLabelManager&);
   ~UnorderedLabelManager() override;
 
-  // If |offset| is stored, returns its index. Otherwise returns |kUnusedIndex|.
-  offset_t IndexOfOffset(offset_t offset) const;
-
-  // Returns the marked index corresponding to |offset_or_marked_index|, which
-  // is a target that is stored either as a marked index, or as an (unmarked)
-  // offset.
-  offset_t MarkedIndexFromOffset(offset_t offset_or_marked_index) const;
+  // BaseLabelManager:
+  offset_t IndexOfOffset(offset_t offset) const override;
+  offset_t MarkedIndexFromOffset(
+      offset_t offset_or_marked_index) const override;
 
   // Clears and reinitializes all stored data. Requires that |labels| consists
   // of unique offsets, but it may have "gaps" in the form of |kUnusedIndex|.
   void Init(std::vector<offset_t>&& labels);
 
-  // Creates a new label for |offset|. Behavior is undefined if |offset| is
-  // already associated with a stored label. If |kUnusedIndex| gaps exist, tries
-  // to reused indices to create new labels, otherwise it allocates new indices.
+  // Creates a new Label for |offset|. Behavior is undefined if |offset| is
+  // already associated with a stored Label. If |kUnusedIndex| gaps exist, tries
+  // to reused indices to create new Labels, otherwise it allocates new indices.
   // Previous lookup results involving stored offsets / indexes remain valid.
   void InsertNewOffset(offset_t offset);
 

chrome/installer/zucchini/label_manager_unittest.cc

@@ -7,7 +7,7 @@
 #include <utility>
 #include <vector>
 
-#include "chrome/installer/zucchini/disassembler.h"
+#include "chrome/installer/zucchini/test_reference_reader.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace zucchini {
@@ -17,23 +17,6 @@ namespace {
 constexpr auto BAD = kUnusedIndex;
 using OffsetVector = std::vector<offset_t>;
 
-// A trivial ReferenceReader that only reads injected references.
-class TestReferenceReader : public ReferenceReader {
- public:
-  explicit TestReferenceReader(const std::vector<Reference>& refs)
-      : references_(refs) {}
-
-  base::Optional<Reference> GetNext() override {
-    if (index_ == references_.size())
-      return base::nullopt;
-    return references_[index_++];
-  }
-
- private:
-  std::vector<Reference> references_;
-  size_t index_ = 0;
-};
-
 }  // namespace
 
 TEST(LabelManagerTest, Ordered) {
@@ -81,13 +64,13 @@ TEST(LabelManagerTest, OrderedInsertTargets) {
 
   // Initialize with some data. |location| does not matter.
   TestReferenceReader reader1({{0, 0x33}, {1, 0x11}, {2, 0x44}, {3, 0x11}});
-  label_manager.InsertTargets(&reader1);
+  label_manager.InsertTargets(std::move(reader1));
   EXPECT_EQ(OffsetVector({0x11, 0x33, 0x44}), label_manager.Labels());
 
   // Insert more data.
   TestReferenceReader reader2(
       {{0, 0x66}, {1, 0x11}, {2, 0x11}, {3, 0x44}, {4, 0x00}});
-  label_manager.InsertTargets(&reader2);
+  label_manager.InsertTargets(std::move(reader2));
   EXPECT_EQ(OffsetVector({0x00, 0x11, 0x33, 0x44, 0x66}),
             label_manager.Labels());
 }

chrome/installer/zucchini/test_reference_reader.cc

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/installer/zucchini/test_reference_reader.h"
+
+namespace zucchini {
+
+TestReferenceReader::TestReferenceReader(const std::vector<Reference>& refs)
+    : references_(refs) {}
+
+TestReferenceReader::~TestReferenceReader() = default;
+
+base::Optional<Reference> TestReferenceReader::GetNext() {
+  if (index_ == references_.size())
+    return base::nullopt;
+  return references_[index_++];
+}
+
+}  // namespace zucchini

chrome/installer/zucchini/test_reference_reader.h

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_INSTALLER_ZUCCHINI_TEST_REFERENCE_READER_H_
+#define CHROME_INSTALLER_ZUCCHINI_TEST_REFERENCE_READER_H_
+
+#include <stddef.h>
+
+#include <vector>
+
+#include "base/optional.h"
+#include "chrome/installer/zucchini/image_utils.h"
+
+namespace zucchini {
+
+// A trivial ReferenceReader that reads injected references.
+class TestReferenceReader : public ReferenceReader {
+ public:
+  explicit TestReferenceReader(const std::vector<Reference>& refs);
+  ~TestReferenceReader() override;
+
+  base::Optional<Reference> GetNext() override;
+
+ private:
+  std::vector<Reference> references_;
+  size_t index_ = 0;
+};
+
+}  // namespace zucchini
+
+#endif  // CHROME_INSTALLER_ZUCCHINI_TEST_REFERENCE_READER_H_

chrome/installer/zucchini/typed_value.h

@@ -34,6 +34,12 @@ class TypedValue {
   friend bool operator!=(const TypedValue& a, const TypedValue& b) {
     return !(a == b);
   }
+  friend bool operator<(const TypedValue& a, const TypedValue& b) {
+    return a.value_ < b.value_;
+  }
+  friend bool operator>(const TypedValue& a, const TypedValue& b) {
+    return b < a;
+  }
 
  private:
   T value_ = {};