Merge tag 'android-security-11.0.0_r49' into int/11/fp3
Android security 11.0.0 release 49
* tag 'android-security-11.0.0_r49':
Fix dm-test invocation
dex2oat_vdex_test: add missing dependency on core boot image.
Do not accept vdex with dex sections from .dm files
Add SafetyNet logging to JNI::NewStringUTF.
Validate input of JNI::NewStringUTF().
Change-Id: I15bff1117bee63c395350711fb1c7b4c38ad8fcc
diff --git a/dex2oat/dex2oat_test.cc b/dex2oat/dex2oat_test.cc
index ab33c19..4de8265 100644
--- a/dex2oat/dex2oat_test.cc
+++ b/dex2oat/dex2oat_test.cc
@@ -44,6 +44,8 @@
#include "dex/dex_file_loader.h"
#include "dex2oat_environment_test.h"
#include "dex2oat_return_codes.h"
+#include "elf_file.h"
+#include "elf_file_impl.h"
#include "gc_root-inl.h"
#include "intern_table-inl.h"
#include "oat.h"
@@ -2489,4 +2491,108 @@
RunTest();
}
+// Regression test for bug 179221298.
+TEST_F(Dex2oatTest, LoadOutOfDateOatFile) {
+ std::unique_ptr<const DexFile> dex(OpenTestDexFile("ManyMethods"));
+ std::string out_dir = GetScratchDir();
+ const std::string base_oat_name = out_dir + "/base.oat";
+ ASSERT_TRUE(GenerateOdexForTest(dex->GetLocation(),
+ base_oat_name,
+ CompilerFilter::Filter::kSpeed,
+ { "--deduplicate-code=false" },
+ /*expect_success=*/ true,
+ /*use_fd=*/ false,
+ /*use_zip_fd=*/ false));
+
+ // Check that we can open the oat file as executable.
+ {
+ std::string error_msg;
+ std::unique_ptr<OatFile> odex_file(OatFile::Open(/*zip_fd=*/ -1,
+ base_oat_name.c_str(),
+ base_oat_name.c_str(),
+ /*executable=*/ true,
+ /*low_4gb=*/ false,
+ dex->GetLocation(),
+ &error_msg));
+ ASSERT_TRUE(odex_file != nullptr) << error_msg;
+ }
+
+ // Rewrite the oat file with wrong version and bogus contents.
+ {
+ std::unique_ptr<File> file(OS::OpenFileReadWrite(base_oat_name.c_str()));
+ ASSERT_TRUE(file != nullptr);
+ // Retrieve the offset and size of the embedded oat file.
+ size_t oatdata_offset;
+ size_t oatdata_size;
+ {
+ std::string error_msg;
+ std::unique_ptr<ElfFile> elf_file(ElfFile::Open(file.get(),
+ /*writable=*/ false,
+ /*program_header_only=*/ true,
+ /*low_4gb=*/ false,
+ &error_msg));
+ ASSERT_TRUE(elf_file != nullptr) << error_msg;
+ ASSERT_TRUE(elf_file->Load(file.get(),
+ /*executable=*/ false,
+ /*low_4gb=*/ false,
+ /*reservation=*/ nullptr,
+ &error_msg)) << error_msg;
+ const uint8_t* base_address = elf_file->Is64Bit()
+ ? elf_file->GetImpl64()->GetBaseAddress()
+ : elf_file->GetImpl32()->GetBaseAddress();
+ const uint8_t* oatdata = elf_file->FindDynamicSymbolAddress("oatdata");
+ ASSERT_TRUE(oatdata != nullptr);
+ ASSERT_TRUE(oatdata > base_address);
+ // Note: We're assuming here that the virtual address offset is the same
+ // as file offset. This is currently true for all oat files we generate.
+ oatdata_offset = static_cast<size_t>(oatdata - base_address);
+ const uint8_t* oatlastword = elf_file->FindDynamicSymbolAddress("oatlastword");
+ ASSERT_TRUE(oatlastword != nullptr);
+ ASSERT_TRUE(oatlastword > oatdata);
+ oatdata_size = oatlastword - oatdata;
+ }
+
+ // Check that we have the right `oatdata_offset`.
+ int64_t length = file->GetLength();
+ ASSERT_GE(length, static_cast<ssize_t>(oatdata_offset + sizeof(OatHeader)));
+ alignas(OatHeader) uint8_t header_data[sizeof(OatHeader)];
+ ASSERT_TRUE(file->PreadFully(header_data, sizeof(header_data), oatdata_offset));
+ const OatHeader& header = reinterpret_cast<const OatHeader&>(header_data);
+ ASSERT_TRUE(header.IsValid()) << header.GetValidationErrorMessage();
+
+ // Overwrite all oat data from version onwards with bytes with value 4.
+ // (0x04040404 is not a valid version, we're using three decimal digits and '\0'.)
+ //
+ // We previously tried to find the value for key "debuggable" (bug 179221298)
+ // in the key-value store before checking the oat header. This test tries to
+ // ensure that such early processing of the key-value store shall crash.
+ // Reading 0x04040404 as the size of the key-value store yields a bit over
+ // 64MiB which should hopefully include some unmapped memory beyond the end
+ // of the loaded oat file. Overwriting the whole embedded oat file ensures
+ // that we do not match the key within the oat file but we could still
+ // accidentally match it in the additional sections of the elf file, so this
+ // approach could fail to catch similar issues. At the time of writing, this
+ // test crashed when run without the fix on 64-bit host (but not 32-bit).
+ static constexpr size_t kVersionOffset = sizeof(OatHeader::kOatMagic);
+ static_assert(kVersionOffset < sizeof(OatHeader));
+ std::vector<uint8_t> data(oatdata_size - kVersionOffset, 4u);
+ ASSERT_TRUE(file->PwriteFully(data.data(), data.size(), oatdata_offset + kVersionOffset));
+ UNUSED(oatdata_size);
+ CHECK_EQ(file->FlushClose(), 0) << "Could not flush and close oat file";
+ }
+
+ // Check that we reject the oat file without crashing.
+ {
+ std::string error_msg;
+ std::unique_ptr<OatFile> odex_file(OatFile::Open(/*zip_fd=*/ -1,
+ base_oat_name.c_str(),
+ base_oat_name.c_str(),
+ /*executable=*/ true,
+ /*low_4gb=*/ false,
+ dex->GetLocation(),
+ &error_msg));
+ ASSERT_FALSE(odex_file != nullptr);
+ }
+}
+
} // namespace art
diff --git a/openjdkjvmti/ti_redefine.cc b/openjdkjvmti/ti_redefine.cc
index c457dba..049e0cf 100644
--- a/openjdkjvmti/ti_redefine.cc
+++ b/openjdkjvmti/ti_redefine.cc
@@ -2761,8 +2761,6 @@
art::Locks::mutator_lock_->AssertExclusiveHeld(driver_->self_);
art::ClassLinker* cl = driver_->runtime_->GetClassLinker();
art::ScopedAssertNoThreadSuspension sants(__FUNCTION__);
- art::ObjPtr<art::mirror::Class> orig(holder.GetMirrorClass());
- art::ObjPtr<art::mirror::Class> replacement(holder.GetNewClassObject());
art::ObjPtr<art::mirror::ObjectArray<art::mirror::Class>> new_classes(holder.GetNewClasses());
art::ObjPtr<art::mirror::ObjectArray<art::mirror::Class>> old_classes(holder.GetOldClasses());
// Collect mappings from old to new fields/methods
@@ -2773,8 +2771,6 @@
holder.GetNewInstanceObjects());
art::ObjPtr<art::mirror::ObjectArray<art::mirror::Object>> old_instances(
holder.GetOldInstanceObjects());
- CHECK(!orig.IsNull());
- CHECK(!replacement.IsNull());
// Once we do the ReplaceReferences old_classes will have the new_classes in it. We want to keep
// ahold of the old classes so copy them now.
std::vector<art::ObjPtr<art::mirror::Class>> old_classes_vec(old_classes->Iterate().begin(),
@@ -2848,16 +2844,26 @@
}
}
// We can only shadow things from our superclasses
- if (LIKELY(!field_or_method->GetDeclaringClass()->IsAssignableFrom(orig))) {
+ auto orig_classes_iter = old_classes->Iterate();
+ auto replacement_classes_iter = new_classes->Iterate();
+ art::ObjPtr<art::mirror::Class> f_or_m_class = field_or_method->GetDeclaringClass();
+ if (LIKELY(!f_or_m_class->IsAssignableFrom(holder.GetMirrorClass()) &&
+ std::find(orig_classes_iter.begin(), orig_classes_iter.end(), f_or_m_class) ==
+ orig_classes_iter.end())) {
return false;
}
if constexpr (is_method) {
- auto direct_methods = replacement->GetDirectMethods(art::kRuntimePointerSize);
- return std::find_if(direct_methods.begin(),
- direct_methods.end(),
- [&](art::ArtMethod& m) REQUIRES(art::Locks::mutator_lock_) {
- return UNLIKELY(m.HasSameNameAndSignature(field_or_method));
- }) != direct_methods.end();
+ return std::any_of(
+ replacement_classes_iter.begin(),
+ replacement_classes_iter.end(),
+ [&](art::ObjPtr<art::mirror::Class> cand) REQUIRES(art::Locks::mutator_lock_) {
+ auto direct_methods = cand->GetDirectMethods(art::kRuntimePointerSize);
+ return std::find_if(direct_methods.begin(),
+ direct_methods.end(),
+ [&](art::ArtMethod& m) REQUIRES(art::Locks::mutator_lock_) {
+ return UNLIKELY(m.HasSameNameAndSignature(field_or_method));
+ }) != direct_methods.end();
+ });
} else {
auto pred = [&](art::ArtField& f) REQUIRES(art::Locks::mutator_lock_) {
return std::string_view(f.GetName()) == std::string_view(field_or_method->GetName()) &&
@@ -2865,11 +2871,21 @@
std::string_view(field_or_method->GetTypeDescriptor());
};
if (field_or_method->IsStatic()) {
- auto sfields = replacement->GetSFields();
- return std::find_if(sfields.begin(), sfields.end(), pred) != sfields.end();
+ return std::any_of(
+ replacement_classes_iter.begin(),
+ replacement_classes_iter.end(),
+ [&](art::ObjPtr<art::mirror::Class> cand) REQUIRES(art::Locks::mutator_lock_) {
+ auto sfields = cand->GetSFields();
+ return std::find_if(sfields.begin(), sfields.end(), pred) != sfields.end();
+ });
} else {
- auto ifields = replacement->GetIFields();
- return std::find_if(ifields.begin(), ifields.end(), pred) != ifields.end();
+ return std::any_of(
+ replacement_classes_iter.begin(),
+ replacement_classes_iter.end(),
+ [&](art::ObjPtr<art::mirror::Class> cand) REQUIRES(art::Locks::mutator_lock_) {
+ auto ifields = cand->GetIFields();
+ return std::find_if(ifields.begin(), ifields.end(), pred) != ifields.end();
+ });
}
}
};
@@ -2879,28 +2895,28 @@
[&](art::ArtField* f, const auto& info) REQUIRES(art::Locks::mutator_lock_) {
DCHECK(f != nullptr) << info;
auto it = field_map.find(f);
- if (it != field_map.end()) {
+ if (UNLIKELY(could_change_resolution_of(f, info))) {
+ // Dex-cache Resolution might change. Just clear the resolved value.
+ VLOG(plugin) << "Clearing resolution " << info << " for (field) " << f->PrettyField();
+ return static_cast<art::ArtField*>(nullptr);
+ } else if (it != field_map.end()) {
VLOG(plugin) << "Updating " << info << " object for (field) "
<< it->second->PrettyField();
return it->second;
- } else if (UNLIKELY(could_change_resolution_of(f, info))) {
- // Resolution might change. Just clear the resolved value.
- VLOG(plugin) << "Clearing resolution " << info << " for (field) " << f->PrettyField();
- return static_cast<art::ArtField*>(nullptr);
}
return f;
},
[&](art::ArtMethod* m, const auto& info) REQUIRES(art::Locks::mutator_lock_) {
DCHECK(m != nullptr) << info;
auto it = method_map.find(m);
- if (it != method_map.end()) {
+ if (UNLIKELY(could_change_resolution_of(m, info))) {
+ // Dex-cache Resolution might change. Just clear the resolved value.
+ VLOG(plugin) << "Clearing resolution " << info << " for (method) " << m->PrettyMethod();
+ return static_cast<art::ArtMethod*>(nullptr);
+ } else if (it != method_map.end()) {
VLOG(plugin) << "Updating " << info << " object for (method) "
<< it->second->PrettyMethod();
return it->second;
- } else if (UNLIKELY(could_change_resolution_of(m, info))) {
- // Resolution might change. Just clear the resolved value.
- VLOG(plugin) << "Clearing resolution " << info << " for (method) " << m->PrettyMethod();
- return static_cast<art::ArtMethod*>(nullptr);
}
return m;
});
@@ -2955,18 +2971,25 @@
if (art::kIsDebugBuild) {
// Just make sure we didn't screw up any of the now obsolete methods or fields. We need their
// declaring-class to still be the obolete class
- orig->VisitMethods([&](art::ArtMethod* method) REQUIRES_SHARED(art::Locks::mutator_lock_) {
- if (method->IsCopied()) {
- // Copied methods have interfaces as their declaring class.
- return;
- }
- DCHECK_EQ(method->GetDeclaringClass(), orig) << method->GetDeclaringClass()->PrettyClass()
- << " vs " << orig->PrettyClass();
- }, art::kRuntimePointerSize);
- orig->VisitFields([&](art::ArtField* field) REQUIRES_SHARED(art::Locks::mutator_lock_) {
- DCHECK_EQ(field->GetDeclaringClass(), orig) << field->GetDeclaringClass()->PrettyClass()
- << " vs " << orig->PrettyClass();
- });
+ std::for_each(
+ old_classes_vec.cbegin(),
+ old_classes_vec.cend(),
+ [](art::ObjPtr<art::mirror::Class> orig) REQUIRES_SHARED(art::Locks::mutator_lock_) {
+ orig->VisitMethods(
+ [&](art::ArtMethod* method) REQUIRES_SHARED(art::Locks::mutator_lock_) {
+ if (method->IsCopied()) {
+ // Copied methods have interfaces as their declaring class.
+ return;
+ }
+ DCHECK_EQ(method->GetDeclaringClass(), orig)
+ << method->GetDeclaringClass()->PrettyClass() << " vs " << orig->PrettyClass();
+ },
+ art::kRuntimePointerSize);
+ orig->VisitFields([&](art::ArtField* field) REQUIRES_SHARED(art::Locks::mutator_lock_) {
+ DCHECK_EQ(field->GetDeclaringClass(), orig)
+ << field->GetDeclaringClass()->PrettyClass() << " vs " << orig->PrettyClass();
+ });
+ });
}
}
diff --git a/runtime/Android.bp b/runtime/Android.bp
index 7e75016..ec59d44 100644
--- a/runtime/Android.bp
+++ b/runtime/Android.bp
@@ -19,8 +19,8 @@
// new jit code is generated. We don't want it to be called when a different function with the same
// (empty) body is called.
JIT_DEBUG_REGISTER_CODE_LDFLAGS = [
- "-Wl,--keep-unique,__jit_debug_register_code",
- "-Wl,--keep-unique,__dex_debug_register_code",
+ // "-Wl,--keep-unique,__jit_debug_register_code",
+ // "-Wl,--keep-unique,__dex_debug_register_code"
]
// These are defaults for native shared libaries that are expected to be
diff --git a/runtime/elf_file_impl.h b/runtime/elf_file_impl.h
index 9900c76..a5937ff 100644
--- a/runtime/elf_file_impl.h
+++ b/runtime/elf_file_impl.h
@@ -59,6 +59,10 @@
return file_path_;
}
+ uint8_t* GetBaseAddress() const {
+ return base_address_;
+ }
+
uint8_t* Begin() const {
return map_.Begin();
}
diff --git a/runtime/oat_file.cc b/runtime/oat_file.cc
index d72fc7e..48e34af 100644
--- a/runtime/oat_file.cc
+++ b/runtime/oat_file.cc
@@ -275,7 +275,17 @@
// We sometimes load oat files without a runtime (eg oatdump) and don't want to do anything in
// that case. If we are debuggable there are no -quick opcodes to unquicken. If the runtime is not
// debuggable we don't care whether there are -quick opcodes or not so no need to do anything.
- return Runtime::Current() != nullptr && !IsDebuggable() && Runtime::Current()->IsJavaDebuggable();
+ Runtime* runtime = Runtime::Current();
+ return (runtime != nullptr && runtime->IsJavaDebuggable()) &&
+ // Note: This is called before `OatFileBase::Setup()` where we validate the
+ // oat file contents. Check that we have at least a valid header, including
+ // oat file version, to avoid parsing the key-value store for a different
+ // version (out-of-date oat file) which can lead to crashes. b/179221298.
+ // TODO: While this is a poor workaround and the correct solution would be
+ // to postpone the unquickening check until after `OatFileBase::Setup()`,
+ // we prefer to avoid larger rewrites because quickening is deprecated and
+ // should be removed completely anyway. b/170086509
+ (GetOatHeader().IsValid() && !IsDebuggable());
}
bool OatFileBase::LoadVdex(const std::string& vdex_filename,
diff --git a/test/2036-structural-subclass-shadow/expected.txt b/test/2036-structural-subclass-shadow/expected.txt
new file mode 100644
index 0000000..7fb86e3
--- /dev/null
+++ b/test/2036-structural-subclass-shadow/expected.txt
@@ -0,0 +1,8 @@
+value-getter is 42
+value is 42
+static-call is 1337
+static-value is 3.14
+value-getter is 0
+value is 0
+static-call is -559038737
+static-value is 0.0
diff --git a/test/2036-structural-subclass-shadow/info.txt b/test/2036-structural-subclass-shadow/info.txt
new file mode 100644
index 0000000..317dbf8
--- /dev/null
+++ b/test/2036-structural-subclass-shadow/info.txt
@@ -0,0 +1,5 @@
+Tests dex-cache invalidation with structural redefinition.
+
+Regression test for b/161846143. Structural redefinition was incorrectly
+failing to invalidate some dex-cache entries causing incorrect field/method
+resolution in limited cases.
diff --git a/test/2036-structural-subclass-shadow/run b/test/2036-structural-subclass-shadow/run
new file mode 100755
index 0000000..ff387ff
--- /dev/null
+++ b/test/2036-structural-subclass-shadow/run
@@ -0,0 +1,17 @@
+#!/bin/bash
+#
+# Copyright 2020 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+./default-run "$@" --jvmti --runtime-option -Xopaque-jni-ids:true
diff --git a/test/2036-structural-subclass-shadow/src-art/Main.java b/test/2036-structural-subclass-shadow/src-art/Main.java
new file mode 100644
index 0000000..d707701
--- /dev/null
+++ b/test/2036-structural-subclass-shadow/src-art/Main.java
@@ -0,0 +1,21 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+public class Main {
+ public static void main(String[] args) throws Exception {
+ art.Test2036.run();
+ }
+}
diff --git a/test/2036-structural-subclass-shadow/src-art/art/Redefinition.java b/test/2036-structural-subclass-shadow/src-art/art/Redefinition.java
new file mode 120000
index 0000000..81eaf31
--- /dev/null
+++ b/test/2036-structural-subclass-shadow/src-art/art/Redefinition.java
@@ -0,0 +1 @@
+../../../jvmti-common/Redefinition.java
\ No newline at end of file
diff --git a/test/2036-structural-subclass-shadow/src-art/art/Test2036.java b/test/2036-structural-subclass-shadow/src-art/art/Test2036.java
new file mode 100644
index 0000000..8ca8e2a
--- /dev/null
+++ b/test/2036-structural-subclass-shadow/src-art/art/Test2036.java
@@ -0,0 +1,136 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package art;
+
+import java.util.Base64;
+
+public class Test2036 {
+ public static class Transform {
+ public Transform() {}
+ }
+
+ public static class SubTransform extends Transform {
+ public SubTransform() {}
+
+ public static int FooBar() {
+ return 1337;
+ }
+
+ public int foo = 42;
+ public static double STATIC_FOO = 3.14d;
+ }
+
+ public static class SubSubTransform extends SubTransform {
+ public SubSubTransform() {}
+ }
+
+ public static class SubSubSubTransform extends SubSubTransform {
+ public SubSubSubTransform() {}
+
+ public int getFoo() {
+ return this.foo;
+ }
+ }
+ /*
+ * base64 encoded class/dex file for
+ * Base64 generated using:
+ * % javac Test2036.java
+ * % d8 Test2035\$Transform.class
+ * % base64 classes.dex| sed 's:^:":' | sed 's:$:" +:'
+ *
+ * package art;
+ * public static class Transform {
+ * public Transform() {}
+ * public int bar;
+ * }
+ */
+ private static final byte[] DEX_BYTES =
+ Base64.getDecoder()
+ .decode(
+ "ZGV4CjAzNQAZlSz5hewOnpRnplr0AnEqh8+AKNc2qhp0AwAAcAAAAHhWNBIAAAAAAAAAALwCAAAP"
+ + "AAAAcAAAAAcAAACsAAAAAQAAAMgAAAABAAAA1AAAAAIAAADcAAAAAQAAAOwAAABoAgAADAEAACgB"
+ + "AAAwAQAAMwEAAE0BAABdAQAAgQEAAKEBAAC1AQAAxAEAAM8BAADSAQAA3wEAAOQBAADqAQAA8QEA"
+ + "AAEAAAACAAAAAwAAAAQAAAAFAAAABgAAAAkAAAAJAAAABgAAAAAAAAABAAAACwAAAAEAAAAAAAAA"
+ + "BQAAAAAAAAABAAAAAQAAAAUAAAAAAAAABwAAAKwCAACOAgAAAAAAAAEAAQABAAAAJAEAAAQAAABw"
+ + "EAEAAAAOABgADgAGPGluaXQ+AAFJABhMYXJ0L1Rlc3QyMDM2JFRyYW5zZm9ybTsADkxhcnQvVGVz"
+ + "dDIwMzY7ACJMZGFsdmlrL2Fubm90YXRpb24vRW5jbG9zaW5nQ2xhc3M7AB5MZGFsdmlrL2Fubm90"
+ + "YXRpb24vSW5uZXJDbGFzczsAEkxqYXZhL2xhbmcvT2JqZWN0OwANVGVzdDIwMzYuamF2YQAJVHJh"
+ + "bnNmb3JtAAFWAAthY2Nlc3NGbGFncwADYmFyAARuYW1lAAV2YWx1ZQCLAX5+RDh7ImNvbXBpbGF0"
+ + "aW9uLW1vZGUiOiJkZWJ1ZyIsImhhcy1jaGVja3N1bXMiOmZhbHNlLCJtaW4tYXBpIjoxLCJzaGEt"
+ + "MSI6IjJkMDE0MjEyNjVlMjUxMWM5MGUxZTY2NTU0NWEzMzliM2M5OWMwYWYiLCJ2ZXJzaW9uIjoi"
+ + "Mi4yLjMtZGV2In0AAgMBDRgCAgQCCgQJDBcIAAEBAAABAIGABIwCAAAAAAAAAgAAAH8CAACFAgAA"
+ + "oAIAAAAAAAAAAAAAAAAAAA8AAAAAAAAAAQAAAAAAAAABAAAADwAAAHAAAAACAAAABwAAAKwAAAAD"
+ + "AAAAAQAAAMgAAAAEAAAAAQAAANQAAAAFAAAAAgAAANwAAAAGAAAAAQAAAOwAAAABIAAAAQAAAAwB"
+ + "AAADIAAAAQAAACQBAAACIAAADwAAACgBAAAEIAAAAgAAAH8CAAAAIAAAAQAAAI4CAAADEAAAAgAA"
+ + "AJwCAAAGIAAAAQAAAKwCAAAAEAAAAQAAALwCAAA=");
+ /*
+ * base64 encoded class/dex file for
+ * Base64 generated using:
+ * % javac Test2036.java
+ * % d8 Test2035\$SubSubTransform.class
+ * % base64 classes.dex| sed 's:^:":' | sed 's:$:" +:'
+ *
+ * package art;
+ * public static class SubSubTransform extends SubTransform {
+ * public SubTransform() {}
+ * public static int FooBar() { return 0xDEADBEEF; }
+ * public int foo;
+ * public static double STATIC_FOO;
+ * }
+ */
+ private static final byte[] DEX_BYTES_SUB_SUB =
+ Base64.getDecoder()
+ .decode(
+ "ZGV4CjAzNQATpeOoP5d8wSzfcaS99KwXbb1DsEne1tDsAwAAcAAAAHhWNBIAAAAAAAAAADQDAAAS"
+ + "AAAAcAAAAAgAAAC4AAAAAgAAANgAAAACAAAA8AAAAAMAAAAAAQAAAQAAABgBAAC0AgAAOAEAAHAB"
+ + "AAB4AQAAewEAAIMBAACGAQAApgEAAMMBAADTAQAA9wEAABcCAAAjAgAANAIAAEMCAABGAgAAUwIA"
+ + "AFgCAABeAgAAZQIAAAEAAAADAAAABAAAAAUAAAAGAAAABwAAAAgAAAAMAAAAAwAAAAEAAAAAAAAA"
+ + "DAAAAAcAAAAAAAAAAgAAAAkAAAACAAEADgAAAAIAAQAAAAAAAgAAAAIAAAADAAEAAAAAAAIAAAAB"
+ + "AAAAAwAAAAAAAAALAAAAJAMAAAIDAAAAAAAAAQAAAAAAAABoAQAABAAAABQA776t3g8AAQABAAEA"
+ + "AABsAQAABAAAAHAQAgAAAA4AIQAOACAADgAGPGluaXQ+AAFEAAZGb29CYXIAAUkAHkxhcnQvVGVz"
+ + "dDIwMzYkU3ViU3ViVHJhbnNmb3JtOwAbTGFydC9UZXN0MjAzNiRTdWJUcmFuc2Zvcm07AA5MYXJ0"
+ + "L1Rlc3QyMDM2OwAiTGRhbHZpay9hbm5vdGF0aW9uL0VuY2xvc2luZ0NsYXNzOwAeTGRhbHZpay9h"
+ + "bm5vdGF0aW9uL0lubmVyQ2xhc3M7AApTVEFUSUNfRk9PAA9TdWJTdWJUcmFuc2Zvcm0ADVRlc3Qy"
+ + "MDM2LmphdmEAAVYAC2FjY2Vzc0ZsYWdzAANmb28ABG5hbWUABXZhbHVlAIsBfn5EOHsiY29tcGls"
+ + "YXRpb24tbW9kZSI6ImRlYnVnIiwiaGFzLWNoZWNrc3VtcyI6ZmFsc2UsIm1pbi1hcGkiOjEsInNo"
+ + "YS0xIjoiMmQwMTQyMTI2NWUyNTExYzkwZTFlNjY1NTQ1YTMzOWIzYzk5YzBhZiIsInZlcnNpb24i"
+ + "OiIyLjIuMy1kZXYifQACBQEQGAQCBgINBAkPFwoBAQIAAAkBAQCBgATQAgEJuAIAAAAAAgAAAPMC"
+ + "AAD5AgAAGAMAAAAAAAAAAAAAAAAAAA8AAAAAAAAAAQAAAAAAAAABAAAAEgAAAHAAAAACAAAACAAA"
+ + "ALgAAAADAAAAAgAAANgAAAAEAAAAAgAAAPAAAAAFAAAAAwAAAAABAAAGAAAAAQAAABgBAAABIAAA"
+ + "AgAAADgBAAADIAAAAgAAAGgBAAACIAAAEgAAAHABAAAEIAAAAgAAAPMCAAAAIAAAAQAAAAIDAAAD"
+ + "EAAAAgAAABQDAAAGIAAAAQAAACQDAAAAEAAAAQAAADQDAAA=");
+
+ public static void run() throws Exception {
+ Redefinition.setTestConfiguration(Redefinition.Config.COMMON_REDEFINE);
+ doTest();
+ }
+
+ public static void doTest() throws Exception {
+ SubSubSubTransform t = new SubSubSubTransform();
+ System.out.println("value-getter is " + t.getFoo());
+ System.out.println("value is " + t.foo);
+ System.out.println("static-call is " + SubSubSubTransform.FooBar());
+ System.out.println("static-value is " + SubSubSubTransform.STATIC_FOO);
+ Redefinition.doMultiStructuralClassRedefinition(
+ new Redefinition.DexOnlyClassDefinition(Transform.class, DEX_BYTES),
+ new Redefinition.DexOnlyClassDefinition(SubSubTransform.class, DEX_BYTES_SUB_SUB));
+ System.out.println("value-getter is " + t.getFoo());
+ System.out.println("value is " + t.foo);
+ System.out.println("static-call is " + SubSubSubTransform.FooBar());
+ System.out.println("static-value is " + SubSubSubTransform.STATIC_FOO);
+ }
+}
diff --git a/test/2036-structural-subclass-shadow/src/Main.java b/test/2036-structural-subclass-shadow/src/Main.java
new file mode 100644
index 0000000..e0477b0
--- /dev/null
+++ b/test/2036-structural-subclass-shadow/src/Main.java
@@ -0,0 +1,21 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+public class Main {
+ public static void main(String[] args) throws Exception {
+ System.out.println("FAIL: Test is only for art!");
+ }
+}
diff --git a/test/knownfailures.json b/test/knownfailures.json
index 398f123..3c72608 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1222,7 +1222,8 @@
"2005-pause-all-redefine-multithreaded",
"2006-virtual-structural-finalizing",
"2007-virtual-structural-finalizable",
- "2035-structural-native-method"],
+ "2035-structural-native-method",
+ "2036-structural-subclass-shadow"],
"variant": "jvm",
"description": ["Doesn't run on RI."]
},
diff --git a/tools/veridex/flow_analysis.cc b/tools/veridex/flow_analysis.cc
index 65f2363..2a8b8a0 100644
--- a/tools/veridex/flow_analysis.cc
+++ b/tools/veridex/flow_analysis.cc
@@ -47,6 +47,9 @@
}
bool VeriFlowAnalysis::MergeRegisterValues(uint32_t dex_pc) {
+ if (dex_pc >= code_item_accessor_.InsnsSizeInCodeUnits()) {
+ return false;
+ }
// TODO: Do the merging. Right now, just return that we should continue
// the iteration if the instruction has not been visited.
if (!instruction_infos_[dex_pc].has_been_visited) {
@@ -85,9 +88,14 @@
}
// Iterate over all instructions and find branching instructions.
+ const uint32_t max_pc = code_item_accessor_.InsnsSizeInCodeUnits();
for (const DexInstructionPcPair& pair : code_item_accessor_) {
const uint32_t dex_pc = pair.DexPc();
const Instruction& instruction = pair.Inst();
+ if (dex_pc >= max_pc) {
+ // We need to prevent abnormal access for outside of code
+ break;
+ }
if (instruction.IsBranch()) {
SetAsBranchTarget(dex_pc + instruction.GetTargetOffset());
@@ -107,22 +115,32 @@
RegisterSource kind,
VeriClass* cls,
uint32_t source_id) {
- current_registers_[dex_register] = RegisterValue(
- kind, DexFileReference(&resolver_->GetDexFile(), source_id), cls);
+ // veridex doesn't do any code verification, so it can be that there are bogus dex
+ // instructions that update a non-existent register.
+ if (dex_register < current_registers_.size()) {
+ current_registers_[dex_register] = RegisterValue(
+ kind, DexFileReference(&resolver_->GetDexFile(), source_id), cls);
+ }
}
void VeriFlowAnalysis::UpdateRegister(uint32_t dex_register, const RegisterValue& value) {
- current_registers_[dex_register] = value;
+ if (dex_register < current_registers_.size()) {
+ current_registers_[dex_register] = value;
+ }
}
void VeriFlowAnalysis::UpdateRegister(uint32_t dex_register, const VeriClass* cls) {
- current_registers_[dex_register] =
- RegisterValue(RegisterSource::kNone, DexFileReference(nullptr, 0), cls);
+ if (dex_register < current_registers_.size()) {
+ current_registers_[dex_register] =
+ RegisterValue(RegisterSource::kNone, DexFileReference(nullptr, 0), cls);
+ }
}
void VeriFlowAnalysis::UpdateRegister(uint32_t dex_register, int32_t value, const VeriClass* cls) {
- current_registers_[dex_register] =
- RegisterValue(RegisterSource::kConstant, value, DexFileReference(nullptr, 0), cls);
+ if (dex_register < current_registers_.size()) {
+ current_registers_[dex_register] =
+ RegisterValue(RegisterSource::kConstant, value, DexFileReference(nullptr, 0), cls);
+ }
}
const RegisterValue& VeriFlowAnalysis::GetRegister(uint32_t dex_register) const {
@@ -199,7 +217,12 @@
work_list.pop_back();
CHECK(IsBranchTarget(dex_pc));
current_registers_ = *dex_registers_[dex_pc].get();
+ const uint32_t max_pc = code_item_accessor_.InsnsSizeInCodeUnits();
while (true) {
+ if (dex_pc >= max_pc) {
+ // We need to prevent abnormal access for outside of code
+ break;
+ }
const uint16_t* insns = code_item_accessor_.Insns() + dex_pc;
const Instruction& inst = *Instruction::At(insns);
ProcessDexInstruction(inst);
diff --git a/tools/veridex/hidden_api_finder.cc b/tools/veridex/hidden_api_finder.cc
index e740cf4..6a5c82e 100644
--- a/tools/veridex/hidden_api_finder.cc
+++ b/tools/veridex/hidden_api_finder.cc
@@ -61,7 +61,14 @@
for (ClassAccessor accessor : dex_file.GetClasses()) {
if (class_filter.Matches(accessor.GetDescriptor())) {
for (const ClassAccessor::Method& method : accessor.GetMethods()) {
- for (const DexInstructionPcPair& inst : method.GetInstructions()) {
+ CodeItemInstructionAccessor codes = method.GetInstructions();
+ const uint32_t max_pc = codes.InsnsSizeInCodeUnits();
+ for (const DexInstructionPcPair& inst : codes) {
+ if (inst.DexPc() >= max_pc) {
+ // We need to prevent abnormal access for outside of code
+ break;
+ }
+
switch (inst->Opcode()) {
case Instruction::CONST_STRING: {
dex::StringIndex string_index(inst->VRegB_21c());