From 07c31a327fc75fd62023d1774e900f85ce982ee7 Mon Sep 17 00:00:00 2001 From: Calder Kitagawa Date: Tue, 10 Apr 2018 20:03:08 +0000 Subject: [Zucchini] Remove Marking Logic With recent refactoring by etiennep@ marking high bits is no longer required. This removes checks for existing high bits which were previously filtered out to avoid conflicts with the marking system used. Change-Id: Iab929b19ade5c9faaf7c6be073136352170c794a Reviewed-on: https://chromium-review.googlesource.com/1002862 Commit-Queue: Calder Kitagawa Reviewed-by: Samuel Huang Cr-Commit-Position: refs/heads/master@{#549620} NOKEYCHECK=True GitOrigin-RevId: 79c5e3545912bb2ec527fbe61b2aafe66c6e5bc2 --- abs32_utils.cc | 7 ------ image_utils.h | 20 ----------------- image_utils_unittest.cc | 60 ------------------------------------------------- rel32_utils.cc | 6 ++--- rel32_utils_unittest.cc | 8 +------ reloc_utils.cc | 7 +----- 6 files changed, 4 insertions(+), 104 deletions(-) diff --git a/abs32_utils.cc b/abs32_utils.cc index b45da7e..ab4372c 100644 --- a/abs32_utils.cc +++ b/abs32_utils.cc @@ -139,13 +139,6 @@ base::Optional Abs32ReaderWin32::GetNext() { offset_t target = target_rva_to_offset_.Convert(unit->target_rva); if (target == kInvalidOffset) continue; - // In rare cases, the most significant bit of |target| is set. This - // interferes with label marking. A quick fix is to reject these. - if (IsMarked(target)) { - LOG(WARNING) << "Warning: Skipping mark-aliased PE abs32 target: " - << AsHex<8>(location) << " -> " << AsHex<8>(target) << "."; - continue; - } return Reference{location, target}; } return base::nullopt; diff --git a/image_utils.h b/image_utils.h index 0374ab7..3765763 100644 --- a/image_utils.h +++ b/image_utils.h @@ -113,26 +113,6 @@ class ReferenceWriter { virtual void PutNext(Reference reference) = 0; }; -// Position of the most significant bit of offset_t. -constexpr offset_t kIndexMarkBitPosition = sizeof(offset_t) * 8 - 1; - -// Helper functions to mark an offset_t, so we can distinguish file offsets from -// Label indices. Implementation: Marking is flagged by the most significant bit -// (MSB). -constexpr inline bool IsMarked(offset_t value) { - return value >> kIndexMarkBitPosition != 0; -} -constexpr inline offset_t MarkIndex(offset_t value) { - return value | (offset_t(1) << kIndexMarkBitPosition); -} -constexpr inline offset_t UnmarkIndex(offset_t value) { - return value & ~(offset_t(1) << kIndexMarkBitPosition); -} - -// Constant as placeholder for non-existing offset for an index. -constexpr offset_t kUnusedIndex = offset_t(-1); -static_assert(IsMarked(kUnusedIndex), "kUnusedIndex must be marked"); - // An Equivalence is a block of length |length| that approximately match in // |old_image| at an offset of |src_offset| and in |new_image| at an offset of // |dst_offset|. diff --git a/image_utils_unittest.cc b/image_utils_unittest.cc index 7cae9d2..cd71a2f 100644 --- a/image_utils_unittest.cc +++ b/image_utils_unittest.cc @@ -14,64 +14,4 @@ TEST(ImageUtilsTest, Bitness) { EXPECT_EQ(8U, WidthOf(kBit64)); } -TEST(ImageUtilsTest, IsMarked) { - EXPECT_FALSE(IsMarked(0x00000000)); - EXPECT_TRUE(IsMarked(0x80000000)); - - EXPECT_FALSE(IsMarked(0x00000001)); - EXPECT_TRUE(IsMarked(0x80000001)); - - EXPECT_FALSE(IsMarked(0x70000000)); - EXPECT_TRUE(IsMarked(0xF0000000)); - - EXPECT_FALSE(IsMarked(0x7FFFFFFF)); - EXPECT_TRUE(IsMarked(0xFFFFFFFF)); - - EXPECT_FALSE(IsMarked(0x70000000)); - EXPECT_TRUE(IsMarked(0xC0000000)); - - EXPECT_FALSE(IsMarked(0x0000BEEF)); - EXPECT_TRUE(IsMarked(0x8000BEEF)); -} - -TEST(ImageUtilsTest, MarkIndex) { - EXPECT_EQ(offset_t(0x80000000), MarkIndex(0x00000000)); - EXPECT_EQ(offset_t(0x80000000), MarkIndex(0x80000000)); - - EXPECT_EQ(offset_t(0x80000001), MarkIndex(0x00000001)); - EXPECT_EQ(offset_t(0x80000001), MarkIndex(0x80000001)); - - EXPECT_EQ(offset_t(0xF0000000), MarkIndex(0x70000000)); - EXPECT_EQ(offset_t(0xF0000000), MarkIndex(0xF0000000)); - - EXPECT_EQ(offset_t(0xFFFFFFFF), MarkIndex(0x7FFFFFFF)); - EXPECT_EQ(offset_t(0xFFFFFFFF), MarkIndex(0xFFFFFFFF)); - - EXPECT_EQ(offset_t(0xC0000000), MarkIndex(0x40000000)); - EXPECT_EQ(offset_t(0xC0000000), MarkIndex(0xC0000000)); - - EXPECT_EQ(offset_t(0x8000BEEF), MarkIndex(0x0000BEEF)); - EXPECT_EQ(offset_t(0x8000BEEF), MarkIndex(0x8000BEEF)); -} - -TEST(ImageUtilsTest, UnmarkIndex) { - EXPECT_EQ(offset_t(0x00000000), UnmarkIndex(0x00000000)); - EXPECT_EQ(offset_t(0x00000000), UnmarkIndex(0x80000000)); - - EXPECT_EQ(offset_t(0x00000001), UnmarkIndex(0x00000001)); - EXPECT_EQ(offset_t(0x00000001), UnmarkIndex(0x80000001)); - - EXPECT_EQ(offset_t(0x70000000), UnmarkIndex(0x70000000)); - EXPECT_EQ(offset_t(0x70000000), UnmarkIndex(0xF0000000)); - - EXPECT_EQ(offset_t(0x7FFFFFFF), UnmarkIndex(0x7FFFFFFF)); - EXPECT_EQ(offset_t(0x7FFFFFFF), UnmarkIndex(0xFFFFFFFF)); - - EXPECT_EQ(offset_t(0x40000000), UnmarkIndex(0x40000000)); - EXPECT_EQ(offset_t(0x40000000), UnmarkIndex(0xC0000000)); - - EXPECT_EQ(offset_t(0x0000BEEF), UnmarkIndex(0x0000BEEF)); - EXPECT_EQ(offset_t(0x0000BEEF), UnmarkIndex(0x8000BEEF)); -} - } // namespace zucchini diff --git a/rel32_utils.cc b/rel32_utils.cc index fa59386..2d42975 100644 --- a/rel32_utils.cc +++ b/rel32_utils.cc @@ -37,10 +37,8 @@ base::Optional Rel32ReaderX86::GetNext() { rva_t loc_rva = location_offset_to_rva_.Convert(loc_offset); rva_t target_rva = loc_rva + 4 + image_.read(loc_offset); offset_t target_offset = target_rva_to_offset_.Convert(target_rva); - // In rare cases, the most significant bit of |target| is set. This - // interferes with label marking. We expect these to already be filtered out - // from |locations|. - DCHECK(!IsMarked(target_offset)); + // |locations| is valid by assumption (see class description). + DCHECK_NE(kInvalidOffset, target_offset); return Reference{loc_offset, target_offset}; } return base::nullopt; diff --git a/rel32_utils_unittest.cc b/rel32_utils_unittest.cc index 6d90112..29e8560 100644 --- a/rel32_utils_unittest.cc +++ b/rel32_utils_unittest.cc @@ -51,7 +51,7 @@ TEST(Rel32UtilsTest, Rel32ReaderX86) { // including rel32 targets, without the full instructions. std::vector bytes = { 0xFF, 0xFF, 0xFF, 0xFF, // 00030000: (Filler) - 0x00, 0x00, 0x00, 0x80, // 00030004: 80030008 Marked, so invalid. + 0xFF, 0xFF, 0xFF, 0xFF, // 0003000C: (Filler) 0x04, 0x00, 0x00, 0x00, // 00030008: 00030010 0xFF, 0xFF, 0xFF, 0xFF, // 0003000C: (Filler) 0x00, 0x00, 0x00, 0x00, // 00030010: 00030014 @@ -82,12 +82,6 @@ TEST(Rel32UtilsTest, Rel32ReaderX86) { Rel32ReaderX86 reader3(buffer, 0x000CU, 0x0018U, &rel32_locations, translator); CheckReader({{0x0010U, 0x0014U}}, &reader3); - - // Marked target encountered (error). - std::vector rel32_marked_locations = {0x00004U}; - Rel32ReaderX86 reader4(buffer, 0x0000U, 0x0020U, &rel32_marked_locations, - translator); - EXPECT_DCHECK_DEATH(reader4.GetNext()); } TEST(Rel32UtilsTest, Rel32WriterX86) { diff --git a/reloc_utils.cc b/reloc_utils.cc index bfad98e..84f488e 100644 --- a/reloc_utils.cc +++ b/reloc_utils.cc @@ -149,15 +149,10 @@ base::Optional RelocReaderWin32::GetNext() { offset_t target = entry_rva_to_offset_.Convert(unit->target_rva); if (target == kInvalidOffset) continue; - offset_t location = unit->location; - if (IsMarked(target)) { - LOG(WARNING) << "Warning: Skipping mark-aliased reloc target: " - << AsHex<8>(location) << " -> " << AsHex<8>(target) << "."; - continue; - } // Ensures the target (abs32 reference) lies entirely within the image. if (target >= offset_bound_) continue; + offset_t location = unit->location; return Reference{location, target}; } return base::nullopt; -- cgit v1.2.3