8220496: Race in java_lang_String::length() when deduplicating
Reviewed-by: rkennke, pliden
--- a/src/hotspot/share/classfile/javaClasses.cpp Wed Mar 13 15:22:28 2019 +0100
+++ b/src/hotspot/share/classfile/javaClasses.cpp Wed Mar 13 17:32:17 2019 +0100
@@ -578,7 +578,7 @@
}
int java_lang_String::utf8_length(oop java_string, typeArrayOop value) {
- assert(oopDesc::equals(value, java_lang_String::value(java_string)),
+ assert(value_equals(value, java_lang_String::value(java_string)),
"value must be same as java_lang_String::value(java_string)");
int length = java_lang_String::length(java_string, value);
if (length == 0) {
@@ -610,7 +610,7 @@
}
char* java_lang_String::as_utf8_string(oop java_string, typeArrayOop value, char* buf, int buflen) {
- assert(oopDesc::equals(value, java_lang_String::value(java_string)),
+ assert(value_equals(value, java_lang_String::value(java_string)),
"value must be same as java_lang_String::value(java_string)");
int length = java_lang_String::length(java_string, value);
bool is_latin1 = java_lang_String::is_latin1(java_string);
@@ -642,7 +642,7 @@
}
char* java_lang_String::as_utf8_string(oop java_string, typeArrayOop value, int start, int len, char* buf, int buflen) {
- assert(oopDesc::equals(value, java_lang_String::value(java_string)),
+ assert(value_equals(value, java_lang_String::value(java_string)),
"value must be same as java_lang_String::value(java_string)");
assert(start + len <= java_lang_String::length(java_string), "just checking");
bool is_latin1 = java_lang_String::is_latin1(java_string);
@@ -686,24 +686,15 @@
assert(str2->klass() == SystemDictionary::String_klass(),
"must be java String");
typeArrayOop value1 = java_lang_String::value_no_keepalive(str1);
- int length1 = java_lang_String::length(str1, value1);
bool is_latin1 = java_lang_String::is_latin1(str1);
typeArrayOop value2 = java_lang_String::value_no_keepalive(str2);
- int length2 = java_lang_String::length(str2, value2);
bool is_latin2 = java_lang_String::is_latin1(str2);
- if ((length1 != length2) || (is_latin1 != is_latin2)) {
- // Strings of different size or with different
- // coders are never equal.
+ if (is_latin1 != is_latin2) {
+ // Strings with different coders are never equal.
return false;
}
- int blength1 = value1->length();
- for (int i = 0; i < blength1; i++) {
- if (value1->byte_at(i) != value2->byte_at(i)) {
- return false;
- }
- }
- return true;
+ return value_equals(value1, value2);
}
void java_lang_String::print(oop java_string, outputStream* st) {
--- a/src/hotspot/share/classfile/javaClasses.hpp Wed Mar 13 15:22:28 2019 +0100
+++ b/src/hotspot/share/classfile/javaClasses.hpp Wed Mar 13 17:32:17 2019 +0100
@@ -194,6 +194,7 @@
static bool equals(oop java_string, const jchar* chars, int len);
static bool equals(oop str1, oop str2);
+ static inline bool value_equals(typeArrayOop str_value1, typeArrayOop str_value2);
// Conversion between '.' and '/' formats
static Handle externalize_classname(Handle java_string, TRAPS) { return char_converter(java_string, '/', '.', THREAD); }
--- a/src/hotspot/share/classfile/javaClasses.inline.hpp Wed Mar 13 15:22:28 2019 +0100
+++ b/src/hotspot/share/classfile/javaClasses.inline.hpp Wed Mar 13 17:32:17 2019 +0100
@@ -39,31 +39,44 @@
assert(initialized, "Must be initialized");
string->obj_field_put_raw(value_offset, buffer);
}
+
void java_lang_String::set_value(oop string, typeArrayOop buffer) {
assert(initialized && (value_offset > 0), "Must be initialized");
string->obj_field_put(value_offset, (oop)buffer);
}
+
void java_lang_String::set_hash(oop string, unsigned int hash) {
assert(initialized && (hash_offset > 0), "Must be initialized");
string->int_field_put(hash_offset, hash);
}
// Accessors
+bool java_lang_String::value_equals(typeArrayOop str_value1, typeArrayOop str_value2) {
+ return (oopDesc::equals(str_value1, str_value2) ||
+ (str_value1->length() == str_value2->length() &&
+ (!memcmp(str_value1->base(T_BYTE),
+ str_value2->base(T_BYTE),
+ str_value2->length() * sizeof(jbyte)))));
+}
+
typeArrayOop java_lang_String::value(oop java_string) {
assert(initialized && (value_offset > 0), "Must be initialized");
assert(is_instance(java_string), "must be java_string");
return (typeArrayOop) java_string->obj_field(value_offset);
}
+
typeArrayOop java_lang_String::value_no_keepalive(oop java_string) {
assert(initialized && (value_offset > 0), "Must be initialized");
assert(is_instance(java_string), "must be java_string");
return (typeArrayOop) java_string->obj_field_access<AS_NO_KEEPALIVE>(value_offset);
}
+
unsigned int java_lang_String::hash(oop java_string) {
assert(initialized && (hash_offset > 0), "Must be initialized");
assert(is_instance(java_string), "must be java_string");
return java_string->int_field(hash_offset);
}
+
bool java_lang_String::is_latin1(oop java_string) {
assert(initialized && (coder_offset > 0), "Must be initialized");
assert(is_instance(java_string), "must be java_string");
@@ -71,11 +84,12 @@
assert(CompactStrings || coder == CODER_UTF16, "Must be UTF16 without CompactStrings");
return coder == CODER_LATIN1;
}
+
int java_lang_String::length(oop java_string, typeArrayOop value) {
assert(initialized, "Must be initialized");
assert(is_instance(java_string), "must be java_string");
- assert(oopDesc::equals(value, java_lang_String::value(java_string)),
- "value must be same as java_lang_String::value(java_string)");
+ assert(value_equals(value, java_lang_String::value(java_string)),
+ "value must be equal to java_lang_String::value(java_string)");
if (value == NULL) {
return 0;
}
@@ -86,6 +100,7 @@
}
return arr_length;
}
+
int java_lang_String::length(oop java_string) {
assert(initialized, "Must be initialized");
assert(is_instance(java_string), "must be java_string");
@@ -101,42 +116,55 @@
oop java_lang_ref_Reference::referent(oop ref) {
return ref->obj_field(referent_offset);
}
+
void java_lang_ref_Reference::set_referent(oop ref, oop value) {
ref->obj_field_put(referent_offset, value);
}
+
void java_lang_ref_Reference::set_referent_raw(oop ref, oop value) {
ref->obj_field_put_raw(referent_offset, value);
}
+
HeapWord* java_lang_ref_Reference::referent_addr_raw(oop ref) {
return ref->obj_field_addr_raw<HeapWord>(referent_offset);
}
+
oop java_lang_ref_Reference::next(oop ref) {
return ref->obj_field(next_offset);
}
+
void java_lang_ref_Reference::set_next(oop ref, oop value) {
ref->obj_field_put(next_offset, value);
}
+
void java_lang_ref_Reference::set_next_raw(oop ref, oop value) {
ref->obj_field_put_raw(next_offset, value);
}
+
HeapWord* java_lang_ref_Reference::next_addr_raw(oop ref) {
return ref->obj_field_addr_raw<HeapWord>(next_offset);
}
+
oop java_lang_ref_Reference::discovered(oop ref) {
return ref->obj_field(discovered_offset);
}
+
void java_lang_ref_Reference::set_discovered(oop ref, oop value) {
ref->obj_field_put(discovered_offset, value);
}
+
void java_lang_ref_Reference::set_discovered_raw(oop ref, oop value) {
ref->obj_field_put_raw(discovered_offset, value);
}
+
HeapWord* java_lang_ref_Reference::discovered_addr_raw(oop ref) {
return ref->obj_field_addr_raw<HeapWord>(discovered_offset);
}
+
bool java_lang_ref_Reference::is_final(oop ref) {
return InstanceKlass::cast(ref->klass())->reference_type() == REF_FINAL;
}
+
bool java_lang_ref_Reference::is_phantom(oop ref) {
return InstanceKlass::cast(ref->klass())->reference_type() == REF_PHANTOM;
}
--- a/src/hotspot/share/gc/shared/stringdedup/stringDedupTable.cpp Wed Mar 13 15:22:28 2019 +0100
+++ b/src/hotspot/share/gc/shared/stringdedup/stringDedupTable.cpp Wed Mar 13 17:32:17 2019 +0100
@@ -269,21 +269,13 @@
*list = entry;
}
-bool StringDedupTable::equals(typeArrayOop value1, typeArrayOop value2) {
- return (oopDesc::equals(value1, value2) ||
- (value1->length() == value2->length() &&
- (!memcmp(value1->base(T_BYTE),
- value2->base(T_BYTE),
- value1->length() * sizeof(jbyte)))));
-}
-
typeArrayOop StringDedupTable::lookup(typeArrayOop value, bool latin1, unsigned int hash,
StringDedupEntry** list, uintx &count) {
for (StringDedupEntry* entry = *list; entry != NULL; entry = entry->next()) {
if (entry->hash() == hash && entry->latin1() == latin1) {
oop* obj_addr = (oop*)entry->obj_addr();
oop obj = NativeAccess<ON_PHANTOM_OOP_REF | AS_NO_KEEPALIVE>::oop_load(obj_addr);
- if (equals(value, static_cast<typeArrayOop>(obj))) {
+ if (java_lang_String::value_equals(value, static_cast<typeArrayOop>(obj))) {
obj = NativeAccess<ON_PHANTOM_OOP_REF>::oop_load(obj_addr);
return static_cast<typeArrayOop>(obj);
}
@@ -633,7 +625,7 @@
while (*entry2 != NULL) {
typeArrayOop value2 = (*entry2)->obj();
bool latin1_2 = (*entry2)->latin1();
- guarantee(latin1_1 != latin1_2 || !equals(value1, value2), "Table entries must not have identical arrays");
+ guarantee(latin1_1 != latin1_2 || !java_lang_String::value_equals(value1, value2), "Table entries must not have identical arrays");
entry2 = (*entry2)->next_addr();
}
entry1 = (*entry1)->next_addr();
--- a/src/hotspot/share/gc/shared/stringdedup/stringDedupTable.hpp Wed Mar 13 15:22:28 2019 +0100
+++ b/src/hotspot/share/gc/shared/stringdedup/stringDedupTable.hpp Wed Mar 13 17:32:17 2019 +0100
@@ -199,8 +199,6 @@
return _table->_hash_seed == 0;
}
- static bool equals(typeArrayOop value1, typeArrayOop value2);
-
// Computes the hash code for the given character array, using the
// currently active hash function and hash seed.
static unsigned int hash_code(typeArrayOop value, bool latin1);