CODETOOLS-7902290 breaks all JTreg tests which use @requires vm.opt.*

[Volker Simonis]

On Thu, Oct 11, 2018 at 11:09 PM Igor Ignatyev <igor.ignatyev at oracle.com> wrote:
>
> Hi Volker,
>
> vm.opt.* options are set by jtreg (RegressionContext::processVMOptions), and their value is expected to be "null" if a flag hasn't been passed to JDK under tests via -javaoptions or -vmoptions. I don't think there is something to fix (at least not in jtreg).
>

Well, if "vm.opt.*" options are indeed only options which are taken
from the command line (i.e. "-javaoption/-vmoption"), than there's
definitely something we have to fix! Because, as I described before,
after CODETOOLS-7902290, all test which check such an option in a
@requires clause (negatively or positively) will now fail if that
option is not given explicitly on the command line.

If "vm.opt.*" options are really only the options given on the command
line (please confirm this and please document it somewhere
prominently!) I think many tests which use them are broken because
they simply check such options like this:

@requires vm.opt.TieredCompilation == true

This would be wrong and would have to be replaced by:

@requires vm.opt.TieredCompilation == null | vm.opt.TieredCompilation == true

to also account for the case where -XX:TieredCompilation was not
explicitly set on the command line. Notice that in in such a case, the
value of "TieredCompilation" in the VM could be either "true" or
"false" (depending on the platform and VM build configuration), but it
wouldn't be possible to test that by using "vm.opt.TieredCompilation".

That said, there are three "vm.opt.final.*" options which are set by VMProps:

        vmOptFinalFlag(map, "ClassUnloading");
        vmOptFinalFlag(map, "UseCompressedOops");
        vmOptFinalFlag(map, "EnableJVMCI");

Notice that these options are set to the values of the corresponding
flags in the agent VM . For tests which run in "othervm" mode (or
which start new VMs with ProcessBuilder or
jdk.test.lib.process.ProcessTools), that doesn't necessarily
corresponds to the values of these flags in the testee VM.

Finally, options specified with "-javaoption/-vmoption" and reflected
in the corresponding "vm.opt.*" flags, may be over-ridden by command
line options specified for tests which run in "othervm" mode (so
checking them with a @requires clause is of limited usefulness as
well).

All this is quite complex and I think we should *really* document it
in a prominent place like the "jtreg: Command Line Options" page [1]
AND the
"The JDK Test Framework: Tag Language Specification" page [2].

Regards,
Volker

[1] http://openjdk.java.net/jtreg/command-help.html
[2] http://openjdk.java.net/jtreg/tag-spec.html

