On Tue, 13 Oct 2020 06:46:27 GMT, Ioi Lam <ik...@openjdk.org> wrote: >> **Problem:** when iterating over the cloned vtables, the original code >> assumes that they are laid out consecutively in >> memory. However, since >> [JDK-8224509](https://bugs.openjdk.java.net/browse/JDK-8224509), the memory >> allocated for each >> of the the cloned vtables is now 8-byte aligned. This introduces gaps >> between the cloned vtables, and causes the assert >> to fail. >> >> **Fix:** the fix is to no longer assume the consecutive memory layout. >> Instead, use the CppVtables::_index array to >> access each individual cloned vtable. >> >> **Note:** I also cleaned up the code significantly. I feel the original code >> is pretty hard to understand, so if I just >> do the bare minimum to fix the bug, it will be pretty hard to review. >> >> I would suggest that the reviewers look at just the new version of the code >> and see if it's working as described >> (instead of looking at the diff to understand what the bug was and how it >> has been fixed). >> This version still uses the x-macro CPP_VTABLE_TYPES_DO to enumerate over >> the classes whose vtables need to be cloned. >> I plan to change that into templates in a future RFE. > > Ioi Lam has updated the pull request with a new target base due to a merge or > a rebase. The incremental webrev excludes > the unrelated changes brought in by the merge/rebase. The pull request > contains four additional commits since the last > revision: > - Merge branch 'master' of https://github.com/openjdk/jdk into > 8254125-cppvtables-assert-on-win32 > - Fixed more cases with align_up(xxx, SharedSpaceObjectAlignment) > - Merge branch 'master' into 8254125-cppvtables-assert-on-win32 > - 8254125: Assertion in cppVtables.cpp during builds on 32bit Windows
Marked as reviewed by ccheung (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/591
