am e8a30f37: Merge "Fix some style nitpicks"
* commit 'e8a30f37bf1530a80a7df17692dbe7a68764ac30':
Fix some style nitpicks
diff --git a/compiler/elf_writer_quick.cc b/compiler/elf_writer_quick.cc
index 06f6e89..4274386 100644
--- a/compiler/elf_writer_quick.cc
+++ b/compiler/elf_writer_quick.cc
@@ -14,8 +14,6 @@
* limitations under the License.
*/
-#include <unordered_set>
-
#include "elf_writer_quick.h"
#include "base/logging.h"
diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc
index 2d25b7a..acfa607 100644
--- a/compiler/image_writer.cc
+++ b/compiler/image_writer.cc
@@ -796,9 +796,9 @@
};
const bool add_patches = compiler_driver_.GetCompilerOptions().GetIncludePatchInformation();
if (add_patches) {
- // TODO if we are adding patches the resulting ELF file might have a
- // potentially rather large amount of free space where patches might have been
- // placed. We should adjust the ELF file to get rid of this excess space.
+ // TODO if we are adding patches the resulting ELF file might have a potentially rather large
+ // amount of free space where patches might have been placed. We should adjust the ELF file to
+ // get rid of this excess space.
patches.reserve(compiler_driver_.GetCodeToPatch().size() +
compiler_driver_.GetMethodsToPatch().size() +
compiler_driver_.GetClassesToPatch().size());
@@ -892,7 +892,7 @@
}
Elf32_Shdr* shdr = file->FindSectionByName(".oat_patches");
if (shdr != nullptr) {
- DCHECK_EQ(shdr, file->FindSectionByType(SHT_OAT_PATCH))
+ CHECK_EQ(shdr, file->FindSectionByType(SHT_OAT_PATCH))
<< "Incorrect type for .oat_patches section";
CHECK_LE(patches.size() * sizeof(uintptr_t), shdr->sh_size)
<< "We got more patches than anticipated";
@@ -903,9 +903,8 @@
<< "Section overlaps onto next section";
// It's mmap'd so we can just memcpy.
memcpy(file->Begin() + shdr->sh_offset, patches.data(), patches.size()*sizeof(uintptr_t));
- // TODO We should fill in the newly empty space between the last patch and
- // the start of the next section by moving the following sections down if
- // possible.
+ // TODO We should fill in the newly empty space between the last patch and the start of the
+ // next section by moving the following sections down if possible.
shdr->sh_size = patches.size() * sizeof(uintptr_t);
} else {
LOG(ERROR) << "Unable to find section header for SHT_OAT_PATCH";
diff --git a/patchoat/patchoat.cc b/patchoat/patchoat.cc
index ea4b880..dcf8c70 100644
--- a/patchoat/patchoat.cc
+++ b/patchoat/patchoat.cc
@@ -64,15 +64,14 @@
bool PatchOat::Patch(const std::string& image_location, off_t delta,
File* output_image, InstructionSet isa,
- TimingLogger& timings) {
- std::string error_msg;
+ TimingLogger* timings) {
CHECK(Runtime::Current() == nullptr);
CHECK(output_image != nullptr);
CHECK_GE(output_image->Fd(), 0);
CHECK(!image_location.empty()) << "image file must have a filename.";
CHECK_NE(isa, kNone);
- TimingLogger::ScopedTiming t("Runtime Setup", &timings);
+ TimingLogger::ScopedTiming t("Runtime Setup", timings);
const char *isa_name = GetInstructionSetString(isa);
std::string image_filename(GetSystemImageFilename(image_location.c_str(), isa));
std::unique_ptr<File> input_image(OS::OpenFileForReading(image_filename.c_str()));
@@ -110,6 +109,7 @@
t.NewTiming("Image and oat Patching setup");
// Create the map where we will write the image patches to.
+ std::string error_msg;
std::unique_ptr<MemMap> image(MemMap::MapFile(image_len, PROT_READ | PROT_WRITE, MAP_PRIVATE,
input_image->Fd(), 0,
input_image->GetPath().c_str(),
@@ -137,8 +137,7 @@
bool PatchOat::Patch(const File* input_oat, const std::string& image_location, off_t delta,
File* output_oat, File* output_image, InstructionSet isa,
- TimingLogger& timings) {
- std::string error_msg;
+ TimingLogger* timings) {
CHECK(Runtime::Current() == nullptr);
CHECK(output_image != nullptr);
CHECK_GE(output_image->Fd(), 0);
@@ -148,7 +147,7 @@
CHECK_GE(output_oat->Fd(), 0);
CHECK(!image_location.empty()) << "image file must have a filename.";
- TimingLogger::ScopedTiming t("Runtime Setup", &timings);
+ TimingLogger::ScopedTiming t("Runtime Setup", timings);
if (isa == kNone) {
Elf32_Ehdr elf_hdr;
@@ -194,6 +193,7 @@
t.NewTiming("Image and oat Patching setup");
// Create the map where we will write the image patches to.
+ std::string error_msg;
std::unique_ptr<MemMap> image(MemMap::MapFile(image_len, PROT_READ | PROT_WRITE, MAP_PRIVATE,
input_image->Fd(), 0,
input_image->GetPath().c_str(),
@@ -234,7 +234,7 @@
}
bool PatchOat::WriteElf(File* out) {
- TimingLogger::ScopedTiming t("Writing Elf File", &timings_);
+ TimingLogger::ScopedTiming t("Writing Elf File", timings_);
CHECK(oat_file_.get() != nullptr);
CHECK(out != nullptr);
size_t expect = oat_file_->Size();
@@ -248,7 +248,7 @@
}
bool PatchOat::WriteImage(File* out) {
- TimingLogger::ScopedTiming t("Writing image File", &timings_);
+ TimingLogger::ScopedTiming t("Writing image File", timings_);
CHECK(image_ != nullptr);
CHECK(out != nullptr);
size_t expect = image_->Size();
@@ -275,7 +275,7 @@
}
{
- TimingLogger::ScopedTiming t("Walk Bitmap", &timings_);
+ TimingLogger::ScopedTiming t("Walk Bitmap", timings_);
// Walk the bitmap.
WriterMutexLock mu(Thread::Current(), *Locks::heap_bitmap_lock_);
bitmap_->Walk(PatchOat::BitmapCallback, this);
@@ -348,7 +348,7 @@
void PatchOat::FixupMethod(mirror::ArtMethod* object, mirror::ArtMethod* copy) {
// Just update the entry points if it looks like we should.
- // TODO sanity check all the pointers' values
+ // TODO: sanity check all the pointers' values
uintptr_t portable = reinterpret_cast<uintptr_t>(
object->GetEntryPointFromPortableCompiledCode<kVerifyNone>());
if (portable != 0) {
@@ -377,12 +377,12 @@
}
}
-bool PatchOat::Patch(File* input_oat, off_t delta, File* output_oat, TimingLogger& timings) {
+bool PatchOat::Patch(File* input_oat, off_t delta, File* output_oat, TimingLogger* timings) {
CHECK(input_oat != nullptr);
CHECK(output_oat != nullptr);
CHECK_GE(input_oat->Fd(), 0);
CHECK_GE(output_oat->Fd(), 0);
- TimingLogger::ScopedTiming t("Setup Oat File Patching", &timings);
+ TimingLogger::ScopedTiming t("Setup Oat File Patching", timings);
std::string error_msg;
std::unique_ptr<ElfFile> elf(ElfFile::Open(const_cast<File*>(input_oat),
@@ -437,7 +437,7 @@
}
bool PatchOat::PatchElf() {
- TimingLogger::ScopedTiming t("Fixup Elf Headers", &timings_);
+ TimingLogger::ScopedTiming t("Fixup Elf Headers", timings_);
// Fixup Phdr's
for (unsigned int i = 0; i < oat_file_->GetProgramHeaderNum(); i++) {
Elf32_Phdr& hdr = oat_file_->GetProgramHeader(i);
@@ -623,28 +623,28 @@
exit(EXIT_FAILURE);
}
-static bool ReadBaseDelta(const char* name, off_t* delta, std::string& error_msg) {
+static bool ReadBaseDelta(const char* name, off_t* delta, std::string* error_msg) {
CHECK(name != nullptr);
CHECK(delta != nullptr);
std::unique_ptr<File> file;
if (OS::FileExists(name)) {
file.reset(OS::OpenFileForReading(name));
if (file.get() == nullptr) {
- error_msg = "Failed to open file %s for reading";
+ *error_msg = "Failed to open file %s for reading";
return false;
}
} else {
- error_msg = "File %s does not exist";
+ *error_msg = "File %s does not exist";
return false;
}
CHECK(file.get() != nullptr);
ImageHeader hdr;
if (sizeof(hdr) != file->Read(reinterpret_cast<char*>(&hdr), sizeof(hdr), 0)) {
- error_msg = "Failed to read file %s";
+ *error_msg = "Failed to read file %s";
return false;
}
if (!hdr.IsValid()) {
- error_msg = "%s does not contain a valid image header.";
+ *error_msg = "%s does not contain a valid image header.";
return false;
}
*delta = hdr.GetPatchDelta();
@@ -661,7 +661,7 @@
}
}
-int patchoat(int argc, char **argv) {
+static int patchoat(int argc, char **argv) {
InitLogging(argv);
const bool debug = kIsDebugBuild;
orig_argc = argc;
@@ -712,7 +712,7 @@
if (log_options) {
LOG(INFO) << "patchoat: option[" << i << "]=" << argv[i];
}
- // TODO GetInstructionSetFromString shouldn't LOG(FATAL).
+ // TODO: GetInstructionSetFromString shouldn't LOG(FATAL).
if (option.starts_with("--instruction-set=")) {
isa_set = true;
const char* isa_str = option.substr(strlen("--instruction-set=")).data();
@@ -921,7 +921,7 @@
} else if (!patched_image_filename.empty()) {
base_delta_set = true;
std::string error_msg;
- if (!ReadBaseDelta(patched_image_filename.c_str(), &base_delta, error_msg)) {
+ if (!ReadBaseDelta(patched_image_filename.c_str(), &base_delta, &error_msg)) {
Usage(error_msg.c_str(), patched_image_filename.c_str());
}
} else {
@@ -1000,14 +1000,14 @@
if (have_image_files && have_oat_files) {
TimingLogger::ScopedTiming pt("patch image and oat", &timings);
ret = PatchOat::Patch(input_oat.get(), input_image_location, base_delta,
- output_oat.get(), output_image.get(), isa, timings);
+ output_oat.get(), output_image.get(), isa, &timings);
} else if (have_oat_files) {
TimingLogger::ScopedTiming pt("patch oat", &timings);
- ret = PatchOat::Patch(input_oat.get(), base_delta, output_oat.get(), timings);
+ ret = PatchOat::Patch(input_oat.get(), base_delta, output_oat.get(), &timings);
} else {
TimingLogger::ScopedTiming pt("patch image", &timings);
CHECK(have_image_files);
- ret = PatchOat::Patch(input_image_location, base_delta, output_image.get(), isa, timings);
+ ret = PatchOat::Patch(input_image_location, base_delta, output_image.get(), isa, &timings);
}
cleanup(ret);
return (ret) ? EXIT_SUCCESS : EXIT_FAILURE;
diff --git a/patchoat/patchoat.h b/patchoat/patchoat.h
index b9f36d4..a63e6f4 100644
--- a/patchoat/patchoat.h
+++ b/patchoat/patchoat.h
@@ -36,38 +36,29 @@
class Reference;
class Class;
class ArtMethod;
-};
-
-int patchoat(int argc, char** argv);
+}; // namespace mirror
class PatchOat {
public:
- static bool Patch(File* oat_in, off_t delta, File* oat_out, TimingLogger& timings);
+ static bool Patch(File* oat_in, off_t delta, File* oat_out, TimingLogger* timings);
static bool Patch(const std::string& art_location, off_t delta, File* art_out, InstructionSet isa,
- TimingLogger& timings);
+ TimingLogger* timings);
static bool Patch(const File* oat_in, const std::string& art_location,
off_t delta, File* oat_out, File* art_out, InstructionSet isa,
- TimingLogger& timings);
+ TimingLogger* timings);
private:
- std::unique_ptr<ElfFile> oat_file_;
- MemMap* image_;
- gc::accounting::ContinuousSpaceBitmap* bitmap_;
- MemMap* heap_;
- off_t delta_;
- TimingLogger& timings_;
-
// Takes ownership only of the ElfFile. All other pointers are only borrowed.
- PatchOat(ElfFile* oat_file, off_t delta, TimingLogger& timings)
+ PatchOat(ElfFile* oat_file, off_t delta, TimingLogger* timings)
: oat_file_(oat_file), delta_(delta), timings_(timings) {}
PatchOat(MemMap* image, gc::accounting::ContinuousSpaceBitmap* bitmap,
- MemMap* heap, off_t delta, TimingLogger& timings)
+ MemMap* heap, off_t delta, TimingLogger* timings)
: image_(image), bitmap_(bitmap), heap_(heap),
delta_(delta), timings_(timings) {}
PatchOat(ElfFile* oat_file, MemMap* image, gc::accounting::ContinuousSpaceBitmap* bitmap,
- MemMap* heap, off_t delta, TimingLogger& timings)
+ MemMap* heap, off_t delta, TimingLogger* timings)
: oat_file_(oat_file), image_(image), bitmap_(bitmap), heap_(heap),
delta_(delta), timings_(timings) {}
~PatchOat() {}
@@ -98,6 +89,8 @@
mirror::Object* RelocatedCopyOf(mirror::Object*);
mirror::Object* RelocatedAddressOf(mirror::Object* obj);
+ // Walks through the old image and patches the mmap'd copy of it to the new offset. It does not
+ // change the heap.
class PatchVisitor {
public:
PatchVisitor(PatchOat* patcher, mirror::Object* copy) : patcher_(patcher), copy_(copy) {}
@@ -112,6 +105,18 @@
mirror::Object* copy_;
};
+ // The elf file we are patching.
+ std::unique_ptr<ElfFile> oat_file_;
+ // A mmap of the image we are patching. This is modified.
+ const MemMap* image_;
+ // The heap we are patching. This is not modified.
+ gc::accounting::ContinuousSpaceBitmap* bitmap_;
+ // The heap we are patching. This is not modified.
+ const MemMap* heap_;
+ // The amount we are changing the offset by.
+ off_t delta_;
+ TimingLogger* timings_;
+
DISALLOW_IMPLICIT_CONSTRUCTORS(PatchOat);
};
