Fix a memory leak
dex_file is meant to take ownership of `container`. There's at least
one path (`error_msg = "Invalid or truncated dex file";`) where
`container` never gets passed along to a dex_file.
Caught by the static analyzer:
art/runtime/dex/dex_file_loader.cc:406:45: warning: Potential memory
leak
Bug: None
Test: Reran the analyzer. Memory leak complaints are gone.
Change-Id: Ib57008e444d32b366bb2beabec8e39b8e84fd9db
diff --git a/libdexfile/dex/compact_dex_file.cc b/libdexfile/dex/compact_dex_file.cc
index ce289d4..f11b678 100644
--- a/libdexfile/dex/compact_dex_file.cc
+++ b/libdexfile/dex/compact_dex_file.cc
@@ -91,7 +91,7 @@
const std::string& location,
uint32_t location_checksum,
const OatDexFile* oat_dex_file,
- DexFileContainer* container)
+ std::unique_ptr<DexFileContainer> container)
: DexFile(base,
size,
data_begin,
@@ -99,7 +99,7 @@
location,
location_checksum,
oat_dex_file,
- container,
+ std::move(container),
/*is_compact_dex*/ true),
debug_info_offsets_(DataBegin() + GetHeader().debug_info_offsets_pos_,
GetHeader().debug_info_base_,
diff --git a/libdexfile/dex/compact_dex_file.h b/libdexfile/dex/compact_dex_file.h
index 47b170c..81ccce4 100644
--- a/libdexfile/dex/compact_dex_file.h
+++ b/libdexfile/dex/compact_dex_file.h
@@ -277,7 +277,7 @@
const std::string& location,
uint32_t location_checksum,
const OatDexFile* oat_dex_file,
- DexFileContainer* container);
+ std::unique_ptr<DexFileContainer> container);
CompactDexDebugInfoOffsetTable::Accessor debug_info_offsets_;
diff --git a/libdexfile/dex/dex_file.cc b/libdexfile/dex/dex_file.cc
index 6a704c1..c055eb2 100644
--- a/libdexfile/dex/dex_file.cc
+++ b/libdexfile/dex/dex_file.cc
@@ -100,7 +100,7 @@
const std::string& location,
uint32_t location_checksum,
const OatDexFile* oat_dex_file,
- DexFileContainer* container,
+ std::unique_ptr<DexFileContainer> container,
bool is_compact_dex)
: begin_(base),
size_(size),
@@ -120,7 +120,7 @@
call_site_ids_(nullptr),
num_call_site_ids_(0),
oat_dex_file_(oat_dex_file),
- container_(container),
+ container_(std::move(container)),
is_compact_dex_(is_compact_dex) {
CHECK(begin_ != nullptr) << GetLocation();
CHECK_GT(size_, 0U) << GetLocation();
diff --git a/libdexfile/dex/dex_file.h b/libdexfile/dex/dex_file.h
index a62ab62..9b980a4 100644
--- a/libdexfile/dex/dex_file.h
+++ b/libdexfile/dex/dex_file.h
@@ -1015,7 +1015,7 @@
const std::string& location,
uint32_t location_checksum,
const OatDexFile* oat_dex_file,
- DexFileContainer* container,
+ std::unique_ptr<DexFileContainer> container,
bool is_compact_dex);
// Top-level initializer that calls other Init methods.
diff --git a/libdexfile/dex/dex_file_loader.cc b/libdexfile/dex/dex_file_loader.cc
index 2c75c5b..f0e54ea 100644
--- a/libdexfile/dex/dex_file_loader.cc
+++ b/libdexfile/dex/dex_file_loader.cc
@@ -313,7 +313,7 @@
bool verify,
bool verify_checksum,
std::string* error_msg,
- DexFileContainer* container,
+ std::unique_ptr<DexFileContainer> container,
VerifyResult* verify_result) {
if (verify_result != nullptr) {
*verify_result = VerifyResult::kVerifyNotAttempted;
@@ -328,7 +328,7 @@
location,
location_checksum,
oat_dex_file,
- container));
+ std::move(container)));
} else if (size >= sizeof(CompactDexFile::Header) && CompactDexFile::IsMagicValid(base)) {
if (data_base == nullptr) {
// TODO: Is there a clean way to support both an explicit data section and reading the one
@@ -345,7 +345,7 @@
location,
location_checksum,
oat_dex_file,
- container));
+ std::move(container)));
} else {
*error_msg = "Invalid or truncated dex file";
}
@@ -403,18 +403,19 @@
return nullptr;
}
VerifyResult verify_result;
- std::unique_ptr<const DexFile> dex_file = OpenCommon(map.data(),
- map.size(),
- /*data_base*/ nullptr,
- /*data_size*/ 0u,
- location,
- zip_entry->GetCrc32(),
- /*oat_dex_file*/ nullptr,
- verify,
- verify_checksum,
- error_msg,
- new VectorContainer(std::move(map)),
- &verify_result);
+ std::unique_ptr<const DexFile> dex_file = OpenCommon(
+ map.data(),
+ map.size(),
+ /*data_base*/ nullptr,
+ /*data_size*/ 0u,
+ location,
+ zip_entry->GetCrc32(),
+ /*oat_dex_file*/ nullptr,
+ verify,
+ verify_checksum,
+ error_msg,
+ std::make_unique<VectorContainer>(std::move(map)),
+ &verify_result);
if (dex_file == nullptr) {
if (verify_result == VerifyResult::kVerifyNotAttempted) {
*error_code = ZipOpenErrorCode::kDexFileError;
diff --git a/libdexfile/dex/dex_file_loader.h b/libdexfile/dex/dex_file_loader.h
index 41d9b16..8d1cfae 100644
--- a/libdexfile/dex/dex_file_loader.h
+++ b/libdexfile/dex/dex_file_loader.h
@@ -170,7 +170,7 @@
bool verify,
bool verify_checksum,
std::string* error_msg,
- DexFileContainer* container,
+ std::unique_ptr<DexFileContainer> container,
VerifyResult* verify_result);
private:
diff --git a/libdexfile/dex/standard_dex_file.h b/libdexfile/dex/standard_dex_file.h
index 9b13caa..999e5b9 100644
--- a/libdexfile/dex/standard_dex_file.h
+++ b/libdexfile/dex/standard_dex_file.h
@@ -93,7 +93,7 @@
const std::string& location,
uint32_t location_checksum,
const OatDexFile* oat_dex_file,
- DexFileContainer* container)
+ std::unique_ptr<DexFileContainer> container)
: DexFile(base,
size,
/*data_begin*/ base,
@@ -101,7 +101,7 @@
location,
location_checksum,
oat_dex_file,
- container,
+ std::move(container),
/*is_compact_dex*/ false) {}
friend class DexFileLoader;
diff --git a/runtime/dex/art_dex_file_loader.cc b/runtime/dex/art_dex_file_loader.cc
index 0817cb4..14386a3 100644
--- a/runtime/dex/art_dex_file_loader.cc
+++ b/runtime/dex/art_dex_file_loader.cc
@@ -192,7 +192,7 @@
verify,
verify_checksum,
error_msg,
- new MemMapContainer(std::move(map)),
+ std::make_unique<MemMapContainer>(std::move(map)),
/*verify_result*/ nullptr);
return dex_file;
}
@@ -315,7 +315,7 @@
verify,
verify_checksum,
error_msg,
- new MemMapContainer(std::move(map)),
+ std::make_unique<MemMapContainer>(std::move(map)),
/*verify_result*/ nullptr);
return dex_file;
@@ -384,7 +384,7 @@
verify,
verify_checksum,
error_msg,
- new MemMapContainer(std::move(map)),
+ std::make_unique<MemMapContainer>(std::move(map)),
&verify_result);
if (dex_file == nullptr) {
if (verify_result == VerifyResult::kVerifyNotAttempted) {