Skip to content

Commit

Permalink
Refactor DexFileLoader
Browse files Browse the repository at this point in the history
The first parameter of the Open methods specifies the data source,
that we intend to load the dex file(s) from. This creates large number
of overloads when multiplied by diversity of the other arguments.

Move the data source parameters to the constructor of DexFileLoader.
Specifically, the constructor just creates the right DexFileContainer,
and the rest of the loader is independent of the data source used.

This removes large amount of the overloads as well as large amount
of copy-pasted code. Majority of ArtDexFileLoader has been removed.

Bug: 266950186
Test: ./art/test.py -b --host --optimizing --64
Change-Id: I6580b49e65441eec93a7e0124be23bd8c859904a
  • Loading branch information
dsrbecky committed Feb 21, 2023
1 parent 4184f23 commit 052f5fb
Show file tree
Hide file tree
Showing 55 changed files with 662 additions and 1,327 deletions.
8 changes: 4 additions & 4 deletions dex2oat/dex2oat_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -619,9 +619,9 @@ class Dex2oatLayoutTest : public Dex2oatTest {
const char* location = dex_location.c_str();
std::string error_msg;
std::vector<std::unique_ptr<const DexFile>> dex_files;
const ArtDexFileLoader dex_file_loader;
ArtDexFileLoader dex_file_loader(location);
ASSERT_TRUE(dex_file_loader.Open(
location, location, /*verify=*/true, /*verify_checksum=*/true, &error_msg, &dex_files));
/*verify=*/true, /*verify_checksum=*/true, &error_msg, &dex_files));
EXPECT_EQ(dex_files.size(), 1U);
std::unique_ptr<const DexFile>& dex_file = dex_files[0];

Expand Down Expand Up @@ -811,9 +811,9 @@ class Dex2oatLayoutTest : public Dex2oatTest {

const char* location = dex_location.c_str();
std::vector<std::unique_ptr<const DexFile>> dex_files;
const ArtDexFileLoader dex_file_loader;
ArtDexFileLoader dex_file_loader(location);
ASSERT_TRUE(dex_file_loader.Open(
location, location, /*verify=*/ true, /*verify_checksum=*/ true, &error_msg, &dex_files));
/*verify=*/true, /*verify_checksum=*/true, &error_msg, &dex_files));
EXPECT_EQ(dex_files.size(), 1U);
std::unique_ptr<const DexFile>& old_dex_file = dex_files[0];

