[crac] RFR: 8364662: [CRaC] Process restore options in the common arguments parser

[Radim Vansa]

On Mon, 4 Aug 2025 18:12:57 GMT, Timofei Pushkin <tpushkin at openjdk.org> wrote:

> Options for restore are now processed during the common VM arguments parsing stage.
> 
> Notable improvements compared to the old code:
> - Options from `-XX:Flags=<file>` and `-XX:VMOptionsFile=<file>` are now also considered
> - Option aliasing is now handled correctly
> 
> **Behavioral change**. If `CRaCIgnoreRestoreIfUnavailable` is true it is now not an error to specify non-restore-settable options on restore — these will just be omitted from forwarding to the restored JVM. This makes it possible to specify non-restore-settable to be used after a failed restore.

src/hotspot/share/runtime/arguments.cpp line 1046:

> 1044: }
> 1045: 
> 1046: void Arguments::build_jvm_restore_flags(const char* arg) {

I would use `append_jvm_restore_flags` - `build` implies the final operation on a builder.

src/hotspot/share/runtime/arguments.cpp line 2970:

> 2968:     // Restored JVM will search for the flag using the name we record here so we
> 2969:     // must ensure the real one is recorded
> 2970:     if (strncmp(name, flag->name(), name_len) == 0) {

If `name` is a prefix of `flag->name()` this will pass - check `name_len == strlen(flag->name())` first

src/hotspot/share/runtime/arguments.cpp line 2975:

> 2973:       const size_t real_tail_len = strlen(arg) + (strlen(flag->name()) - name_len);
> 2974:       char* const real_tail = AllocateHeap(real_tail_len + 1, MemTag::mtArguments);
> 2975:       const int ret = jio_snprintf(real_tail, real_tail_len + 1, "%s%s", flag->name(), arg + name_len);

Where are `+BoolOpt` flags handled?

src/hotspot/share/runtime/crac.cpp line 638:

> 636:         // should be concatenated. But with the current code the last occurence
> 637:         // will just overwrite the previous ones.
> 638:         assert(!JVMFlag::find_flag(cursor)->ccstr_accumulates(),

Ouch! I think that using multiple `-XX:CRaCEngineOptions` would be pretty common (at least some of my scripts do that). Could we fix this?

And this error should be guarantee/another check that is applied in release build, too because it verifies user input, not some invariants in internal components.

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

PR Review Comment: https://git.openjdk.org/crac/pull/258#discussion_r2259472908
PR Review Comment: https://git.openjdk.org/crac/pull/258#discussion_r2259483064
PR Review Comment: https://git.openjdk.org/crac/pull/258#discussion_r2259480715
PR Review Comment: https://git.openjdk.org/crac/pull/258#discussion_r2259340658