RFR: 8261918: two runtime/cds/appcds/VerifierTest failed with "Unable to use shared archive"

[Yumin Qi]

On Tue, 9 Mar 2021 23:27:42 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

> In the VerifierTest_*.java, we only dump the static archive once. When the -Xverify setting has changed for the dynamic archive, the static archive cannot be used due to different -Xverify settings in the following check in filemap.cpp:
> 
>   if (_has_platform_or_app_classes && 
>       ((!_verify_local && BytecodeVerificationLocal) || 
>        (!_verify_remote && BytecodeVerificationRemote))) { 
>     FileMapInfo::fail_continue("The shared archive file was created with less restrictive " 
>                   "verification setting than the current setting."); 
>     return false; 
>   } 
> The changes in TestCommon.java is to look for the -Xverify options in the input opts.suffix. If there exists -Xverify options and "DYNAMIC_DUMP", it will dump the static archive again using the new -Xverify option(s). 
> 
> This change also separates the above condition into 2; the `_has_platform_or_app_classes` should only be tied to `BytecodeVerificationRemote`.
> 
> Passed mach5 tiers 1 - 4 tests.
> 
> Thanks,
> Calvin

Generally LGTM, only a few comments to clear.

test/hotspot/jtreg/runtime/cds/appcds/VerifierTest.java line 45:

> 43:     static final String ERR =
> 44:         "ERROR: class VerifierTestC was loaded unexpectedly";
> 45:     static final String MAP_FAIL =

MAP_FAIL no longer used. Can be removed.

src/hotspot/share/memory/filemap.cpp line 2236:

> 2234:   // if the archive only contains system classes.
> 2235:   if (_has_platform_or_app_classes
> 2236:       && !_verify_remote // we didn't verify the archived platform/app classes

Should we use "don't verify" to keep grammar consistency with above/down words? I feel confused here about the action.

src/hotspot/share/memory/filemap.cpp line 2240:

> 2238:     FileMapInfo::fail_continue("The shared archive file was created with less restrictive "
> 2239:                                "verification setting than the current setting.");
> 2240:     // Pretend that we didn't have any archived platform/app classes, so they won't be loaded

Same here as above.

-------------

Marked as reviewed by minqi (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2903