> -- Igor
>
>
> > On Oct 11, 2018, at 1:59 PM, Volker Simonis <volker.simonis at gmail.com> wrote:
> >
> > Jonathan Gibbons <jonathan.gibbons at oracle.com> schrieb am Do. 11. Okt. 2018
> > um 19:16:
> >
> >> Volker,
> >>
> >> It's a typo/oops on my part.  I'll fix it.
> >>
> >
> > Hi John,
> >
> > thanks for looking at this issue!
> >
> > What do you mean  with “ typo”? Returning null instead of the string “null”?
> >
> > That would “fix” the corresponding tests in the sense that they would run
> > again, however still with bogus results for all the @requires guards which
> > check the “vm.opt.*” options. Shouldn’t we fix that as well?
> >
> > Regards,
> > Volker
> >
> >
> >
> >> -- Jon
> >>
> >>
> >> On 10/11/18 10:13 AM, Jonathan Gibbons wrote:
> >>> Volker,
> >>>
> >>> Thanks for the report.  I'll take a look shortly.
> >>>
> >>> -- Jon
> >>>
> >>>
> >>> On 10/11/18 10:01 AM, Volker Simonis wrote:
> >>>> Hi,
> >>>>
> >>>> the change "CODETOOLS-7902290: introduce error state of @requires
> >>>> properties" [1] which was recently pushed to the codetools repo,
> >>>> breaks all JTreg tests which have a require clause that queries a non
> >>>> final (i.e. no "vm.opt.final") VM option.
> >>>>
> >>>> For example the test
> >>>> "hotspot/jtreg/runtime/ReservedStack/ReservedStackTest.java" which
> >>>> uses "@requires vm.opt.DeoptimizeALot != true" will now fail with:
> >>>>
> >>>> test result: Error. Error evaluating expression: name not defined:
> >>>> vm.opt.DeoptimizeALot
> >>>>
> >>>> This is because
> >>>> com.sun.javatest.regtest.config.RegressionContext.get() was changed to
> >>>> return null instead of the string "null" for unknown properties.
> >>>> Looking at requires.VMProps, I realized that "vm.opt.DeoptimizeALot"
> >>>> is indeed not defined (as many other "vm.opt" properties, see below).
> >>>> So it turns out that all the tests which @required a "vm.opt" option
> >>>> did run in a faulty configuration until now.
> >>>>
> >>>> I could for example easily fix
> >>>> "hotspot/jtreg/runtime/ReservedStack/ReservedStackTest.java" by adding
> >>>> the corresponding option in VMProps.java:
> >>>>
> >>>> $ hg diff
> >>>> diff -r 7a1e2d7ac55a test/jtreg-ext/requires/VMProps.java
> >>>> --- a/test/jtreg-ext/requires/VMProps.java      Thu Oct 11 10:11:18
> >>>> 2018 -0400
> >>>> +++ b/test/jtreg-ext/requires/VMProps.java      Thu Oct 11 18:51:37
> >>>> 2018 +0200
> >>>> @@ -266,6 +266,8 @@
> >>>>          vmOptFinalFlag(map, "ClassUnloading");
> >>>>          vmOptFinalFlag(map, "UseCompressedOops");
> >>>>          vmOptFinalFlag(map, "EnableJVMCI");
> >>>> +        String value =
> >>>> String.valueOf(WB.getBooleanVMFlag("DeoptimizeALot"));
> >>>> +        map.put("vm.opt.DeoptimizeALot", value);
> >>>>      }
> >>>>
> >>>> Following is a list of all "vm.opt" options used in @requires clauses
> >>>> without being defined in VMProps.java:
> >>>>
> >>>> vm.opt.AggressiveOpts
> >>>> vm.opt.ClassUnloading
> >>>> vm.opt.ClassUnloadingWithConcurrentMark
> >>>> vm.opt.DeoptimizeALot
> >>>> vm.opt.DisableExplicitGC
> >>>> vm.opt.ExplicitGCInvokesConcurrent
> >>>> vm.opt.FlightRecorder
> >>>> vm.opt.G1HeapRegionSize
> >>>> vm.opt.Inline
> >>>> vm.opt.MaxGCPauseMillis
> >>>> vm.opt.StartFlightRecording
> >>>> vm.opt.SurvivorAlignmentInBytes
> >>>> vm.opt.TieredCompilation
> >>>> vm.opt.TieredStopAtLevel
> >>>> vm.opt.TieredStopAtLevel==4)
> >>>> vm.opt.TieredStopAtLevel==null
> >>>> vm.opt.UseJVMCICompiler
> >>>>
> >>>> What's the current plan? Do we want to add all these options in
> >>>> VMProps.java? Or should we better add all the -XX VM options together
> >>>> to VMProps such that they can all be freely used within @requires
> >>>> clauses?
> >>>>
> >>>> Thank you and best regards,
> >>>> Volker
> >>>>
> >>>> PS: I've noticed that there's "8209807: improve handling exception in
> >>>> requires.VMProps" [2] to apparently adapt VMProps.java to
> >>>> CODETOOLS-7902290 but I've tried the attached webrev and it doesn't
> >>>> help with the described problem.
> >>>>
> >>>> [1] https://bugs.openjdk.java.net/browse/CODETOOLS-7902290
> >>>> [2] https://bugs.openjdk.java.net/browse/JDK-8209807
> >>>
> >>
> >>
>