Skip to content

Commit cc54d2c

Browse files
author
Ashutosh Mehra
committed
8371418: Methods in AdapterHandlerLibrary use HashtableBase iterate method incorrectly
Reviewed-by: kvn, adinn
1 parent 8a911ae commit cc54d2c

File tree

7 files changed

+65
-37
lines changed

7 files changed

+65
-37
lines changed

src/hotspot/share/cds/lambdaProxyClassDictionary.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,7 @@ void LambdaProxyClassDictionary::print_on(const char* prefix,
518518
if (!dictionary->empty()) {
519519
st->print_cr("%sShared Lambda Dictionary", prefix);
520520
SharedLambdaDictionaryPrinter ldp(st, start_index);
521-
dictionary->iterate(&ldp);
521+
dictionary->iterate_all(&ldp);
522522
}
523523
}
524524

src/hotspot/share/classfile/compactHashtable.hpp

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,7 @@ class CompactHashtable : public SimpleCompactHashtable {
294294
return nullptr;
295295
}
296296

297+
// Iterate through the values in the table, stopping when do_value() returns false.
297298
template <class ITER>
298299
inline void iterate(ITER* iter) const { iterate([&](V v) { iter->do_value(v); }); }
299300

@@ -302,6 +303,7 @@ class CompactHashtable : public SimpleCompactHashtable {
302303
iterate(const_cast<Function&>(function));
303304
}
304305

