[GR-64566] Parse system properties before options.

PullRequest: graal/20662

Author: christianhaeubl

substratevm/src/com.oracle.svm.common/src/com/oracle/svm/common/option/CommonOptionParser.java

@@ -357,15 +357,15 @@ private static Class<?> getMultiOptionValueElementType(OptionKey<?> optionKey) {
     }
 
     public static long parseLong(String v) {
-        String valueString = v.trim().toLowerCase();
+        String valueString = v.trim();
         long scale = 1;
-        if (valueString.endsWith("k")) {
+        if (valueString.endsWith("k") || valueString.endsWith("K")) {
             scale = 1024L;
-        } else if (valueString.endsWith("m")) {
+        } else if (valueString.endsWith("m") || valueString.endsWith("M")) {
             scale = 1024L * 1024L;
-        } else if (valueString.endsWith("g")) {
+        } else if (valueString.endsWith("g") || valueString.endsWith("G")) {
             scale = 1024L * 1024L * 1024L;
-        } else if (valueString.endsWith("t")) {
+        } else if (valueString.endsWith("t") || valueString.endsWith("T")) {
             scale = 1024L * 1024L * 1024L * 1024L;
         }
 

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_jdk_internal_util_StaticProperty.java

@@ -24,10 +24,11 @@
  */
 package com.oracle.svm.core.jdk;
 
+import java.util.Objects;
+
 import com.oracle.svm.core.SubstrateUtil;
 import com.oracle.svm.core.annotate.Alias;
 import com.oracle.svm.core.annotate.Delete;
-import com.oracle.svm.core.annotate.KeepOriginal;
 import com.oracle.svm.core.annotate.RecomputeFieldValue;
 import com.oracle.svm.core.annotate.Substitute;
 import com.oracle.svm.core.annotate.TargetClass;
@@ -42,14 +43,14 @@
  * corresponding system properties.
  * <p>
  * We {@link Substitute substitute} the whole class so that it is possible to use a custom static
- * constructor at run-time.
+ * constructor at run-time. If this class is used before the system properties are fully parsed and
+ * initialized, it can happen that we return or cache invalid values (see GR-64572).
  * <p>
  * Note for updating: use {@link Delete} for static fields that should be unreachable (e.g, because
  * we substituted an accessor and the field is therefore unused). Use {@link Alias} for static
  * fields that can be initialized in our custom static constructor. Use {@link Substitute} for
  * methods that access expensive lazily initialized system properties (see
- * {@link SystemPropertiesSupport} for a list of all lazily initialized properties). Use
- * {@link KeepOriginal} for methods that we don't want to substitute.
+ * {@link SystemPropertiesSupport} for a list of all lazily initialized properties).
  */
 @Substitute
 @TargetClass(jdk.internal.util.StaticProperty.class)
@@ -284,39 +285,66 @@ private static String javaIoTmpDir() {
         return SystemPropertiesSupport.singleton().getInitialProperty("java.io.tmpdir");
     }
 
-    @KeepOriginal
-    public static native String sunBootLibraryPath();
+    @Substitute
+    public static String sunBootLibraryPath() {
+        assert Objects.equals(SUN_BOOT_LIBRARY_PATH, SystemPropertiesSupport.singleton().getInitialProperty("sun.boot.library.path", ""));
+        return SUN_BOOT_LIBRARY_PATH;
+    }
 
-    @KeepOriginal
-    public static native String jdkSerialFilter();
+    @Substitute
+    public static String jdkSerialFilter() {
+        assert Objects.equals(JDK_SERIAL_FILTER, SystemPropertiesSupport.singleton().getInitialProperty("jdk.serialFilter"));
+        return JDK_SERIAL_FILTER;
+    }
 
-    @KeepOriginal
-    public static native String jdkSerialFilterFactory();
+    @Substitute
+    public static String jdkSerialFilterFactory() {
+        assert Objects.equals(JDK_SERIAL_FILTER_FACTORY, SystemPropertiesSupport.singleton().getInitialProperty("jdk.serialFilterFactory"));
+        return JDK_SERIAL_FILTER_FACTORY;
+    }
 
-    @KeepOriginal
-    public static native String nativeEncoding();
+    @Substitute
+    public static String nativeEncoding() {
+        assert Objects.equals(NATIVE_ENCODING, SystemPropertiesSupport.singleton().getInitialProperty("native.encoding"));
+        return NATIVE_ENCODING;
+    }
 
-    @KeepOriginal
-    public static native String fileEncoding();
+    @Substitute
+    public static String fileEncoding() {
+        assert Objects.equals(FILE_ENCODING, SystemPropertiesSupport.singleton().getInitialProperty("file.encoding"));
+        return FILE_ENCODING;
+    }
 
-    @KeepOriginal
-    public static native String javaPropertiesDate();
+    @Substitute
+    public static String javaPropertiesDate() {
+        assert Objects.equals(JAVA_PROPERTIES_DATE, SystemPropertiesSupport.singleton().getInitialProperty("java.properties.date"));
+        return JAVA_PROPERTIES_DATE;
+    }
 
-    @KeepOriginal
-    public static native String jnuEncoding();
+    @Substitute
+    public static String jnuEncoding() {
+        assert Objects.equals(SUN_JNU_ENCODING, SystemPropertiesSupport.singleton().getInitialProperty("sun.jnu.encoding"));
+        return SUN_JNU_ENCODING;
+    }
 
-    @KeepOriginal
-    public static native String javaLocaleUseOldISOCodes();
+    @Substitute
+    public static String javaLocaleUseOldISOCodes() {
+        assert Objects.equals(JAVA_LOCALE_USE_OLD_ISO_CODES, SystemPropertiesSupport.singleton().getInitialProperty("java.locale.useOldISOCodes", ""));
+        return JAVA_LOCALE_USE_OLD_ISO_CODES;
+    }
 
     @Substitute
     @TargetElement(onlyWith = JDKLatest.class)//
     public static String osName() {
         return SystemPropertiesSupport.singleton().getInitialProperty("os.name");
     }
 
-    @KeepOriginal
+    @Substitute
     @TargetElement(onlyWith = JDKLatest.class)//
-    public static native String osArch();
+    public static String osArch() {
+        assert Objects.equals(OS_ARCH, SystemPropertiesSupport.singleton().getInitialProperty("os.arch"));
+        return OS_ARCH;
+    }
 
     @Substitute
     @TargetElement(onlyWith = JDKLatest.class)//

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/option/RuntimeOptionParser.java

@@ -40,7 +40,7 @@
 import com.oracle.svm.core.SubstrateOptions;
 import com.oracle.svm.core.graal.RuntimeCompilation;
 import com.oracle.svm.core.log.Log;
-import com.oracle.svm.core.properties.RuntimePropertyParser;
+import com.oracle.svm.core.properties.RuntimeSystemPropertyParser;
 import com.oracle.svm.core.util.ImageHeapMap;
 
 import jdk.graal.compiler.api.replacements.Fold;
@@ -92,8 +92,9 @@ public final class RuntimeOptionParser {
     public static String[] parseAndConsumeAllOptions(String[] initialArgs, boolean ignoreUnrecognized) {
         String[] args = initialArgs;
         if (SubstrateOptions.ParseRuntimeOptions.getValue()) {
+            /* JDK code may access and cache system properties, so parse them early. */
+            args = RuntimeSystemPropertyParser.parse(args, GRAAL_OPTION_PREFIX, LEGACY_GRAAL_OPTION_PREFIX);
             args = RuntimeOptionParser.singleton().parse(args, NORMAL_OPTION_PREFIX, GRAAL_OPTION_PREFIX, LEGACY_GRAAL_OPTION_PREFIX, X_OPTION_PREFIX, ignoreUnrecognized);
-            args = RuntimePropertyParser.parse(args);
         } else if (RuntimeCompilation.isEnabled() && SubstrateOptions.supportCompileInIsolates() && IsolateArgumentParser.isCompilationIsolate()) {
             /*
              * Compilation isolates always need to parse the Native Image options that the main

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/properties/RuntimePropertyParser.java renamed to substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/properties/RuntimeSystemPropertyParser.java

@@ -31,7 +31,7 @@
 
 import com.oracle.svm.core.jdk.SystemPropertiesSupport;
 
-public final class RuntimePropertyParser {
+public final class RuntimeSystemPropertyParser {
 
     private static final String PROPERTY_PREFIX = "-D";
 
@@ -41,14 +41,12 @@ public final class RuntimePropertyParser {
      * property is set. The returned array of arguments are those that should be passed through to
      * the application.
      */
-    public static String[] parse(String[] args) {
+    public static String[] parse(String[] args, String graalOptionPrefix, String legacyGraalOptionPrefix) {
         int newIdx = 0;
         EconomicMap<String, String> properties = EconomicMap.create();
         for (int oldIdx = 0; oldIdx < args.length; oldIdx++) {
             String arg = args[oldIdx];
-            if (arg.startsWith(PROPERTY_PREFIX) && parseProperty(arg.substring(PROPERTY_PREFIX.length()), properties)) {
-                // Option consumed
-            } else {
+            if (!parseProperty(arg, properties, graalOptionPrefix, legacyGraalOptionPrefix)) {
                 assert newIdx <= oldIdx;
                 args[newIdx] = arg;
                 newIdx++;
@@ -66,17 +64,23 @@ public static String[] parse(String[] args) {
         }
     }
 
-    private static boolean parseProperty(String property, EconomicMap<String, String> parsedProperties) {
+    private static boolean parseProperty(String arg, EconomicMap<String, String> properties, String graalOptionPrefix, String legacyGraalOptionPrefix) {
+        if (!arg.startsWith(PROPERTY_PREFIX) || arg.startsWith(graalOptionPrefix) || arg.startsWith(legacyGraalOptionPrefix)) {
+            return false;
+        }
+        return parseProperty0(arg, properties);
+    }
+
+    private static boolean parseProperty0(String arg, EconomicMap<String, String> parsedProperties) {
+        String property = arg.substring(PROPERTY_PREFIX.length());
         int splitIndex = property.indexOf('=');
         if (splitIndex == -1) {
             return false;
         }
 
         String key = property.substring(0, splitIndex);
         String value = property.substring(splitIndex + 1);
-
         parsedProperties.put(key, value);
-
         return true;
     }
 }