From f15ceeb7841ef97c24a0b7708732756d433c5d0d Mon Sep 17 00:00:00 2001 From: Dmitriy Ivanov Date: Tue, 21 Apr 2015 15:03:04 -0700 Subject: [PATCH] Always use signed leb128 encoding According to runs on /system/lib there using unsigned leb128 does not save us any additional space. In order to keep packing as simple as possible switch to using signed leb128 for everything. Bug: http://b/18051137 Change-Id: I1a47cb9eb2175895b3c3f7c13b4c6b1060de86c0 --- tools/relocation_packer/Android.mk | 2 - tools/relocation_packer/src/elf_file.cc | 16 ++- tools/relocation_packer/src/elf_file.h | 5 +- .../src/elf_file_unittest.cc | 10 +- tools/relocation_packer/src/leb128.cc | 87 -------------- tools/relocation_packer/src/leb128.h | 75 ------------ .../relocation_packer/src/leb128_unittest.cc | 111 ------------------ tools/relocation_packer/src/packer.cc | 37 ++---- .../relocation_packer/src/packer_unittest.cc | 24 ++-- .../elf_file_unittest_relocs_arm32_packed.so | Bin 89114 -> 89114 bytes .../elf_file_unittest_relocs_arm64_packed.so | Bin 113651 -> 113651 bytes 11 files changed, 50 insertions(+), 317 deletions(-) delete mode 100644 tools/relocation_packer/src/leb128.cc delete mode 100644 tools/relocation_packer/src/leb128.h delete mode 100644 tools/relocation_packer/src/leb128_unittest.cc diff --git a/tools/relocation_packer/Android.mk b/tools/relocation_packer/Android.mk index eabe0daa3..75dba7143 100644 --- a/tools/relocation_packer/Android.mk +++ b/tools/relocation_packer/Android.mk @@ -26,7 +26,6 @@ LOCAL_SRC_FILES := \ src/debug.cc \ src/delta_encoder.cc \ src/elf_file.cc \ - src/leb128.cc \ src/packer.cc \ src/sleb128.cc \ @@ -67,7 +66,6 @@ LOCAL_SRC_FILES := \ src/debug_unittest.cc \ src/delta_encoder_unittest.cc \ src/elf_file_unittest.cc \ - src/leb128_unittest.cc \ src/sleb128_unittest.cc \ src/packer_unittest.cc \ diff --git a/tools/relocation_packer/src/elf_file.cc b/tools/relocation_packer/src/elf_file.cc index 6843f5bae..11af9ed97 100644 --- a/tools/relocation_packer/src/elf_file.cc +++ b/tools/relocation_packer/src/elf_file.cc @@ -190,6 +190,7 @@ bool ElfFile::Load() { // these; both is unsupported. bool has_rel_relocations = false; bool has_rela_relocations = false; + bool has_android_relocations = false; Elf_Scn* section = NULL; while ((section = elf_nextscn(elf, section)) != nullptr) { @@ -209,6 +210,11 @@ bool ElfFile::Load() { if ((name == ".rel.dyn" || name == ".rela.dyn") && section_header->sh_size > 0) { found_relocations_section = section; + + // Note if relocation section is already packed + has_android_relocations = + section_header->sh_type == SHT_ANDROID_REL || + section_header->sh_type == SHT_ANDROID_RELA; } if (section_header->sh_offset == dynamic_program_header->p_offset) { @@ -250,6 +256,7 @@ bool ElfFile::Load() { relocations_section_ = found_relocations_section; dynamic_section_ = found_dynamic_section; relocations_type_ = has_rel_relocations ? REL : RELA; + has_android_relocations_ = has_android_relocations; return true; } @@ -610,10 +617,15 @@ template bool ElfFile::PackTypedRelocations(std::vector* relocations) { typedef typename ELF::Rela Rela; + if (has_android_relocations_) { + LOG(ERROR) << "Relocation table is already packed"; + return false; + } + // If no relocations then we have nothing packable. Perhaps // the shared object has already been packed? if (relocations->empty()) { - LOG(ERROR) << "No relocations found (already packed?)"; + LOG(ERROR) << "No relocations found"; return false; } @@ -737,7 +749,7 @@ bool ElfFile::UnpackRelocations() { packed.size() > 3 && packed[0] == 'A' && packed[1] == 'P' && - (packed[2] == 'U' || packed[2] == 'S') && + packed[2] == 'S' && packed[3] == '2') { LOG(INFO) << "Relocations : " << (relocations_type_ == REL ? "REL" : "RELA"); } else { diff --git a/tools/relocation_packer/src/elf_file.h b/tools/relocation_packer/src/elf_file.h index a749d5066..d6acc76b7 100644 --- a/tools/relocation_packer/src/elf_file.h +++ b/tools/relocation_packer/src/elf_file.h @@ -36,7 +36,7 @@ class ElfFile { explicit ElfFile(int fd) : fd_(fd), is_padding_relocations_(false), elf_(NULL), relocations_section_(NULL), dynamic_section_(NULL), - relocations_type_(NONE) {} + relocations_type_(NONE), has_android_relocations_(false) {} ~ElfFile() {} // Set padding mode. When padding, PackRelocations() will not shrink @@ -111,6 +111,9 @@ class ElfFile { // Relocation type found, assigned by Load(). relocations_type_t relocations_type_; + + // Elf-file has android relocations section + bool has_android_relocations_; }; } // namespace relocation_packer diff --git a/tools/relocation_packer/src/elf_file_unittest.cc b/tools/relocation_packer/src/elf_file_unittest.cc index 434f10102..5271eef87 100644 --- a/tools/relocation_packer/src/elf_file_unittest.cc +++ b/tools/relocation_packer/src/elf_file_unittest.cc @@ -175,13 +175,19 @@ static void RunPackRelocationsTestFor(const std::string& arch) { namespace relocation_packer { -TEST(ElfFile, PackRelocations) { +TEST(ElfFile, PackRelocationsArm32) { RunPackRelocationsTestFor("arm32"); +} + +TEST(ElfFile, PackRelocationsArm64) { RunPackRelocationsTestFor("arm64"); } -TEST(ElfFile, UnpackRelocations) { +TEST(ElfFile, UnpackRelocationsArm32) { RunUnpackRelocationsTestFor("arm32"); +} + +TEST(ElfFile, UnpackRelocationsArm64) { RunUnpackRelocationsTestFor("arm64"); } diff --git a/tools/relocation_packer/src/leb128.cc b/tools/relocation_packer/src/leb128.cc deleted file mode 100644 index 101c55778..000000000 --- a/tools/relocation_packer/src/leb128.cc +++ /dev/null @@ -1,87 +0,0 @@ -// Copyright 2014 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 "leb128.h" - -#include -#include - -#include "elf_traits.h" - -namespace relocation_packer { - -// Empty constructor and destructor to silence chromium-style. -template -Leb128Encoder::Leb128Encoder() { } - -template -Leb128Encoder::~Leb128Encoder() { } - -// Add a single value to the encoding. Values are encoded with variable -// length. The least significant 7 bits of each byte hold 7 bits of data, -// and the most significant bit is set on each byte except the last. -template -void Leb128Encoder::Enqueue(uint_t value) { - uint_t uvalue = static_cast(value); - do { - const uint8_t byte = uvalue & 127; - uvalue >>= 7; - encoding_.push_back((uvalue ? 128 : 0) | byte); - } while (uvalue); -} - -// Add a vector of values to the encoding. -template -void Leb128Encoder::EnqueueAll(const std::vector& values) { - for (size_t i = 0; i < values.size(); ++i) { - Enqueue(values[i]); - } -} - -// Create a new decoder for the given encoded stream. -template -Leb128Decoder::Leb128Decoder(const std::vector& encoding, size_t start_with) { - encoding_ = encoding; - cursor_ = start_with; -} - -// Empty destructor to silence chromium-style. -template -Leb128Decoder::~Leb128Decoder() { } - -// Decode and retrieve a single value from the encoding. Read forwards until -// a byte without its most significant bit is found, then read the 7 bit -// fields of the bytes spanned to re-form the value. -template -uint_t Leb128Decoder::Dequeue() { - uint_t value = 0; - - size_t shift = 0; - uint8_t byte; - - // Loop until we reach a byte with its high order bit clear. - do { - byte = encoding_[cursor_++]; - value |= static_cast(byte & 127) << shift; - shift += 7; - } while (byte & 128); - - return value; -} - -// Decode and retrieve all remaining values from the encoding. -template -void Leb128Decoder::DequeueAll(std::vector* values) { - while (cursor_ < encoding_.size()) { - values->push_back(Dequeue()); - } -} - -template class Leb128Encoder; -template class Leb128Encoder; - -template class Leb128Decoder; -template class Leb128Decoder; - -} // namespace relocation_packer diff --git a/tools/relocation_packer/src/leb128.h b/tools/relocation_packer/src/leb128.h deleted file mode 100644 index 67fc4b829..000000000 --- a/tools/relocation_packer/src/leb128.h +++ /dev/null @@ -1,75 +0,0 @@ -// Copyright 2014 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. - -// LEB128 encoder and decoder for packed relative relocations. -// -// Packed relocations consist of a large number of relatively small -// integer values. Encoding these as LEB128 saves space. -// -// For more on LEB128 see http://en.wikipedia.org/wiki/LEB128. - -#ifndef TOOLS_RELOCATION_PACKER_SRC_LEB128_H_ -#define TOOLS_RELOCATION_PACKER_SRC_LEB128_H_ - -#include -#include - -#include "elf_traits.h" - -namespace relocation_packer { - -// Encode packed words as a LEB128 byte stream. -template -class Leb128Encoder { - public: - // Explicit (but empty) constructor and destructor, for chromium-style. - Leb128Encoder(); - ~Leb128Encoder(); - - // Add a value to the encoding stream. - // |value| is the unsigned int to add. - void Enqueue(uint_t value); - - // Add a vector of values to the encoding stream. - // |values| is the vector of unsigned ints to add. - void EnqueueAll(const std::vector& values); - - // Retrieve the encoded representation of the values. - // |encoding| is the returned vector of encoded data. - void GetEncoding(std::vector* encoding) { *encoding = encoding_; } - - private: - // Growable vector holding the encoded LEB128 stream. - std::vector encoding_; -}; - -// Decode a LEB128 byte stream to produce packed words. -template -class Leb128Decoder { - public: - // Create a new decoder for the given encoded stream. - // |encoding| is the vector of encoded data. - explicit Leb128Decoder(const std::vector& encoding, size_t start_with); - - // Explicit (but empty) destructor, for chromium-style. - ~Leb128Decoder(); - - // Retrieve the next value from the encoded stream. - uint_t Dequeue(); - - // Retrieve all remaining values from the encoded stream. - // |values| is the vector of decoded data. - void DequeueAll(std::vector* values); - - private: - // Encoded LEB128 stream. - std::vector encoding_; - - // Cursor indicating the current stream retrieval point. - size_t cursor_; -}; - -} // namespace relocation_packer - -#endif // TOOLS_RELOCATION_PACKER_SRC_LEB128_H_ diff --git a/tools/relocation_packer/src/leb128_unittest.cc b/tools/relocation_packer/src/leb128_unittest.cc deleted file mode 100644 index 8a7028cbc..000000000 --- a/tools/relocation_packer/src/leb128_unittest.cc +++ /dev/null @@ -1,111 +0,0 @@ -// Copyright 2014 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 "leb128.h" - -#include -#include "gtest/gtest.h" - -namespace relocation_packer { - -TEST(Leb128, Encoder64) { - std::vector values; - values.push_back(624485); - values.push_back(0); - values.push_back(1); - values.push_back(127); - values.push_back(128); - - Leb128Encoder encoder; - encoder.EnqueueAll(values); - - encoder.Enqueue(4294967295); - encoder.Enqueue(18446744073709551615ul); - - std::vector encoding; - encoder.GetEncoding(&encoding); - - EXPECT_EQ(23U, encoding.size()); - // 624485 - EXPECT_EQ(0xe5, encoding[0]); - EXPECT_EQ(0x8e, encoding[1]); - EXPECT_EQ(0x26, encoding[2]); - // 0 - EXPECT_EQ(0x00, encoding[3]); - // 1 - EXPECT_EQ(0x01, encoding[4]); - // 127 - EXPECT_EQ(0x7f, encoding[5]); - // 128 - EXPECT_EQ(0x80, encoding[6]); - EXPECT_EQ(0x01, encoding[7]); - // 4294967295 - EXPECT_EQ(0xff, encoding[8]); - EXPECT_EQ(0xff, encoding[9]); - EXPECT_EQ(0xff, encoding[10]); - EXPECT_EQ(0xff, encoding[11]); - EXPECT_EQ(0x0f, encoding[12]); - // 18446744073709551615 - EXPECT_EQ(0xff, encoding[13]); - EXPECT_EQ(0xff, encoding[14]); - EXPECT_EQ(0xff, encoding[15]); - EXPECT_EQ(0xff, encoding[16]); - EXPECT_EQ(0xff, encoding[17]); - EXPECT_EQ(0xff, encoding[18]); - EXPECT_EQ(0xff, encoding[19]); - EXPECT_EQ(0xff, encoding[20]); - EXPECT_EQ(0xff, encoding[21]); - EXPECT_EQ(0x01, encoding[22]); -} - -TEST(Leb128, Decoder64) { - std::vector encoding; - // 624485 - encoding.push_back(0xe5); - encoding.push_back(0x8e); - encoding.push_back(0x26); - // 0 - encoding.push_back(0x00); - // 1 - encoding.push_back(0x01); - // 127 - encoding.push_back(0x7f); - // 128 - encoding.push_back(0x80); - encoding.push_back(0x01); - // 4294967295 - encoding.push_back(0xff); - encoding.push_back(0xff); - encoding.push_back(0xff); - encoding.push_back(0xff); - encoding.push_back(0x0f); - // 18446744073709551615 - encoding.push_back(0xff); - encoding.push_back(0xff); - encoding.push_back(0xff); - encoding.push_back(0xff); - encoding.push_back(0xff); - encoding.push_back(0xff); - encoding.push_back(0xff); - encoding.push_back(0xff); - encoding.push_back(0xff); - encoding.push_back(0x01); - - Leb128Decoder decoder(encoding, 0); - - EXPECT_EQ(624485U, decoder.Dequeue()); - - std::vector dequeued; - decoder.DequeueAll(&dequeued); - - EXPECT_EQ(6U, dequeued.size()); - EXPECT_EQ(0U, dequeued[0]); - EXPECT_EQ(1U, dequeued[1]); - EXPECT_EQ(127U, dequeued[2]); - EXPECT_EQ(128U, dequeued[3]); - EXPECT_EQ(4294967295U, dequeued[4]); - EXPECT_EQ(18446744073709551615UL, dequeued[5]); -} - -} // namespace relocation_packer diff --git a/tools/relocation_packer/src/packer.cc b/tools/relocation_packer/src/packer.cc index 8e30612e5..433611faa 100644 --- a/tools/relocation_packer/src/packer.cc +++ b/tools/relocation_packer/src/packer.cc @@ -9,7 +9,6 @@ #include "debug.h" #include "delta_encoder.h" #include "elf_traits.h" -#include "leb128.h" #include "sleb128.h" namespace relocation_packer { @@ -28,32 +27,17 @@ void RelocationPacker::PackRelocations(const std::vector sleb128_encoder; - Leb128Encoder leb128_encoder; - std::vector leb128_packed; std::vector sleb128_packed; - leb128_encoder.EnqueueAll(packed_words); - leb128_encoder.GetEncoding(&leb128_packed); - sleb128_encoder.EnqueueAll(packed_words); sleb128_encoder.GetEncoding(&sleb128_packed); - // TODO (simonb): Estimate savings on current android system image and consider using - // one encoder for all packed relocations to reduce complexity. - if (leb128_packed.size() <= sleb128_packed.size()) { - packed->push_back('A'); - packed->push_back('P'); - packed->push_back('U'); - packed->push_back('2'); - packed->insert(packed->end(), leb128_packed.begin(), leb128_packed.end()); - } else { - packed->push_back('A'); - packed->push_back('P'); - packed->push_back('S'); - packed->push_back('2'); - packed->insert(packed->end(), sleb128_packed.begin(), sleb128_packed.end()); - } + packed->push_back('A'); + packed->push_back('P'); + packed->push_back('S'); + packed->push_back('2'); + packed->insert(packed->end(), sleb128_packed.begin(), sleb128_packed.end()); } // Unpack relative relocations from a run-length encoded packed @@ -67,16 +51,11 @@ void RelocationPacker::UnpackRelocations( CHECK(packed.size() > 4 && packed[0] == 'A' && packed[1] == 'P' && - (packed[2] == 'U' || packed[2] == 'S') && + packed[2] == 'S' && packed[3] == '2'); - if (packed[2] == 'U') { - Leb128Decoder decoder(packed, 4); - decoder.DequeueAll(&packed_words); - } else { - Sleb128Decoder decoder(packed, 4); - decoder.DequeueAll(&packed_words); - } + Sleb128Decoder decoder(packed, 4); + decoder.DequeueAll(&packed_words); RelocationDeltaCodec codec; codec.Decode(packed_words, relocations); diff --git a/tools/relocation_packer/src/packer_unittest.cc b/tools/relocation_packer/src/packer_unittest.cc index 8dddd8b41..424b92cd0 100644 --- a/tools/relocation_packer/src/packer_unittest.cc +++ b/tools/relocation_packer/src/packer_unittest.cc @@ -39,6 +39,7 @@ template static void DoPackNoAddend() { std::vector relocations; std::vector packed; + bool is_32 = sizeof(typename ELF::Addr) == 4; // Initial relocation. AddRelocation(0xd1ce0000, 0x11, 0, &relocations); // Two more relocations, 4 byte deltas. @@ -59,16 +60,16 @@ static void DoPackNoAddend() { size_t ndx = 0; EXPECT_EQ('A', packed[ndx++]); EXPECT_EQ('P', packed[ndx++]); - EXPECT_EQ('U', packed[ndx++]); + EXPECT_EQ('S', packed[ndx++]); EXPECT_EQ('2', packed[ndx++]); // relocation count EXPECT_EQ(6, packed[ndx++]); - // base relocation = 0xd1cdfffc -> fc, ff, b7, 8e, 0d + // base relocation = 0xd1cdfffc -> fc, ff, b7, 8e, 7d/0d (32/64bit) EXPECT_EQ(0xfc, packed[ndx++]); EXPECT_EQ(0xff, packed[ndx++]); EXPECT_EQ(0xb7, packed[ndx++]); EXPECT_EQ(0x8e, packed[ndx++]); - EXPECT_EQ(0x0d, packed[ndx++]); + EXPECT_EQ(is_32 ? 0x7d : 0x0d, packed[ndx++]); // first group EXPECT_EQ(3, packed[ndx++]); // size EXPECT_EQ(3, packed[ndx++]); // flags @@ -83,8 +84,11 @@ static void DoPackNoAddend() { EXPECT_EQ(ndx, packed.size()); } -TEST(Packer, PackNoAddend) { +TEST(Packer, PackNoAddend32) { DoPackNoAddend(); +} + +TEST(Packer, PackNoAddend64) { DoPackNoAddend(); } @@ -92,18 +96,19 @@ template static void DoUnpackNoAddend() { std::vector relocations; std::vector packed; + bool is_32 = sizeof(typename ELF::Addr) == 4; packed.push_back('A'); packed.push_back('P'); - packed.push_back('U'); + packed.push_back('S'); packed.push_back('2'); // relocation count packed.push_back(6); - // base relocation = 0xd1cdfffc -> fc, ff, b7, 8e, 0d + // base relocation = 0xd1cdfffc -> fc, ff, b7, 8e, 7d/0d (32/64bit) packed.push_back(0xfc); packed.push_back(0xff); packed.push_back(0xb7); packed.push_back(0x8e); - packed.push_back(0x0d); + packed.push_back(is_32 ? 0x7d : 0x0d); // first group packed.push_back(3); // size packed.push_back(3); // flags @@ -131,8 +136,11 @@ static void DoUnpackNoAddend() { EXPECT_EQ(ndx, relocations.size()); } -TEST(Packer, UnpackNoAddend) { +TEST(Packer, UnpackNoAddend32) { DoUnpackNoAddend(); +} + +TEST(Packer, UnpackNoAddend64) { DoUnpackNoAddend(); } diff --git a/tools/relocation_packer/test_data/elf_file_unittest_relocs_arm32_packed.so b/tools/relocation_packer/test_data/elf_file_unittest_relocs_arm32_packed.so index d97ef825260d3a5dfec58d42b477693dc6220532..6ac2eef059748100a76160e041941f4790f3933b 100755 GIT binary patch delta 31 ncmbQWgLT#p)(v7z?7>Da+5h}z*et_j&)*y)xIIRYai13ey-Euv delta 30 mcmbQWgLT#p)(v7zY@tRk+5c2;mSwW%Z;lq+9xceY-wOb)^$Mo| diff --git a/tools/relocation_packer/test_data/elf_file_unittest_relocs_arm64_packed.so b/tools/relocation_packer/test_data/elf_file_unittest_relocs_arm64_packed.so index e44e4597b30e56d749a748ae7df6c8756e4cd1db..a2b0039187a95808876f93ef8ccceee0dcd1979c 100755 GIT binary patch delta 20 ccmezTo$d2?whdLxjKR&-%-gG(87p=G0BP+A$p8QV delta 20 ccmezTo$d2?whdLxjG@id%-gG(87p=G0BQjU%K!iX