[GR-29743] Make internal symbols local.

PullRequest: graal/8426

Author: Christian Wimmer

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

@@ -379,6 +379,7 @@ protected void onValueUpdate(EconomicMap<OptionKey<?>, Object> values, String ol
                  * functions makes it incoherent with the executable.
                  */
                 RemoveUnusedSymbols.update(values, false);
+                InternalSymbolsAreGlobal.update(values, true);
                 /*
                  * The LLVM backend doesn't support speculative execution attack mitigation
                  */
@@ -399,6 +400,10 @@ public static boolean useLLVMBackend() {
     public static final HostedOptionKey<Boolean> RemoveUnusedSymbols = new HostedOptionKey<>(false);
     @Option(help = "Use linker option to remove all local symbols from image.")//
     public static final HostedOptionKey<Boolean> DeleteLocalSymbols = new HostedOptionKey<>(true);
+    @Option(help = "Compatibility option to make symbols used for the image heap global. " +
+                    "Using global symbols is problematic for shared libraries because the loader implicitly changes the value when the symbol is already defined in the executable loading the library. " +
+                    "Setting this option to true preserves the broken behavior of old Native Image versions.")//
+    public static final HostedOptionKey<Boolean> InternalSymbolsAreGlobal = new HostedOptionKey<>(true);
 
     @Option(help = "Common prefix used by method symbols in image.")//
     public static final HostedOptionKey<String> ImageSymbolsPrefix = new HostedOptionKey<>("");

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

@@ -27,45 +27,22 @@
 import org.graalvm.compiler.serviceprovider.JavaVersionUtil;
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.Platforms;
-import org.graalvm.nativeimage.c.type.CCharPointer;
 
-import com.oracle.svm.core.c.CGlobalData;
-import com.oracle.svm.core.c.CGlobalDataFactory;
-import com.oracle.svm.core.util.Utf8;
 import com.oracle.svm.core.util.VMError;
 
 public final class VM {
-    @Platforms(Platform.HOSTED_ONLY.class) //
-    public static final String valueSeparator = "=";
-    @Platforms(Platform.HOSTED_ONLY.class) //
-    private static final String versionValue = getVersionValue();
+    public static final String VERSION = buildVersion();
 
     @Platforms(Platform.HOSTED_ONLY.class)
-    private static String getVersionValue() {
+    private static String buildVersion() {
         String version = System.getProperty("org.graalvm.version");
         VMError.guarantee(version != null);
-        version = VM.class.getName() + valueSeparator + "GraalVM " + version;
+        version = "GraalVM " + version;
         version += " Java " + JavaVersionUtil.JAVA_SPEC;
         String config = System.getProperty("org.graalvm.config", "");
         if (!config.isEmpty()) {
             version += " " + config;
         }
         return version;
     }
-
-    private static final String VERSION_INFO_SYMBOL_NAME = "__svm_version_info";
-    private static final CGlobalData<CCharPointer> VERSION_INFO = CGlobalDataFactory.createCString(versionValue, VERSION_INFO_SYMBOL_NAME);
-
-    private static final int versionValueHash = versionValue.hashCode();
-
-    public static String getVersion() {
-        CCharPointer versionInfoBytes = VERSION_INFO.get();
-        String version = Utf8.utf8ToString(versionInfoBytes);
-        if (version == null || version.hashCode() != versionValueHash) {
-            VMError.shouldNotReachHere("HashCode mismatch for " + VERSION_INFO_SYMBOL_NAME +
-                            ": actual " + (version == null ? "null" : String.valueOf(version.hashCode())) +
-                            " (expected " + versionValueHash + ")");
-        }
-        return SubstrateUtil.split(version, valueSeparator)[1];
-    }
 }

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/code/CGlobalDataInfo.java

@@ -42,6 +42,8 @@ public final class CGlobalDataInfo {
     private final CGlobalDataImpl<?> data;
     private final boolean isSymbolReference;
 
+    private boolean isGlobalSymbol;
+    private boolean isHiddenSymbol;
     private int offset = -1;
 
     /** Cache until writing the image in case the {@link Supplier} is costly or has side-effects. */
@@ -71,6 +73,23 @@ public int getOffset() {
         return offset;
     }
 
+    public void makeGlobalSymbol() {
+        VMError.guarantee(!isSymbolReference && data.symbolName != null, "Cannot change the local/global status of a symbol reference");
+        isGlobalSymbol = true;
+    }
+
+    public boolean isGlobalSymbol() {
+        return isGlobalSymbol;
+    }
+
+    public void makeHiddenSymbol() {
+        isHiddenSymbol = true;
+    }
+
+    public boolean isHiddenSymbol() {
+        return isHiddenSymbol;
+    }
+
     public boolean isSymbolReference() {
         return isSymbolReference;
     }

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

@@ -102,6 +102,7 @@ protected SystemPropertiesSupport() {
 
         initializeProperty("java.vm.name", "Substrate VM");
         initializeProperty("java.vm.vendor", "Oracle Corporation");
+        initializeProperty("java.vm.version", VM.VERSION);
         initializeProperty("java.vendor", "Oracle Corporation");
         initializeProperty("java.vendor.url", "https://www.graalvm.org/");
 
@@ -126,7 +127,6 @@ protected SystemPropertiesSupport() {
         lazyRuntimeValues.put("user.dir", this::userDir);
         lazyRuntimeValues.put("java.io.tmpdir", this::tmpDir);
         lazyRuntimeValues.put("os.version", this::osVersionValue);
-        lazyRuntimeValues.put("java.vm.version", VM::getVersion);
 
         String targetName = System.getProperty("svm.targetName");
         if (targetName != null) {

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/VMFeature.java

@@ -24,7 +24,6 @@
  */
 package com.oracle.svm.hosted;
 
-import java.lang.reflect.Field;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.stream.Collectors;
@@ -41,7 +40,6 @@
 import com.oracle.svm.core.c.CGlobalData;
 import com.oracle.svm.core.c.CGlobalDataFactory;
 import com.oracle.svm.core.c.libc.LibCBase;
-import com.oracle.svm.core.util.VMError;
 import com.oracle.svm.hosted.c.CGlobalDataFeature;
 import com.oracle.svm.hosted.c.NativeLibraries;
 import com.oracle.svm.hosted.c.codegen.CCompilerInvoker;
@@ -51,6 +49,8 @@ public class VMFeature implements Feature {
 
     private NativeLibraries nativeLibraries;
     private static final String STATIC_BINARY_MARKER_SYMBOL_NAME = "__svm_vm_is_static_binary";
+    private static final String VERSION_INFO_SYMBOL_NAME = "__svm_version_info";
+    private static final String valueSeparator = "=";
 
     @Override
     public void beforeAnalysis(BeforeAnalysisAccess a) {
@@ -64,19 +64,13 @@ public void beforeAnalysis(BeforeAnalysisAccess a) {
         }
 
         FeatureImpl.BeforeAnalysisAccessImpl access = (FeatureImpl.BeforeAnalysisAccessImpl) a;
-        String fieldName = "VERSION_INFO";
-        try {
-            Field declaredField = VM.class.getDeclaredField(fieldName);
-            access.registerAsRead(declaredField);
-        } catch (NoSuchFieldException e) {
-            VMError.shouldNotReachHere(VM.class.getName() + " should have field " + fieldName);
-        }
-
         nativeLibraries = access.getNativeLibraries();
     }
 
     @Override
     public void afterAnalysis(AfterAnalysisAccess access) {
+        CGlobalDataFeature.singleton().registerWithGlobalSymbol(CGlobalDataFactory.createCString(VM.class.getName() + valueSeparator + VM.VERSION, VERSION_INFO_SYMBOL_NAME));
+
         addCGlobalDataString("Target.Platform", ImageSingletons.lookup(Platform.class).getClass().getName());
         addCGlobalDataString("Target.LibC", ImageSingletons.lookup(LibCBase.class).getClass().getName());
 
@@ -94,12 +88,12 @@ public void afterAnalysis(AfterAnalysisAccess access) {
         }
 
         CGlobalData<PointerBase> isStaticBinaryMarker = CGlobalDataFactory.createWord(WordFactory.unsigned(SubstrateOptions.StaticExecutable.getValue() ? 1 : 0), STATIC_BINARY_MARKER_SYMBOL_NAME);
-        CGlobalDataFeature.singleton().registerAsAccessedOrGet(isStaticBinaryMarker);
+        CGlobalDataFeature.singleton().registerWithGlobalHiddenSymbol(isStaticBinaryMarker);
     }
 
     private static void addCGlobalDataString(String infoType, String content) {
-        String data = VM.class.getName() + "." + infoType + VM.valueSeparator + content;
+        String data = VM.class.getName() + "." + infoType + valueSeparator + content;
         String symbolName = "__svm_vm_" + infoType.toLowerCase().replace(".", "_");
-        CGlobalDataFeature.singleton().registerAsAccessedOrGet(CGlobalDataFactory.createCString(data, symbolName));
+        CGlobalDataFeature.singleton().registerWithGlobalSymbol(CGlobalDataFactory.createCString(data, symbolName));
     }
 }

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/c/CGlobalDataFeature.java

@@ -32,8 +32,9 @@
 import java.nio.ByteBuffer;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.function.BiFunction;
+import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
 import org.graalvm.compiler.api.replacements.SnippetReflectionProvider;
@@ -204,6 +205,36 @@ public CGlobalDataInfo registerAsAccessedOrGet(CGlobalData<?> obj) {
                         });
     }
 
+    /**
+     * Makes the provided object available in the binary as a global symbol
+     *
+     * Warning: Global symbols are affected by linking and loading rules that are OS dependent. So
+     * accessing a global symbol content at run time using the symbol name could return a different
+     * content than the one provided at build time. This happens for example on Linux when a shared
+     * library is loaded and the main executable already defines a symbol with the same name.
+     */
+    public void registerWithGlobalSymbol(CGlobalData<?> obj) {
+        registerWithGlobalSymbolImpl(obj);
+    }
+
+    /**
+     * Same as {@link #registerWithGlobalSymbol(CGlobalData)} but hides the provided object so that
+     * it will not show up in the dynamic symbol table of the final image.
+     */
+    public void registerWithGlobalHiddenSymbol(CGlobalData<?> obj) {
+        registerWithGlobalSymbolImpl(obj).makeHiddenSymbol();
+    }
+
+    private CGlobalDataInfo registerWithGlobalSymbolImpl(CGlobalData<?> obj) {
+        CGlobalDataInfo info = registerAsAccessedOrGet(obj);
+        info.makeGlobalSymbol();
+        return info;
+    }
+
+    public Set<String> getGlobalHiddenSymbols() {
+        return map.entrySet().stream().filter(entry -> entry.getValue().isGlobalSymbol() && entry.getValue().isHiddenSymbol()).map(entry -> entry.getKey().symbolName).collect(Collectors.toSet());
+    }
+
     private Object replaceObject(Object obj) {
         if (obj instanceof CGlobalDataImpl<?>) {
             registerAsAccessedOrGet((CGlobalData<?>) obj);
@@ -250,7 +281,11 @@ public int getSize() {
         return totalSize;
     }
 
-    public void writeData(RelocatableBuffer buffer, BiFunction<Integer, String, ?> createSymbol, BiFunction<Integer, String, ?> createSymbolReference) {
+    public interface SymbolConsumer {
+        void apply(int offset, String symbolName, boolean isGlobalSymbol);
+    }
+
+    public void writeData(RelocatableBuffer buffer, SymbolConsumer createSymbol, SymbolConsumer createSymbolReference) {
         assert isLayouted() : "Not layouted yet";
         ByteBuffer bufferBytes = buffer.getByteBuffer();
         int start = bufferBytes.position();
@@ -263,10 +298,10 @@ public void writeData(RelocatableBuffer buffer, BiFunction<Integer, String, ?> c
             }
             CGlobalDataImpl<?> data = info.getData();
             if (data.symbolName != null && !info.isSymbolReference()) {
-                createSymbol.apply(info.getOffset(), data.symbolName);
+                createSymbol.apply(info.getOffset(), data.symbolName, info.isGlobalSymbol());
             }
             if (data.nonConstant) {
-                createSymbolReference.apply(info.getOffset(), data.symbolName);
+                createSymbolReference.apply(info.getOffset(), data.symbolName, info.isGlobalSymbol());
             }
         }
     }

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/LIRNativeImageCodeCache.java

@@ -29,6 +29,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
 import java.util.concurrent.ForkJoinPool;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -48,6 +49,7 @@
 import com.oracle.svm.core.config.ConfigurationValues;
 import com.oracle.svm.core.meta.SubstrateObjectConstant;
 import com.oracle.svm.core.util.VMError;
+import com.oracle.svm.hosted.c.CGlobalDataFeature;
 import com.oracle.svm.hosted.code.HostedImageHeapConstantPatch;
 import com.oracle.svm.hosted.code.HostedPatcher;
 import com.oracle.svm.hosted.image.NativeImage.NativeTextSectionImpl;
@@ -244,7 +246,8 @@ protected void defineMethodSymbol(String name, boolean global, ObjectFile.Elemen
     public List<ObjectFile.Symbol> getSymbols(ObjectFile objectFile, boolean onlyGlobal) {
         Stream<ObjectFile.Symbol> stream = StreamSupport.stream(objectFile.getSymbolTable().spliterator(), false);
         if (onlyGlobal) {
-            stream = stream.filter(ObjectFile.Symbol::isGlobal);
+            Set<String> globalHiddenSymbols = CGlobalDataFeature.singleton().getGlobalHiddenSymbols();
+            stream = stream.filter(symbol -> symbol.isGlobal() && !globalHiddenSymbols.contains(symbol.getName()));
         }
         return stream.filter(ObjectFile.Symbol::isDefined).collect(Collectors.toList());
     }

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImage.java

@@ -380,7 +380,7 @@ private boolean shouldWriteHeader(HostedMethod method) {
     }
 
     private ObjectFile.Symbol defineDataSymbol(String name, Element section, long position) {
-        return objectFile.createDefinedSymbol(name, section, position, wordSize, false, true);
+        return objectFile.createDefinedSymbol(name, section, position, wordSize, false, SubstrateOptions.InternalSymbolsAreGlobal.getValue());
     }
 
     private ObjectFile.Symbol defineRelocationForSymbol(String name, long position) {
@@ -429,7 +429,7 @@ public void build(String imageName, DebugContext debug) {
 
             // Define symbols for the sections.
             objectFile.createDefinedSymbol(textSection.getName(), textSection, 0, 0, false, false);
-            objectFile.createDefinedSymbol("__svm_text_end", textSection, textSectionSize, 0, false, true);
+            objectFile.createDefinedSymbol("__svm_text_end", textSection, textSectionSize, 0, false, SubstrateOptions.InternalSymbolsAreGlobal.getValue());
             objectFile.createDefinedSymbol(roDataSection.getName(), roDataSection, 0, 0, false, false);
             objectFile.createDefinedSymbol(rwDataSection.getName(), rwDataSection, 0, 0, false, false);
 
@@ -441,8 +441,9 @@ public void build(String imageName, DebugContext debug) {
             codeCache.writeConstants(writer, roDataBuffer);
             // - Non-heap global data goes at the beginning of the read-write data section.
             cGlobals.writeData(rwDataBuffer,
-                            (offset, symbolName) -> defineDataSymbol(symbolName, rwDataSection, offset + RWDATA_CGLOBALS_PARTITION_OFFSET),
-                            (offset, symbolName) -> defineRelocationForSymbol(symbolName, offset));
+                            (offset, symbolName, isGlobalSymbol) -> objectFile.createDefinedSymbol(symbolName, rwDataSection, offset + RWDATA_CGLOBALS_PARTITION_OFFSET, wordSize, false,
+                                            isGlobalSymbol || SubstrateOptions.InternalSymbolsAreGlobal.getValue()),
+                            (offset, symbolName, isGlobalSymbol) -> defineRelocationForSymbol(symbolName, offset));
             defineDataSymbol(CGlobalDataInfo.CGLOBALDATA_BASE_SYMBOL_NAME, rwDataSection, RWDATA_CGLOBALS_PARTITION_OFFSET);
 
             /*