Revert "Revert "Fixed Literal String intern mismatch.""
This reverts commit f7ab8348b88b7ce63c5fa112f5a71756da541763.
Test: make test-ar-host -j64
Change-Id: I688b7c7905c8be575959fa78ad594ac1c782af9a
diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc
index 622448f..7e00471 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -2406,30 +2406,6 @@
}
}
- bool NoPotentialInternStrings(Handle<mirror::Class> klass,
- Handle<mirror::ClassLoader>* class_loader)
- REQUIRES_SHARED(Locks::mutator_lock_) {
- StackHandleScope<1> hs(Thread::Current());
- Handle<mirror::DexCache> h_dex_cache = hs.NewHandle(klass->GetDexCache());
- const DexFile* dex_file = h_dex_cache->GetDexFile();
- const DexFile::ClassDef* class_def = klass->GetClassDef();
- annotations::RuntimeEncodedStaticFieldValueIterator value_it(*dex_file,
- &h_dex_cache,
- class_loader,
- manager_->GetClassLinker(),
- *class_def);
-
- const auto jString = annotations::RuntimeEncodedStaticFieldValueIterator::kString;
- for ( ; value_it.HasNext(); value_it.Next()) {
- if (value_it.GetValueType() == jString) {
- // We don't want cache the static encoded strings which is a potential intern.
- return false;
- }
- }
-
- return true;
- }
-
bool ResolveTypesOfMethods(Thread* self, ArtMethod* m)
REQUIRES_SHARED(Locks::mutator_lock_) {
auto rtn_type = m->GetReturnType(true); // return value is discarded because resolve will be done internally.
@@ -2559,7 +2535,7 @@
}
}
- return NoPotentialInternStrings(klass, class_loader);
+ return true;
}
const ParallelCompilationManager* const manager_;
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 928645a..3f86516 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -55,6 +55,7 @@
#include "gc_root-inl.h"
#include "gc/accounting/card_table-inl.h"
#include "gc/accounting/heap_bitmap-inl.h"
+#include "gc/accounting/space_bitmap-inl.h"
#include "gc/heap.h"
#include "gc/scoped_gc_critical_section.h"
#include "gc/space/image_space.h"
@@ -88,6 +89,7 @@
#include "mirror/method_handles_lookup.h"
#include "mirror/object-inl.h"
#include "mirror/object_array-inl.h"
+#include "mirror/object-refvisitor-inl.h"
#include "mirror/proxy.h"
#include "mirror/reference-inl.h"
#include "mirror/stack_trace_element.h"
@@ -1193,6 +1195,63 @@
gc::accounting::HeapBitmap* const live_bitmap_;
};
+class FixupInternVisitor {
+ public:
+ ALWAYS_INLINE ObjPtr<mirror::Object> TryInsertIntern(mirror::Object* obj) const
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ if (obj != nullptr && obj->IsString()) {
+ const auto intern = Runtime::Current()->GetInternTable()->InternStrong(obj->AsString());
+ return intern;
+ }
+ return obj;
+ }
+
+ ALWAYS_INLINE void VisitRootIfNonNull(
+ mirror::CompressedReference<mirror::Object>* root) const
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ if (!root->IsNull()) {
+ VisitRoot(root);
+ }
+ }
+
+ ALWAYS_INLINE void VisitRoot(mirror::CompressedReference<mirror::Object>* root) const
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ root->Assign(TryInsertIntern(root->AsMirrorPtr()));
+ }
+
+ // Visit Class Fields
+ ALWAYS_INLINE void operator()(ObjPtr<mirror::Object> obj,
+ MemberOffset offset,
+ bool is_static ATTRIBUTE_UNUSED) const
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ // There could be overlap between ranges, we must avoid visiting the same reference twice.
+ // Avoid the class field since we already fixed it up in FixupClassVisitor.
+ if (offset.Uint32Value() != mirror::Object::ClassOffset().Uint32Value()) {
+ // Updating images, don't do a read barrier.
+ // Only string fields are fixed, don't do a verify.
+ mirror::Object* ref = obj->GetFieldObject<mirror::Object, kVerifyNone, kWithoutReadBarrier>(
+ offset);
+ obj->SetFieldObject<false, false>(offset, TryInsertIntern(ref));
+ }
+ }
+
+ void operator()(ObjPtr<mirror::Class> klass ATTRIBUTE_UNUSED,
+ ObjPtr<mirror::Reference> ref) const
+ REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(Locks::heap_bitmap_lock_) {
+ this->operator()(ref, mirror::Reference::ReferentOffset(), false);
+ }
+
+ void operator()(mirror::Object* obj) const
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ if (obj->IsDexCache()) {
+ obj->VisitReferences<true, kVerifyNone, kWithoutReadBarrier>(*this, *this);
+ } else {
+ // Don't visit native roots for non-dex-cache
+ obj->VisitReferences<false, kVerifyNone, kWithoutReadBarrier>(*this, *this);
+ }
+ }
+};
+
// Copies data from one array to another array at the same position
// if pred returns false. If there is a page of continuous data in
// the src array for which pred consistently returns true then
@@ -1285,6 +1344,7 @@
return false;
}
}
+
// Only add the classes to the class loader after the points where we can return false.
for (size_t i = 0; i < num_dex_caches; i++) {
ObjPtr<mirror::DexCache> dex_cache = dex_caches->Get(i);
@@ -1448,6 +1508,21 @@
}
}
}
+ {
+ // Fixup all the literal strings happens at app images which are supposed to be interned.
+ ScopedTrace timing("Fixup String Intern in image and dex_cache");
+ const auto& image_header = space->GetImageHeader();
+ const auto bitmap = space->GetMarkBitmap(); // bitmap of objects
+ const uint8_t* target_base = space->GetMemMap()->Begin();
+ const ImageSection& objects_section =
+ image_header.GetImageSection(ImageHeader::kSectionObjects);
+
+ uintptr_t objects_begin = reinterpret_cast<uintptr_t>(target_base + objects_section.Offset());
+ uintptr_t objects_end = reinterpret_cast<uintptr_t>(target_base + objects_section.End());
+
+ FixupInternVisitor fixup_intern_visitor;
+ bitmap->VisitMarkedRange(objects_begin, objects_end, fixup_intern_visitor);
+ }
if (*out_forward_dex_cache_array) {
ScopedTrace timing("Fixup ArtMethod dex cache arrays");
FixupArtMethodArrayVisitor visitor(header);
diff --git a/test/596-app-images/src/Main.java b/test/596-app-images/src/Main.java
index 8ee3c88..88d95f4 100644
--- a/test/596-app-images/src/Main.java
+++ b/test/596-app-images/src/Main.java
@@ -14,6 +14,10 @@
* limitations under the License.
*/
+import java.lang.reflect.Field;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Method;
+
class Main {
static class Inner {
final public static int abc = 10;
@@ -46,13 +50,76 @@
if (!checkInitialized(StaticFieldsInit.class))
System.out.println("StaticFieldsInit class is not initialized!");
- if (checkInitialized(StaticInternString.class))
- System.out.println("StaticInternString class is initialized!");
+ if (!checkInitialized(StaticInternString.class))
+ System.out.println("StaticInternString class is not initialized!");
+
+ StringBuffer sb = new StringBuffer();
+ sb.append("java.");
+ sb.append("abc.");
+ sb.append("Action");
+
+ String tmp = sb.toString();
+ String intern = tmp.intern();
+
+ assertNotEqual(tmp, intern, "Dynamically constructed String, not interned.");
+ assertEqual(intern, StaticInternString.intent, "Static encoded literal String not interned.");
+ assertEqual(BootInternedString.boot, BootInternedString.boot.intern(),
+ "Static encoded literal String not moved back to runtime intern table.");
+
+ try {
+ Field f = StaticInternString.class.getDeclaredField("intent");
+ assertEqual(intern, f.get(null), "String Literals are not interned properly.");
+
+ } catch (Exception e) {
+ System.out.println("Exception");
+ }
+
+ assertEqual(StaticInternString.getIntent(), StaticInternString2.getIntent(),
+ "String Literals are not intenred properly, App image static strings duplicated.");
+
+ // reload the class StaticInternString, check whether static strings interned properly
+ final String DEX_FILE = System.getenv("DEX_LOCATION") + "/596-app-images.jar";
+ final String LIBRARY_SEARCH_PATH = System.getProperty("java.library.path");
+
+ try {
+ Class<?> pathClassLoader = Class.forName("dalvik.system.PathClassLoader");
+ if (pathClassLoader == null) {
+ throw new AssertionError("Counldn't find path class loader class");
+ }
+ Constructor<?> ctor =
+ pathClassLoader.getDeclaredConstructor(String.class, String.class, ClassLoader.class);
+ ClassLoader loader = (ClassLoader) ctor.newInstance(
+ DEX_FILE, LIBRARY_SEARCH_PATH, null);
+
+ Class<?> staticInternString = loader.loadClass("StaticInternString");
+
+ if (!checkAppImageContains(staticInternString)) {
+ System.out.println("Not loaded again.");
+ }
+ Method getIntent = staticInternString.getDeclaredMethod("getIntent");
+
+ assertEqual(StaticInternString.getIntent(), getIntent.invoke(staticInternString),
+ "Dynamically loaded app image's literal strings not interned properly.");
+ } catch (Exception e) {
+ e.printStackTrace(System.out);
+ }
+
}
public static native boolean checkAppImageLoaded();
public static native boolean checkAppImageContains(Class<?> klass);
public static native boolean checkInitialized(Class<?> klass);
+
+ public static void assertEqual(Object a, Object b, String msg) {
+ if (a != b)
+ System.out.println(msg);
+ }
+
+ public static void assertNotEqual(Object a, Object b, String msg) {
+ if (a == b)
+ System.out.println(msg);
+ }
+
}
class StaticFields{
@@ -68,6 +135,21 @@
}
class StaticInternString {
- final public static String intern = "java.abc.Action";
+ final public static String intent = "java.abc.Action";
+ static public String getIntent() {
+ return intent;
+ }
+}
+
+class BootInternedString {
+ final public static String boot = "double";
+}
+
+class StaticInternString2 {
+ final public static String intent = "java.abc.Action";
+
+ static String getIntent() {
+ return intent;
+ }
}