Merge "Fix default method verifier check for compact dex"
diff --git a/dexlayout/compact_dex_writer.cc b/dexlayout/compact_dex_writer.cc
index f2d4619..f44eaef 100644
--- a/dexlayout/compact_dex_writer.cc
+++ b/dexlayout/compact_dex_writer.cc
@@ -27,7 +27,9 @@
header.checksum_ = header_->Checksum();
std::copy_n(header_->Signature(), DexFile::kSha1DigestSize, header.signature_);
header.file_size_ = header_->FileSize();
- header.header_size_ = header_->GetSize();
+ // Since we are not necessarily outputting the same format as the input, avoid using the stored
+ // header size.
+ header.header_size_ = GetHeaderSize();
header.endian_tag_ = header_->EndianTag();
header.link_size_ = header_->LinkSize();
header.link_off_ = header_->LinkOffset();
@@ -47,7 +49,17 @@
header.class_defs_off_ = collections.ClassDefsOffset();
header.data_size_ = header_->DataSize();
header.data_off_ = header_->DataOffset();
+ header.feature_flags_ = 0u;
+ // In cases where apps are converted to cdex during install, maintain feature flags so that
+ // the verifier correctly verifies apps that aren't targetting default methods.
+ if (header_->SupportDefaultMethods()) {
+ header.feature_flags_ |= static_cast<uint32_t>(CompactDexFile::FeatureFlags::kDefaultMethods);
+ }
UNUSED(Write(reinterpret_cast<uint8_t*>(&header), sizeof(header), 0u));
}
+size_t CompactDexWriter::GetHeaderSize() const {
+ return sizeof(CompactDexFile::Header);
+}
+
} // namespace art
diff --git a/dexlayout/compact_dex_writer.h b/dexlayout/compact_dex_writer.h
index e28efab..d13333b 100644
--- a/dexlayout/compact_dex_writer.h
+++ b/dexlayout/compact_dex_writer.h
@@ -35,6 +35,8 @@
protected:
void WriteHeader() OVERRIDE;
+ size_t GetHeaderSize() const OVERRIDE;
+
const CompactDexLevel compact_dex_level_;
private:
diff --git a/dexlayout/dex_ir.h b/dexlayout/dex_ir.h
index b25e164..a54c46f 100644
--- a/dexlayout/dex_ir.h
+++ b/dexlayout/dex_ir.h
@@ -524,7 +524,8 @@
uint32_t link_size,
uint32_t link_offset,
uint32_t data_size,
- uint32_t data_offset)
+ uint32_t data_offset,
+ bool support_default_methods)
: Item(0, kHeaderItemSize),
checksum_(checksum),
endian_tag_(endian_tag),
@@ -533,7 +534,8 @@
link_size_(link_size),
link_offset_(link_offset),
data_size_(data_size),
- data_offset_(data_offset) {
+ data_offset_(data_offset),
+ support_default_methods_(support_default_methods) {
memcpy(magic_, magic, sizeof(magic_));
memcpy(signature_, signature, sizeof(signature_));
}
@@ -567,6 +569,10 @@
void Accept(AbstractDispatcher* dispatch) { dispatch->Dispatch(this); }
+ bool SupportDefaultMethods() const {
+ return support_default_methods_;
+ }
+
private:
uint8_t magic_[8];
uint32_t checksum_;
@@ -578,6 +584,7 @@
uint32_t link_offset_;
uint32_t data_size_;
uint32_t data_offset_;
+ const bool support_default_methods_;
Collections collections_;
diff --git a/dexlayout/dex_ir_builder.cc b/dexlayout/dex_ir_builder.cc
index 1fd963f..231826b 100644
--- a/dexlayout/dex_ir_builder.cc
+++ b/dexlayout/dex_ir_builder.cc
@@ -37,7 +37,8 @@
disk_header.link_size_,
disk_header.link_off_,
disk_header.data_size_,
- disk_header.data_off_);
+ disk_header.data_off_,
+ dex_file.SupportsDefaultMethods());
Collections& collections = header->GetCollections();
collections.SetEagerlyAssignOffsets(eagerly_assign_offsets);
// Walk the rest of the header fields.
diff --git a/dexlayout/dex_writer.cc b/dexlayout/dex_writer.cc
index 4fd4157..41fdcbd 100644
--- a/dexlayout/dex_writer.cc
+++ b/dexlayout/dex_writer.cc
@@ -780,7 +780,7 @@
header.checksum_ = header_->Checksum();
std::copy_n(header_->Signature(), DexFile::kSha1DigestSize, header.signature_);
header.file_size_ = header_->FileSize();
- header.header_size_ = header_->GetSize();
+ header.header_size_ = GetHeaderSize();
header.endian_tag_ = header_->EndianTag();
header.link_size_ = header_->LinkSize();
header.link_off_ = header_->LinkOffset();
@@ -801,13 +801,18 @@
header.data_size_ = header_->DataSize();
header.data_off_ = header_->DataOffset();
+ CHECK_EQ(sizeof(header), GetHeaderSize());
static_assert(sizeof(header) == 0x70, "Size doesn't match dex spec");
UNUSED(Write(reinterpret_cast<uint8_t*>(&header), sizeof(header), 0u));
}
+size_t DexWriter::GetHeaderSize() const {
+ return sizeof(StandardDexFile::Header);
+}
+
void DexWriter::WriteMemMap() {
// Starting offset is right after the header.
- uint32_t offset = sizeof(StandardDexFile::Header);
+ uint32_t offset = GetHeaderSize();
dex_ir::Collections& collection = header_->GetCollections();
diff --git a/dexlayout/dex_writer.h b/dexlayout/dex_writer.h
index c47898e..3929526 100644
--- a/dexlayout/dex_writer.h
+++ b/dexlayout/dex_writer.h
@@ -91,6 +91,7 @@
// Header and id section
virtual void WriteHeader();
+ virtual size_t GetHeaderSize() const;
// reserve_only means don't write, only reserve space. This is required since the string data
// offsets must be assigned.
uint32_t WriteStringIds(uint32_t offset, bool reserve_only);
diff --git a/runtime/cdex/compact_dex_file.cc b/runtime/cdex/compact_dex_file.cc
index 82ffdb0..a92ab28 100644
--- a/runtime/cdex/compact_dex_file.cc
+++ b/runtime/cdex/compact_dex_file.cc
@@ -46,4 +46,9 @@
return IsVersionValid(header_->magic_);
}
+bool CompactDexFile::SupportsDefaultMethods() const {
+ return (GetHeader().GetFeatureFlags() &
+ static_cast<uint32_t>(FeatureFlags::kDefaultMethods)) != 0;
+}
+
} // namespace art
diff --git a/runtime/cdex/compact_dex_file.h b/runtime/cdex/compact_dex_file.h
index f17f8cf..e47f97e 100644
--- a/runtime/cdex/compact_dex_file.h
+++ b/runtime/cdex/compact_dex_file.h
@@ -17,6 +17,7 @@
#ifndef ART_RUNTIME_CDEX_COMPACT_DEX_FILE_H_
#define ART_RUNTIME_CDEX_COMPACT_DEX_FILE_H_
+#include "base/casts.h"
#include "dex_file.h"
namespace art {
@@ -27,8 +28,20 @@
static constexpr uint8_t kDexMagic[kDexMagicSize] = { 'c', 'd', 'e', 'x' };
static constexpr uint8_t kDexMagicVersion[] = {'0', '0', '1', '\0'};
+ enum class FeatureFlags : uint32_t {
+ kDefaultMethods = 0x1,
+ };
+
class Header : public DexFile::Header {
- // Same for now.
+ public:
+ uint32_t GetFeatureFlags() const {
+ return feature_flags_;
+ }
+
+ private:
+ uint32_t feature_flags_ = 0u;
+
+ friend class CompactDexWriter;
};
struct CodeItem : public DexFile::CodeItem {
@@ -51,6 +64,12 @@
static bool IsVersionValid(const uint8_t* magic);
virtual bool IsVersionValid() const OVERRIDE;
+ const Header& GetHeader() const {
+ return down_cast<const Header&>(DexFile::GetHeader());
+ }
+
+ virtual bool SupportsDefaultMethods() const OVERRIDE;
+
private:
// Not supported yet.
CompactDexFile(const uint8_t* base,
diff --git a/runtime/class_linker-inl.h b/runtime/class_linker-inl.h
index 4b317f8..cd6e8d5 100644
--- a/runtime/class_linker-inl.h
+++ b/runtime/class_linker-inl.h
@@ -174,7 +174,7 @@
break;
}
case kDirect:
- if (dex_cache->GetDexFile()->GetVersion() >= DexFile::kDefaultMethodsVersion) {
+ if (dex_cache->GetDexFile()->SupportsDefaultMethods()) {
break;
}
FALLTHROUGH_INTENDED;
diff --git a/runtime/dex_file.h b/runtime/dex_file.h
index e80a13c..6c6ae2d 100644
--- a/runtime/dex_file.h
+++ b/runtime/dex_file.h
@@ -69,8 +69,6 @@
static constexpr size_t kDexMagicSize = 4;
static constexpr size_t kDexVersionLen = 4;
- // First Dex format version supporting default methods.
- static const uint32_t kDefaultMethodsVersion = 37;
// First Dex format version enforcing class definition ordering rules.
static const uint32_t kClassDefinitionOrderEnforcedVersion = 37;
@@ -481,7 +479,7 @@
}
// Decode the dex magic version
- uint32_t GetVersion() const {
+ uint32_t GetDexVersion() const {
return GetHeader().GetVersion();
}
@@ -491,6 +489,9 @@
// Returns true if the byte string after the magic is the correct value.
virtual bool IsVersionValid() const = 0;
+ // Returns true if the dex file supports default methods.
+ virtual bool SupportsDefaultMethods() const = 0;
+
// Returns the number of string identifiers in the .dex file.
size_t NumStringIds() const {
DCHECK(header_ != nullptr) << GetLocation();
@@ -1044,6 +1045,9 @@
ALWAYS_INLINE const CompactDexFile* AsCompactDexFile() const;
protected:
+ // First Dex format version supporting default methods.
+ static const uint32_t kDefaultMethodsVersion = 37;
+
DexFile(const uint8_t* base,
size_t size,
const std::string& location,
diff --git a/runtime/dex_file_verifier.cc b/runtime/dex_file_verifier.cc
index 8656bcc..d6f685a 100644
--- a/runtime/dex_file_verifier.cc
+++ b/runtime/dex_file_verifier.cc
@@ -386,7 +386,14 @@
return false;
}
- if (header_->header_size_ != sizeof(DexFile::Header)) {
+ bool size_matches = false;
+ if (dex_file_->IsCompactDexFile()) {
+ size_matches = header_->header_size_ == sizeof(CompactDexFile::Header);
+ } else {
+ size_matches = header_->header_size_ == sizeof(StandardDexFile::Header);
+ }
+
+ if (!size_matches) {
ErrorStringPrintf("Bad header size: %ud", header_->header_size_);
return false;
}
@@ -3004,7 +3011,7 @@
GetFieldDescriptionOrError(begin_, header_, idx).c_str(),
field_access_flags,
PrettyJavaAccessFlags(field_access_flags).c_str());
- if (header_->GetVersion() >= DexFile::kDefaultMethodsVersion) {
+ if (dex_file_->SupportsDefaultMethods()) {
return false;
} else {
// Allow in older versions, but warn.
@@ -3019,7 +3026,7 @@
GetFieldDescriptionOrError(begin_, header_, idx).c_str(),
field_access_flags,
PrettyJavaAccessFlags(field_access_flags).c_str());
- if (header_->GetVersion() >= DexFile::kDefaultMethodsVersion) {
+ if (dex_file_->SupportsDefaultMethods()) {
return false;
} else {
// Allow in older versions, but warn.
@@ -3102,7 +3109,7 @@
*error_msg = StringPrintf("Constructor %" PRIu32 "(%s) is not flagged correctly wrt/ static.",
method_index,
GetMethodDescriptionOrError(begin_, header_, method_index).c_str());
- if (header_->GetVersion() >= DexFile::kDefaultMethodsVersion) {
+ if (dex_file_->SupportsDefaultMethods()) {
return false;
} else {
// Allow in older versions, but warn.
@@ -3131,14 +3138,14 @@
if ((class_access_flags & kAccInterface) != 0) {
// Non-static interface methods must be public or private.
uint32_t desired_flags = (kAccPublic | kAccStatic);
- if (dex_file_->GetVersion() >= DexFile::kDefaultMethodsVersion) {
+ if (dex_file_->SupportsDefaultMethods()) {
desired_flags |= kAccPrivate;
}
if ((method_access_flags & desired_flags) == 0) {
*error_msg = StringPrintf("Interface virtual method %" PRIu32 "(%s) is not public",
method_index,
GetMethodDescriptionOrError(begin_, header_, method_index).c_str());
- if (header_->GetVersion() >= DexFile::kDefaultMethodsVersion) {
+ if (dex_file_->SupportsDefaultMethods()) {
return false;
} else {
// Allow in older versions, but warn.
@@ -3163,7 +3170,7 @@
*error_msg = StringPrintf("Constructor %u(%s) must not be abstract or native",
method_index,
GetMethodDescriptionOrError(begin_, header_, method_index).c_str());
- if (header_->GetVersion() >= DexFile::kDefaultMethodsVersion) {
+ if (dex_file_->SupportsDefaultMethods()) {
return false;
} else {
// Allow in older versions, but warn.
@@ -3197,7 +3204,7 @@
*error_msg = StringPrintf("Interface method %" PRIu32 "(%s) is not public and abstract",
method_index,
GetMethodDescriptionOrError(begin_, header_, method_index).c_str());
- if (header_->GetVersion() >= DexFile::kDefaultMethodsVersion) {
+ if (dex_file_->SupportsDefaultMethods()) {
return false;
} else {
// Allow in older versions, but warn.
diff --git a/runtime/standard_dex_file.cc b/runtime/standard_dex_file.cc
index 4c1d308..b608341 100644
--- a/runtime/standard_dex_file.cc
+++ b/runtime/standard_dex_file.cc
@@ -63,4 +63,8 @@
return IsVersionValid(header_->magic_);
}
+bool StandardDexFile::SupportsDefaultMethods() const {
+ return GetDexVersion() >= DexFile::kDefaultMethodsVersion;
+}
+
} // namespace art
diff --git a/runtime/standard_dex_file.h b/runtime/standard_dex_file.h
index 5d53597..80c406e 100644
--- a/runtime/standard_dex_file.h
+++ b/runtime/standard_dex_file.h
@@ -56,6 +56,8 @@
static bool IsVersionValid(const uint8_t* magic);
virtual bool IsVersionValid() const OVERRIDE;
+ virtual bool SupportsDefaultMethods() const OVERRIDE;
+
private:
StandardDexFile(const uint8_t* base,
size_t size,
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index 63f21f8..5e5e96b 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -836,7 +836,7 @@
return false;
} else {
uint32_t access_flag_options = kAccPublic;
- if (dex_file_->GetVersion() >= DexFile::kDefaultMethodsVersion) {
+ if (dex_file_->SupportsDefaultMethods()) {
access_flag_options |= kAccPrivate;
}
if (!(method_access_flags_ & access_flag_options)) {
@@ -3965,10 +3965,10 @@
// while this check implies an IncompatibleClassChangeError.
if (klass->IsInterface()) {
// methods called on interfaces should be invoke-interface, invoke-super, invoke-direct (if
- // dex file version is 37 or greater), or invoke-static.
+ // default methods are supported for the dex file), or invoke-static.
if (method_type != METHOD_INTERFACE &&
method_type != METHOD_STATIC &&
- ((dex_file_->GetVersion() < DexFile::kDefaultMethodsVersion) ||
+ (!dex_file_->SupportsDefaultMethods() ||
method_type != METHOD_DIRECT) &&
method_type != METHOD_SUPER) {
Fail(VERIFY_ERROR_CLASS_CHANGE)
diff --git a/runtime/verifier/method_verifier.h b/runtime/verifier/method_verifier.h
index f26f3e2..b66433c 100644
--- a/runtime/verifier/method_verifier.h
+++ b/runtime/verifier/method_verifier.h
@@ -127,10 +127,6 @@
return *dex_file_;
}
- uint32_t DexFileVersion() const {
- return dex_file_->GetVersion();
- }
-
RegTypeCache* GetRegTypeCache() {
return ®_types_;
}
diff --git a/test/knownfailures.json b/test/knownfailures.json
index dc051d9..a12510c 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -649,11 +649,6 @@
"tests": "661-oat-writer-layout",
"variant": "interp-ac | interpreter | jit | no-dex2oat | no-prebuild | no-image | trace",
"description": ["Test is designed to only check --compiler-filter=speed"]
- },
- {
- "tests": ["975-iface-private"],
- "variant": "cdex-fast",
- "description": ["CompactDex doesn't yet work with 975-iface private"]
}
]