Fix review feedback

Author: lazar-mitrovic

sdk/src/org.graalvm.nativeimage/src/org/graalvm/nativeimage/impl/ProcessPropertiesSupport.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * The Universal Permissive License (UPL), Version 1.0
@@ -40,18 +40,13 @@
  */
 package org.graalvm.nativeimage.impl;
 
-import java.net.URL;
 import java.nio.file.Path;
 
 import org.graalvm.nativeimage.c.function.CEntryPointLiteral;
 
 public interface ProcessPropertiesSupport {
     String getExecutableName();
 
-    default URL getExecutableURL() {
-        return null;
-    }
-
     long getProcessID();
 
     long getProcessID(Process process);

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/BaseProcessPropertiesSupport.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -27,23 +27,7 @@
 
 import org.graalvm.nativeimage.impl.ProcessPropertiesSupport;
 
-import java.io.File;
-import java.net.MalformedURLException;
-import java.net.URL;
-
 public abstract class BaseProcessPropertiesSupport implements ProcessPropertiesSupport {
-
-    @Override
-    public URL getExecutableURL() {
-        try {
-            return new File(getExecutableName()).toURI().toURL();
-        } catch (MalformedURLException ex) {
-            // This should not really happen; the file is cannonicalized, absolute, so it should
-            // always have file:// URL.
-            return null;
-        }
-    }
-
     @Override
     public int getArgumentVectorBlockSize() {
         return JavaMainWrapper.getCRuntimeArgumentBlockLength();

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/DynamicHub.java

@@ -26,6 +26,7 @@
 
 //Checkstyle: allow reflection
 
+import java.io.File;
 import java.io.InputStream;
 import java.io.Serializable;
 import java.lang.annotation.Annotation;
@@ -42,6 +43,7 @@
 import java.lang.reflect.Modifier;
 import java.lang.reflect.Type;
 import java.lang.reflect.TypeVariable;
+import java.net.MalformedURLException;
 import java.net.URL;
 import java.security.CodeSource;
 import java.security.ProtectionDomain;
@@ -53,12 +55,14 @@
 import java.util.Set;
 import java.util.StringJoiner;
 
+import com.oracle.svm.core.BaseProcessPropertiesSupport;
 import org.graalvm.compiler.core.common.NumUtil;
 import org.graalvm.compiler.core.common.SuppressFBWarnings;
 import org.graalvm.compiler.serviceprovider.JavaVersionUtil;
 import org.graalvm.nativeimage.ImageSingletons;
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.Platforms;
+import org.graalvm.nativeimage.ProcessProperties;
 import org.graalvm.nativeimage.c.function.CFunctionPointer;
 import org.graalvm.nativeimage.impl.ProcessPropertiesSupport;
 import org.graalvm.util.DirectAnnotationAccess;
@@ -327,19 +331,26 @@ public void setModule(Object module) {
     }
 
     /**
-     * Final fields in subsituted classes are treated as implicitly RecomputeFieldValue even when
+     * Final fields in substituted classes are treated as implicitly RecomputeFieldValue even when
      * not annotated with @RecomputeFieldValue. Their name must not match a field in the original
      * class, i.e., allPermDomain.
      */
     static final LazyFinalReference<java.security.ProtectionDomain> allPermDomainReference = new LazyFinalReference<>(() -> {
         java.security.Permissions perms = new java.security.Permissions();
         perms.add(SecurityConstants.ALL_PERMISSION);
-        CodeSource cs;
-
-        // Try to use executable image's name as code source for the class.
-        // The file location can be used by Java code to determine its location on disk, similar
-        // to argv[0].
-        cs = new CodeSource(ImageSingletons.lookup(ProcessPropertiesSupport.class).getExecutableURL(), (Certificate[]) null);
+        CodeSource cs = null;
+
+        if (ImageSingletons.lookup(ProcessPropertiesSupport.class) instanceof BaseProcessPropertiesSupport) {
+            // Try to use executable image's name as code source for the class.
+            // The file location can be used by Java code to determine its location on disk, similar
+            // to argv[0].
+            try {
+                cs = new CodeSource(new File(ProcessProperties.getExecutableName()).toURI().toURL(), (Certificate[]) null);
+            } catch (MalformedURLException e) {
+                // This should not really happen; the file is cannonicalized, absolute, so it should
+                // always have file:// URL.
+            }
+        }
 
         return new java.security.ProtectionDomain(cs, perms);
     });

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

@@ -40,7 +40,7 @@
 import java.security.Provider;
 import java.security.SecureRandom;
 import java.util.ArrayDeque;
-import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -49,7 +49,6 @@
 
 import jdk.vm.ci.meta.MetaAccessProvider;
 import jdk.vm.ci.meta.ResolvedJavaField;
-import org.graalvm.collections.Pair;
 import org.graalvm.compiler.serviceprovider.JavaVersionUtil;
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.Platforms;
@@ -135,16 +134,11 @@ private static ProtectionDomain getProtectionDomain(final Class<?> caller) {
 
     @Substitute
     @TargetElement(onlyWith = JDK14OrLater.class)
-    @SuppressWarnings("deprecation") // deprecated starting JDK 17
     static <T> T executePrivileged(PrivilegedExceptionAction<T> action, AccessControlContext context, Class<?> caller) throws Throwable {
         if (action == null) {
             throw new NullPointerException("Null action");
         }
 
-        if (context != null && context.equals(AccessControllerUtil.NO_CONTEXT_SINGLETON)) {
-            VMError.shouldNotReachHere("Invoked AccessControlContext was replaced at build time but wasn't reinitialized at run time.");
-        }
-
         AccessControllerUtil.PrivilegedStack.push(context, caller);
         try {
             return action.run();
@@ -157,17 +151,11 @@ static <T> T executePrivileged(PrivilegedExceptionAction<T> action, AccessContro
 
     @Substitute
     @TargetElement(onlyWith = JDK14OrLater.class)
-    @SuppressWarnings("deprecation") // deprecated starting JDK 17
     static <T> T executePrivileged(PrivilegedAction<T> action, AccessControlContext context, Class<?> caller) throws Throwable {
         if (action == null) {
             throw new NullPointerException("Null action");
         }
 
-        if (context != null && context.equals(AccessControllerUtil.NO_CONTEXT_SINGLETON)) {
-            VMError.shouldNotReachHere("Invoked AccessControlContext was replaced at build time but wasn't reinitialized at run time.\n" +
-                            "This might be an indicator of improper build time initialization, or of a non-compatible JDK version.");
-        }
-
         AccessControllerUtil.PrivilegedStack.push(context, caller);
         try {
             return action.run();
@@ -180,11 +168,21 @@ static <T> T executePrivileged(PrivilegedAction<T> action, AccessControlContext
 
     @Substitute
     @TargetElement(onlyWith = JDK14OrLater.class)
-    @SuppressWarnings("unused")
+    @SuppressWarnings({"unused", "deprecation"})
     static AccessControlContext checkContext(AccessControlContext context, Class<?> caller) {
+
+        if (context != null && context.equals(AccessControllerUtil.NO_CONTEXT_SINGLETON)) {
+            VMError.shouldNotReachHere("Non-allowed AccessControlContext that was replaced with a blank one at build time was invoked without being reinitialized at run time.\n" +
+                            "This might be an indicator of improper build time initialization, or of a non-compatible JDK version.\n" +
+                            "In order to fix this you can either:\n" +
+                            "    * Annotate the offending context's field with @RecomputeFieldValue\n" +
+                            "    * Implement a custom runtime accessor and annotate said field with @InjectAccessors\n" +
+                            "    * If this context originates from the JDK, and it doesn't leak sensitive info, you can allow it in 'AccessControlContextFeature.duringSetup'");
+        }
+
         // check if caller is authorized to create context
         if (System.getSecurityManager() != null) {
-            throw VMError.shouldNotReachHere("Needs to be implemented when SecurityManager is supported");
+            throw VMError.unsupportedFeature("SecurityManager isn't supported");
         }
         return context;
     }
@@ -224,7 +222,7 @@ public Class<?> getCaller() {
             }
         }
 
-        @SuppressWarnings("rawtypes") private static final FastThreadLocalObject<ArrayDeque> stack = FastThreadLocalFactory.createObject(ArrayDeque.class, "accStack");
+        @SuppressWarnings("rawtypes") private static final FastThreadLocalObject<ArrayDeque> stack = FastThreadLocalFactory.createObject(ArrayDeque.class, "AccessControlContextStack");
 
         @SuppressWarnings("unchecked")
         private static ArrayDeque<StackElement> getStack() {
@@ -270,7 +268,7 @@ static Throwable wrapCheckedException(Throwable ex) {
 @SuppressWarnings({"unused"})
 class AccessControlContextFeature implements Feature {
 
-    static List<Pair<String, AccessControlContext>> allowedContexts = new ArrayList<>();
+    static Map<String, AccessControlContext> allowedContexts = new HashMap<>();
 
     static void allowContextIfExists(String className, String fieldName) {
         try {
@@ -280,7 +278,7 @@ static void allowContextIfExists(String className, String fieldName) {
             String description = className + "." + fieldName;
             try {
                 AccessControlContext acc = ReflectionUtil.readStaticField(clazz, fieldName);
-                allowedContexts.add(Pair.create(description, acc));
+                allowedContexts.put(description, acc);
             } catch (ReflectionUtil.ReflectionUtilError e) {
                 VMError.shouldNotReachHere("Following field isn't present in JDK" + JavaVersionUtil.JAVA_SPEC + ": " + description);
             }
@@ -323,7 +321,7 @@ public void duringSetup(DuringSetupAccess access) {
 
     private static Object replaceAccessControlContext(Object obj) {
         if (obj instanceof AccessControlContext && obj != AccessControllerUtil.NO_CONTEXT_SINGLETON) {
-            if (allowedContexts.stream().anyMatch((e) -> obj.equals(e.getRight()))) {
+            if (allowedContexts.containsValue(obj)) {
                 return obj;
             } else {
                 return AccessControllerUtil.NO_CONTEXT_SINGLETON;