Expand Down
24 changes: 9 additions & 15 deletions dex2oat/linker/oat_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -462,11 +462,9 @@ bool OatWriter::AddDexFileSource(const char* filename, const char* location) {
bool OatWriter::AddDexFileSource(File&& dex_file_fd, const char* location) {
DCHECK(write_state_ == WriteState::kAddingDexFileSources);
std::string error_msg;
const ArtDexFileLoader loader;
ArtDexFileLoader loader(dex_file_fd.Release(), location);
std::vector<std::unique_ptr<const DexFile>> dex_files;
if (!loader.Open(dex_file_fd.Release(),
location,
/*verify=*/false,
if (!loader.Open(/*verify=*/false,
/*verify_checksum=*/false,
&error_msg,
&dex_files)) {
Expand Down Expand Up @@ -524,11 +522,8 @@ bool OatWriter::AddRawDexFileSource(const ArrayRef<const uint8_t>& data,
uint32_t location_checksum) {
DCHECK(write_state_ == WriteState::kAddingDexFileSources);
std::string error_msg;
const ArtDexFileLoader loader;
auto dex_file = loader.Open(data.data(),
data.size(),
location,
location_checksum,
ArtDexFileLoader loader(data.data(), data.size(), location);
auto dex_file = loader.Open(location_checksum,
nullptr,
/*verify=*/false,
/*verify_checksum=*/false,
Expand Down Expand Up @@ -3187,7 +3182,8 @@ bool OatWriter::WriteDexFiles(File* file,
if (copy_dex_files == CopyOption::kOnlyIfCompressed) {
extract_dex_files_into_vdex_ = false;
for (OatDexFile& oat_dex_file : oat_dex_files_) {
if (!oat_dex_file.GetDexFile()->GetContainer()->IsDirectMmap()) {
const DexFileContainer* container = oat_dex_file.GetDexFile()->GetContainer();
if (!(container->IsZip() && container->IsFileMap())) {
extract_dex_files_into_vdex_ = true;
break;
}
Expand Down Expand Up @@ -3468,7 +3464,6 @@ bool OatWriter::OpenDexFiles(

DCHECK_EQ(opened_dex_files_map->size(), 1u);
DCHECK(vdex_begin_ == opened_dex_files_map->front().Begin());
const ArtDexFileLoader dex_file_loader;
std::vector<std::unique_ptr<const DexFile>> dex_files;
for (OatDexFile& oat_dex_file : oat_dex_files_) {
const uint8_t* raw_dex_file = vdex_begin_ + oat_dex_file.dex_file_offset_;
Expand All @@ -3490,10 +3485,9 @@ bool OatWriter::OpenDexFiles(

// Now, open the dex file.
std::string error_msg;
dex_files.emplace_back(dex_file_loader.Open(raw_dex_file,
oat_dex_file.dex_file_size_,
oat_dex_file.GetLocation(),
oat_dex_file.dex_file_location_checksum_,
ArtDexFileLoader dex_file_loader(
raw_dex_file, oat_dex_file.dex_file_size_, oat_dex_file.GetLocation());
dex_files.emplace_back(dex_file_loader.Open(oat_dex_file.dex_file_location_checksum_,
/* oat_dex_file */ nullptr,
verify,
verify,
Expand Down
12 changes: 3 additions & 9 deletions dexdump/dexdump.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1962,18 +1962,12 @@ int processFile(const char* fileName) {
LOG(ERROR) << "ReadFileToString failed";
return -1;
}
const DexFileLoader dex_file_loader;
DexFileLoaderErrorCode error_code;
std::string error_msg;
std::vector<std::unique_ptr<const DexFile>> dex_files;
if (!dex_file_loader.OpenAll(reinterpret_cast<const uint8_t*>(content.data()),
content.size(),
fileName,
kVerify,
kVerifyChecksum,
&error_code,
&error_msg,
&dex_files)) {
DexFileLoader dex_file_loader(
reinterpret_cast<const uint8_t*>(content.data()), content.size(), fileName);
if (!dex_file_loader.Open(kVerify, kVerifyChecksum, &error_code, &error_msg, &dex_files)) {
// Display returned error message to user. Note that this error behavior
// differs from the error messages shown by the original Dalvik dexdump.
LOG(ERROR) << error_msg;
Expand Down
7 changes: 4 additions & 3 deletions dexdump/dexdump_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@
* Also, ODEX files are no longer supported.
*/

#include "dexdump.h"

#include <android-base/logging.h>
#include <base/mem_map.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>

#include <android-base/logging.h>
#include "dexdump.h"

namespace art {

Expand Down Expand Up @@ -157,6 +157,7 @@ int dexdumpDriver(int argc, char** argv) {
int main(int argc, char** argv) {
// Output all logging to stderr.
android::base::SetLogger(android::base::StderrLogger);
art::MemMap::Init();

return art::dexdumpDriver(argc, argv);
}
10 changes: 4 additions & 6 deletions dexlayout/dexlayout.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2009,17 +2009,15 @@ bool DexLayout::ProcessDexFile(const char* file_name,
std::string location = "memory mapped file for " + std::string(file_name);
// Dex file verifier cannot handle compact dex.
bool verify = options_.compact_dex_level_ == CompactDexLevel::kCompactDexLevelNone;
const ArtDexFileLoader dex_file_loader;
DexContainer::Section* const main_section = (*dex_container)->GetMainSection();
DexContainer::Section* const data_section = (*dex_container)->GetDataSection();
DCHECK_EQ(file_size, main_section->Size())
<< main_section->Size() << " " << data_section->Size();
auto container = std::make_unique<DexLoaderContainer>(
main_section->Begin(), main_section->End(), data_section->Begin(), data_section->End());
ArtDexFileLoader dex_file_loader(std::move(container), location);
std::unique_ptr<const DexFile> output_dex_file(
dex_file_loader.Open(std::move(container),
location,
/* location_checksum= */ 0,
dex_file_loader.Open(/* location_checksum= */ 0,
/*oat_dex_file=*/nullptr,
verify,
/*verify_checksum=*/false,
Expand Down Expand Up @@ -2057,10 +2055,10 @@ int DexLayout::ProcessFile(const char* file_name) {
// all of which are Zip archives with "classes.dex" inside.
const bool verify_checksum = !options_.ignore_bad_checksum_;
std::string error_msg;
const ArtDexFileLoader dex_file_loader;
ArtDexFileLoader dex_file_loader(file_name);
std::vector<std::unique_ptr<const DexFile>> dex_files;
if (!dex_file_loader.Open(
file_name, file_name, /* verify= */ true, verify_checksum, &error_msg, &dex_files)) {
/* verify= */ true, verify_checksum, &error_msg, &dex_files)) {
// Display returned error message to user. Note that this error behavior
// differs from the error messages shown by the original Dalvik dexdump.
LOG(ERROR) << error_msg;
Expand Down
37 changes: 17 additions & 20 deletions dexlayout/dexlayout_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -330,11 +330,9 @@ class DexLayoutTest : public CommonArtTest {
const std::string& out_profile) {
std::vector<std::unique_ptr<const DexFile>> dex_files;
std::string error_msg;
const ArtDexFileLoader dex_file_loader;
bool result = dex_file_loader.Open(input_dex.c_str(),
input_dex,
/*verify=*/ true,
/*verify_checksum=*/ false,
ArtDexFileLoader dex_file_loader(input_dex);
bool result = dex_file_loader.Open(/*verify=*/true,
/*verify_checksum=*/false,
&error_msg,
&dex_files);

Expand Down Expand Up @@ -774,14 +772,15 @@ TEST_F(DexLayoutTest, LinkData) {
TEST_F(DexLayoutTest, ClassFilter) {
std::vector<std::unique_ptr<const DexFile>> dex_files;
std::string error_msg;
const ArtDexFileLoader dex_file_loader;
const std::string input_jar = GetTestDexFileName("ManyMethods");
CHECK(dex_file_loader.Open(input_jar.c_str(),
input_jar.c_str(),
/*verify=*/ true,
/*verify_checksum=*/ true,
&error_msg,
&dex_files)) << error_msg;
{
ArtDexFileLoader dex_file_loader(input_jar);
CHECK(dex_file_loader.Open(/*verify=*/true,
/*verify_checksum=*/true,
&error_msg,
&dex_files))
<< error_msg;
}
ASSERT_EQ(dex_files.size(), 1u);
for (const std::unique_ptr<const DexFile>& dex_file : dex_files) {
EXPECT_GT(dex_file->NumClassDefs(), 1u);
Expand All @@ -808,14 +807,12 @@ TEST_F(DexLayoutTest, ClassFilter) {
out->GetMainSection()->End(),
out->GetDataSection()->Begin(),
out->GetDataSection()->End());
std::unique_ptr<const DexFile> output_dex_file(
dex_file_loader.Open(std::move(container),
dex_file->GetLocation().c_str(),
/* location_checksum= */ 0,
/*oat_dex_file=*/nullptr,
/* verify= */ true,
/*verify_checksum=*/false,
&error_msg));
ArtDexFileLoader dex_file_loader(std::move(container), dex_file->GetLocation());
std::unique_ptr<const DexFile> output_dex_file(dex_file_loader.Open(/* location_checksum= */ 0,
/*oat_dex_file=*/nullptr,
/* verify= */ true,
/*verify_checksum=*/false,
&error_msg));
ASSERT_TRUE(output_dex_file != nullptr);

ASSERT_EQ(output_dex_file->NumClassDefs(), options.class_filter_.size());
Expand Down
15 changes: 6 additions & 9 deletions dexlist/dexlist.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <android-base/file.h>
#include <android-base/logging.h>

#include "base/mem_map.h"
#include "dex/class_accessor-inl.h"
#include "dex/code_item_accessors-inl.h"
#include "dex/dex_file-inl.h"
Expand Down Expand Up @@ -168,15 +169,10 @@ static int processFile(const char* fileName) {
std::vector<std::unique_ptr<const DexFile>> dex_files;
DexFileLoaderErrorCode error_code;
std::string error_msg;
const DexFileLoader dex_file_loader;
if (!dex_file_loader.OpenAll(reinterpret_cast<const uint8_t*>(content.data()),
content.size(),
fileName,
/*verify=*/ true,
kVerifyChecksum,
&error_code,
&error_msg,
&dex_files)) {
DexFileLoader dex_file_loader(
reinterpret_cast<const uint8_t*>(content.data()), content.size(), fileName);
if (!dex_file_loader.Open(
/*verify=*/true, kVerifyChecksum, &error_code, &error_msg, &dex_files)) {
LOG(ERROR) << error_msg;
return -1;
}
Expand Down Expand Up @@ -281,6 +277,7 @@ int dexlistDriver(int argc, char** argv) {
int main(int argc, char** argv) {
// Output all logging to stderr.
android::base::SetLogger(android::base::StderrLogger);
art::MemMap::Init();

return art::dexlistDriver(argc, argv);
}
Expand Down
18 changes: 4 additions & 14 deletions libartbase/base/common_art_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -389,14 +389,9 @@ std::unique_ptr<const DexFile> CommonArtTestImpl::LoadExpectSingleDexFile(const
std::string error_msg;
MemMap::Init();
static constexpr bool kVerifyChecksum = true;
const ArtDexFileLoader dex_file_loader;
std::string filename(IsHost() ? GetAndroidBuildTop() + location : location);
if (!dex_file_loader.Open(filename.c_str(),
std::string(location),
/* verify= */ true,
kVerifyChecksum,
&error_msg,
&dex_files)) {
ArtDexFileLoader dex_file_loader(filename.c_str(), std::string(location));
if (!dex_file_loader.Open(/* verify= */ true, kVerifyChecksum, &error_msg, &dex_files)) {
LOG(FATAL) << "Could not open .dex file '" << filename << "': " << error_msg << "\n";
UNREACHABLE();
}
Expand Down Expand Up @@ -516,14 +511,9 @@ std::vector<std::unique_ptr<const DexFile>> CommonArtTestImpl::OpenDexFiles(cons
static constexpr bool kVerify = true;
static constexpr bool kVerifyChecksum = true;
std::string error_msg;
const ArtDexFileLoader dex_file_loader;
ArtDexFileLoader dex_file_loader(filename);
std::vector<std::unique_ptr<const DexFile>> dex_files;
bool success = dex_file_loader.Open(filename,
filename,
kVerify,
kVerifyChecksum,
&error_msg,
&dex_files);
bool success = dex_file_loader.Open(kVerify, kVerifyChecksum, &error_msg, &dex_files);
CHECK(success) << "Failed to open '" << filename << "': " << error_msg;
for (auto& dex_file : dex_files) {
CHECK(dex_file->IsReadOnly());
Expand Down
9 changes: 4 additions & 5 deletions libartbase/base/common_art_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,13 +173,12 @@ class CommonArtTestImpl {
bool MutateDexFile(File* output_dex, const std::string& input_jar, const Mutator& mutator) {
std::vector<std::unique_ptr<const DexFile>> dex_files;
std::string error_msg;
const ArtDexFileLoader dex_file_loader;
CHECK(dex_file_loader.Open(input_jar.c_str(),
input_jar.c_str(),
/*verify*/ true,
ArtDexFileLoader dex_file_loader(input_jar);
CHECK(dex_file_loader.Open(/*verify*/ true,
/*verify_checksum*/ true,
&error_msg,
&dex_files)) << error_msg;
&dex_files))
<< error_msg;
EXPECT_EQ(dex_files.size(), 1u) << "Only one input dex is supported";
const std::unique_ptr<const DexFile>& dex = dex_files[0];
CHECK(dex->EnableWrite()) << "Failed to enable write";
Expand Down
2 changes: 2 additions & 0 deletions libartbase/base/mem_map.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1021,6 +1021,8 @@ void MemMap::Init() {
TargetMMapInit();
}

bool MemMap::IsInitialized() { return mem_maps_lock_ != nullptr; }

void MemMap::Shutdown() {
if (mem_maps_lock_ == nullptr) {
// If MemMap::Shutdown is called more than once, there is no effect.
Expand Down
1 change: 1 addition & 0 deletions libartbase/base/mem_map.h
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ class MemMap {
// time after the first call to Init and before the first call to Shutodwn.
static void Init() REQUIRES(!MemMap::mem_maps_lock_);
static void Shutdown() REQUIRES(!MemMap::mem_maps_lock_);
static bool IsInitialized();

// If the map is PROT_READ, try to read each page of the map to check it is in fact readable (not
// faulting). This is used to diagnose a bug b/19894268 where mprotect doesn't seem to be working
Expand Down
1 change: 1 addition & 0 deletions libartbase/base/systrace.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ class ScopedTrace {
}

explicit ScopedTrace(const std::string& name) : ScopedTrace(name.c_str()) {}
ScopedTrace(ScopedTrace&&) = default;

~ScopedTrace() {
ATraceEnd();
Expand Down
18 changes: 18 additions & 0 deletions libartbase/base/zip_archive.cc
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,24 @@ ZipArchive* ZipArchive::OpenFromOwnedFd(int fd, const char* filename, std::strin
return OpenFromFdInternal(fd, /*assume_ownership=*/false, filename, error_msg);
}

ZipArchive* ZipArchive::OpenFromMemory(const uint8_t* data,
size_t size,
const char* filename,
std::string* error_msg) {
DCHECK(filename != nullptr);
DCHECK(data != nullptr);

ZipArchiveHandle handle;
const int32_t error = OpenArchiveFromMemory(data, size, filename, &handle);
if (error != 0) {
*error_msg = std::string(ErrorCodeString(error));
CloseArchive(handle);
return nullptr;
}

return new ZipArchive(handle);
}

ZipArchive* ZipArchive::OpenFromFdInternal(int fd,
bool assume_ownership,
const char* filename,
Expand Down
4 changes: 4 additions & 0 deletions libartbase/base/zip_archive.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ class ZipArchive {
static ZipArchive* Open(const char* filename, std::string* error_msg);
static ZipArchive* OpenFromFd(int fd, const char* filename, std::string* error_msg);
static ZipArchive* OpenFromOwnedFd(int fd, const char* filename, std::string* error_msg);
static ZipArchive* OpenFromMemory(const uint8_t* data,
size_t size,
const char* filename,
std::string* error_msg);

ZipEntry* Find(const char* name, std::string* error_msg) const;

Expand Down
Loading

0 comments on commit 052f5fb

Please sign in to comment.