306+
// Iterate through the values in the table, stopping when the lambda returns false.
305307
template<typename Function>
306308
inline void iterate(Function& function) const { // lambda enabled API
307309
for (u4 i = 0; i < _bucket_count; i++) {
@@ -311,17 +313,35 @@ class CompactHashtable : public SimpleCompactHashtable {
311313
u4* entry = _entries + bucket_offset;
312314

313315
if (bucket_type == VALUE_ONLY_BUCKET_TYPE) {
314-
function(decode(entry[0]));
316+
if (!function(decode(entry[0]))) {
317+
return;
318+
}
315319
} else {
316320
u4* entry_max = _entries + BUCKET_OFFSET(_buckets[i + 1]);
317321
while (entry < entry_max) {
318-
function(decode(entry[1]));
322+
if (!function(decode(entry[1]))) {
323+
return;
324+
}
319325
entry += 2;
320326
}
321327
}
322328
}
323329
}
324330

331+
// Unconditionally iterate through all the values in the table
332+
template <class ITER>
333+
inline void iterate_all(ITER* iter) const { iterate_all([&](V v) { iter->do_value(v); }); }
334+
335+
// Unconditionally iterate through all the values in the table using lambda
336+
template<typename Function>
337+
void iterate_all(Function function) const { // lambda enabled API
338+
auto wrapper = [&] (V v) {
339+
function(v);
340+
return true;
341+
};
342+
iterate(wrapper);
343+
}
344+
325345
void print_table_statistics(outputStream* st, const char* name) {
326346
st->print_cr("%s statistics:", name);
327347
int total_entries = 0;

src/hotspot/share/classfile/stringTable.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -908,7 +908,7 @@ void StringTable::dump(outputStream* st, bool verbose) {
908908
st->print_cr("# Shared strings:");
909909
st->print_cr("#----------------");
910910
PrintSharedString pss(thr, st);
911-
_shared_table.iterate(&pss);
911+
_shared_table.iterate_all(&pss);
912912
}
913913
#endif
914914
}

src/hotspot/share/classfile/symbolTable.cpp

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -272,9 +272,7 @@ class SharedSymbolIterator {
272272
void SymbolTable::symbols_do(SymbolClosure *cl) {
273273
assert(SafepointSynchronize::is_at_safepoint(), "Must be at safepoint");
274274
// all symbols from shared table
275-
SharedSymbolIterator iter(cl);
276-
_shared_table.iterate(&iter);
277-
_dynamic_shared_table.iterate(&iter);
275+
shared_symbols_do(cl);
278276

279277
// all symbols from the dynamic table
280278
SymbolsDo sd(cl);
@@ -284,8 +282,8 @@ void SymbolTable::symbols_do(SymbolClosure *cl) {
284282
// Call function for all symbols in shared table. Used by -XX:+PrintSharedArchiveAndExit
285283
void SymbolTable::shared_symbols_do(SymbolClosure *cl) {
286284
SharedSymbolIterator iter(cl);
287-
_shared_table.iterate(&iter);
288-
_dynamic_shared_table.iterate(&iter);
285+
_shared_table.iterate_all(&iter);
286+
_dynamic_shared_table.iterate_all(&iter);
289287
}
290288

291289
Symbol* SymbolTable::lookup_dynamic(const char* name,
@@ -669,14 +667,14 @@ void SymbolTable::dump(outputStream* st, bool verbose) {
669667
st->print_cr("# Shared symbols:");
670668
st->print_cr("#----------------");
671669
DumpSharedSymbol dss(st);
672-
_shared_table.iterate(&dss);
670+
_shared_table.iterate_all(&dss);
673671
}
674672
if (!_dynamic_shared_table.empty()) {
675673
st->print_cr("#------------------------");
676674
st->print_cr("# Dynamic shared symbols:");
677675
st->print_cr("#------------------------");
678676
DumpSharedSymbol dss(st);
679-
_dynamic_shared_table.iterate(&dss);
677+
_dynamic_shared_table.iterate_all(&dss);
680678
}
681679
}
682680
}

src/hotspot/share/classfile/systemDictionaryShared.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1431,11 +1431,11 @@ const char* SystemDictionaryShared::loader_type_for_shared_class(Klass* k) {
14311431
}
14321432

14331433
void SystemDictionaryShared::get_all_archived_classes(bool is_static_archive, GrowableArray<Klass*>* classes) {
1434-
get_archive(is_static_archive)->_builtin_dictionary.iterate([&] (const RunTimeClassInfo* record) {
1434+
get_archive(is_static_archive)->_builtin_dictionary.iterate_all([&] (const RunTimeClassInfo* record) {
14351435
classes->append(record->klass());
14361436
});
14371437

1438-
get_archive(is_static_archive)->_unregistered_dictionary.iterate([&] (const RunTimeClassInfo* record) {
1438+
get_archive(is_static_archive)->_unregistered_dictionary.iterate_all([&] (const RunTimeClassInfo* record) {
14391439
classes->append(record->klass());
14401440
});
14411441
}
@@ -1464,9 +1464,9 @@ void SystemDictionaryShared::ArchiveInfo::print_on(const char* prefix,
14641464
st->print_cr("%sShared Dictionary", prefix);
14651465
SharedDictionaryPrinter p(st);
14661466
st->print_cr("%sShared Builtin Dictionary", prefix);
1467-
_builtin_dictionary.iterate(&p);
1467+
_builtin_dictionary.iterate_all(&p);
14681468
st->print_cr("%sShared Unregistered Dictionary", prefix);
1469-
_unregistered_dictionary.iterate(&p);
1469+
_unregistered_dictionary.iterate_all(&p);
14701470
LambdaProxyClassDictionary::print_on(prefix, st, p.index(), is_static_archive);
14711471
}
14721472

src/hotspot/share/oops/trainingData.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ static void verify_archived_entry(TrainingData* td, const TrainingData::Key* k)
8383

8484
void TrainingData::verify() {
8585
if (TrainingData::have_data() && !TrainingData::assembling_data()) {
86-
archived_training_data_dictionary()->iterate([&](TrainingData* td) {
86+
archived_training_data_dictionary()->iterate_all([&](TrainingData* td) {
8787
if (td->is_KlassTrainingData()) {
8888
KlassTrainingData* ktd = td->as_KlassTrainingData();
8989
if (ktd->has_holder() && ktd->holder()->is_loaded()) {
@@ -466,7 +466,7 @@ void TrainingData::init_dumptime_table(TRAPS) {
466466
precond((!assembling_data() && !need_data()) || need_data() != assembling_data());
467467
if (assembling_data()) {
468468
_dumptime_training_data_dictionary = new DumptimeTrainingDataDictionary();
469-
_archived_training_data_dictionary.iterate([&](TrainingData* record) {
469+
_archived_training_data_dictionary.iterate_all([&](TrainingData* record) {
470470
_dumptime_training_data_dictionary->append(record);
471471
});
472472
}
@@ -692,7 +692,7 @@ void TrainingData::print_archived_training_data_on(outputStream* st) {
692692
st->print_cr("Archived TrainingData Dictionary");
693693
TrainingDataPrinter tdp(st);
694694
TrainingDataLocker::initialize();
695-
_archived_training_data_dictionary.iterate(&tdp);
695+
_archived_training_data_dictionary.iterate_all(&tdp);
696696
}
697697

698698
void TrainingData::Key::metaspace_pointers_do(MetaspaceClosure *iter) {

src/hotspot/share/runtime/sharedRuntime.cpp

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3036,7 +3036,7 @@ void AdapterHandlerLibrary::link_aot_adapters() {
30363036
* result in collision of adapter ids between AOT stored handlers and runtime generated handlers.
30373037
* To avoid such situation, initialize the _id_counter with the largest adapter id among the AOT stored handlers.
30383038
*/
3039-
_aot_adapter_handler_table.iterate([&](AdapterHandlerEntry* entry) {
3039+
_aot_adapter_handler_table.iterate_all([&](AdapterHandlerEntry* entry) {
30403040
assert(!entry->is_linked(), "AdapterHandlerEntry is already linked!");
30413041
entry->link();
30423042
max_id = MAX2(max_id, entry->id());
@@ -3388,34 +3388,44 @@ JRT_LEAF(void, SharedRuntime::OSR_migration_end( intptr_t* buf) )
33883388
FREE_C_HEAP_ARRAY(intptr_t, buf);
33893389
JRT_END
33903390

3391+
const char* AdapterHandlerLibrary::name(AdapterHandlerEntry* handler) {
3392+
return handler->fingerprint()->as_basic_args_string();
3393+
}
3394+
3395+
uint32_t AdapterHandlerLibrary::id(AdapterHandlerEntry* handler) {
3396+
return handler->id();
3397+
}
3398+
33913399
bool AdapterHandlerLibrary::contains(const CodeBlob* b) {
33923400
bool found = false;
33933401
#if INCLUDE_CDS
33943402
if (AOTCodeCache::is_using_adapter()) {
33953403
auto findblob_archived_table = [&] (AdapterHandlerEntry* handler) {
3396-
return (found = (b == CodeCache::find_blob(handler->get_i2c_entry())));
3404+
if (b == CodeCache::find_blob(handler->get_i2c_entry())) {
3405+
found = true;
3406+
return false; // abort iteration
3407+
} else {
3408+
return true; // keep looking
3409+
}
33973410
};
33983411
_aot_adapter_handler_table.iterate(findblob_archived_table);
33993412
}
34003413
#endif // INCLUDE_CDS
34013414
if (!found) {
3402-
auto findblob_runtime_table = [&] (AdapterFingerPrint* key, AdapterHandlerEntry* a) {
3403-
return (found = (b == CodeCache::find_blob(a->get_i2c_entry())));
3415+
auto findblob_runtime_table = [&] (AdapterFingerPrint* key, AdapterHandlerEntry* handler) {
3416+
if (b == CodeCache::find_blob(handler->get_i2c_entry())) {
3417+
found = true;
3418+
return false; // abort iteration
3419+
} else {
3420+
return true; // keep looking
3421+
}
34043422
};
34053423
assert_locked_or_safepoint(AdapterHandlerLibrary_lock);
34063424
_adapter_handler_table->iterate(findblob_runtime_table);
34073425
}
34083426
return found;
34093427
}
34103428

3411-
const char* AdapterHandlerLibrary::name(AdapterHandlerEntry* handler) {
3412-
return handler->fingerprint()->as_basic_args_string();
3413-
}
3414-
3415-
uint32_t AdapterHandlerLibrary::id(AdapterHandlerEntry* handler) {
3416-
return handler->id();
3417-
}
3418-
34193429
void AdapterHandlerLibrary::print_handler_on(outputStream* st, const CodeBlob* b) {
34203430
bool found = false;
34213431
#if INCLUDE_CDS
@@ -3425,23 +3435,23 @@ void AdapterHandlerLibrary::print_handler_on(outputStream* st, const CodeBlob* b
34253435
found = true;
34263436
st->print("Adapter for signature: ");
34273437
handler->print_adapter_on(st);
3428-
return true;
3438+
return false; // abort iteration
34293439
} else {
3430-
return false; // keep looking
3440+
return true; // keep looking
34313441
}
34323442
};
34333443
_aot_adapter_handler_table.iterate(findblob_archived_table);
34343444
}
34353445
#endif // INCLUDE_CDS
34363446
if (!found) {
3437-
auto findblob_runtime_table = [&] (AdapterFingerPrint* key, AdapterHandlerEntry* a) {
3438-
if (b == CodeCache::find_blob(a->get_i2c_entry())) {
3447+
auto findblob_runtime_table = [&] (AdapterFingerPrint* key, AdapterHandlerEntry* handler) {
3448+
if (b == CodeCache::find_blob(handler->get_i2c_entry())) {
34393449
found = true;
34403450
st->print("Adapter for signature: ");
3441-
a->print_adapter_on(st);
3442-
return true;
3451+
handler->print_adapter_on(st);
3452+
return false; // abort iteration
34433453
} else {
3444-
return false; // keep looking
3454+
return true; // keep looking
34453455
}
34463456
};
34473457
assert_locked_or_safepoint(AdapterHandlerLibrary_lock);

0 commit comments

Comments
 (0)