[GR-59071] Make CGlobals layer-aware.

PullRequest: graal/21612

Author: teshull

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/ObjectFile.java

@@ -1805,7 +1805,7 @@ public interface Symbol {
 
     public abstract Symbol createDefinedSymbol(String name, Element baseSection, long position, int size, boolean isCode, boolean isGlobal);
 
-    public abstract Symbol createUndefinedSymbol(String name, int size, boolean isCode);
+    public abstract Symbol createUndefinedSymbol(String name, boolean isCode);
 
     protected abstract SymbolTable createSymbolTable();
 

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/ELFObjectFile.java

@@ -190,7 +190,7 @@ public Symbol createDefinedSymbol(String name, Element baseSection, long positio
     }
 
     @Override
-    public Symbol createUndefinedSymbol(String name, int size, boolean isCode) {
+    public Symbol createUndefinedSymbol(String name, boolean isCode) {
         ELFSymtab symtab = createSymbolTable();
         return symtab.newUndefinedEntry(name, isCode);
     }

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachOObjectFile.java

@@ -180,7 +180,7 @@ public Symbol createDefinedSymbol(String name, Element baseSection, long positio
     }
 
     @Override
-    public Symbol createUndefinedSymbol(String name, int size, boolean isCode) {
+    public Symbol createUndefinedSymbol(String name, boolean isCode) {
         MachOSymtab symtab = (MachOSymtab) getOrCreateSymbolTable();
         return symtab.newUndefinedEntry(name, isCode);
     }

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/pecoff/PECoffObjectFile.java

@@ -114,7 +114,7 @@ public Symbol createDefinedSymbol(String name, Element baseSection, long positio
     }
 
     @Override
-    public Symbol createUndefinedSymbol(String name, int size, boolean isCode) {
+    public Symbol createUndefinedSymbol(String name, boolean isCode) {
         PECoffSymtab st = createSymbolTable();
         return st.newUndefinedEntry(name, isCode);
     }

substratevm/src/com.oracle.svm.core.graal.llvm/src/com/oracle/svm/core/graal/llvm/LLVMNativeImageCodeCache.java

@@ -293,7 +293,7 @@ public void patchMethods(DebugContext debug, RelocatableBuffer relocs, ObjectFil
                     CGlobalDataInfo info = ((CGlobalDataReference) dataPatch.reference).getDataInfo();
                     CGlobalDataImpl<?> data = info.getData();
                     if (info.isSymbolReference() && objectFile.getOrCreateSymbolTable().getSymbol(data.symbolName) == null) {
-                        objectFile.createUndefinedSymbol(data.symbolName, 0, true);
+                        objectFile.createUndefinedSymbol(data.symbolName, true);
                     }
 
                     String symbolName = (String) dataPatch.note;
@@ -319,7 +319,7 @@ public NativeTextSectionImpl getTextSectionImpl(RelocatableBuffer buffer, Object
         return new NativeTextSectionImpl(buffer, objectFile, codeCache) {
             @Override
             protected void defineMethodSymbol(String name, boolean global, Element section, HostedMethod method, CompilationResult result) {
-                ObjectFile.Symbol symbol = objectFile.createUndefinedSymbol(name, 0, true);
+                ObjectFile.Symbol symbol = objectFile.createUndefinedSymbol(name, true);
                 if (global) {
                     globalSymbols.add(symbol);
                 }

substratevm/src/com.oracle.svm.core.graal.llvm/src/com/oracle/svm/core/graal/llvm/objectfile/LLVMObjectFile.java

@@ -165,7 +165,7 @@ public Symbol createDefinedSymbol(String name, Element baseSection, long positio
     }
 
     @Override
-    public Symbol createUndefinedSymbol(String name, int size, boolean isCode) {
+    public Symbol createUndefinedSymbol(String name, boolean isCode) {
         SymbolTable symtab = getOrCreateSymbolTable();
         return symtab.newUndefinedEntry(name, isCode);
     }

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/c/CGlobalData.java

@@ -31,7 +31,7 @@
 /**
  * An object of this class represents a chunk of static global data that is located outside the heap
  * and can be accessed directly {@linkplain PointerBase by address}. No
- * {@linkplain CEntryPointOptions#prologue()} Java execution context} is required.
+ * {@linkplain CEntryPointOptions#prologue() Java execution context} is required.
  */
 public abstract class CGlobalData<T extends PointerBase> {
     /**

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/c/CGlobalDataImpl.java

@@ -24,21 +24,38 @@
  */
 package com.oracle.svm.core.c;
 
+import java.util.EnumSet;
+import java.util.Optional;
 import java.util.function.IntSupplier;
 import java.util.function.Supplier;
 
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.Platforms;
 import org.graalvm.word.PointerBase;
 
+import com.oracle.svm.core.graal.code.CGlobalDataInfo;
+
+/**
+ * Stores static information about a CGlobal. Build-specific information is stored in
+ * {@link CGlobalDataInfo}. This separation of data is used to facilitate storing
+ * {@link CGlobalDataImpl} within static fields.
+ */
 public final class CGlobalDataImpl<T extends PointerBase> extends CGlobalData<T> {
+
     /**
      * The name of the symbol to create for this data (or null to create no symbol), or if the other
      * fields are null, the name of the symbol to be referenced by this instance.
      */
     @Platforms(Platform.HOSTED_ONLY.class) //
     public final String symbolName;
 
+    /**
+     * When a CGlobal does not have a {@link #symbolName}, when building layered images,
+     * {@link #computeCodeLocation} is used as a unique key.
+     */
+    @Platforms(Platform.HOSTED_ONLY.class) //
+    public final StackWalker.StackFrame codeLocation;
+
     @Platforms(Platform.HOSTED_ONLY.class) //
     public final Supplier<byte[]> bytesSupplier;
     @Platforms(Platform.HOSTED_ONLY.class) //
@@ -74,12 +91,38 @@ public final class CGlobalDataImpl<T extends PointerBase> extends CGlobalData<T>
         this(symbolName, null, null, nonConstant);
     }
 
+    @Platforms(Platform.HOSTED_ONLY.class)
+    public boolean isSymbolReference() {
+        return bytesSupplier == null && sizeSupplier == null;
+    }
+
     @Platforms(Platform.HOSTED_ONLY.class)
     private CGlobalDataImpl(String symbolName, Supplier<byte[]> bytesSupplier, IntSupplier sizeSupplier, boolean nonConstant) {
         assert !(bytesSupplier != null && sizeSupplier != null);
         this.symbolName = symbolName;
         this.bytesSupplier = bytesSupplier;
         this.sizeSupplier = sizeSupplier;
         this.nonConstant = nonConstant;
+        /*
+         * Note the uniqueness of code locations is checked in
+         * CGlobalDataFeature#createCGlobalDataInfo.
+         */
+        this.codeLocation = this.symbolName == null ? computeCodeLocation() : null;
+    }
+
+    /**
+     * A CGlobal's code location is defined to be the caller of the {@link CGlobalDataFactory} used
+     * to create the instance.
+     */
+    @Platforms(Platform.HOSTED_ONLY.class)
+    private static StackWalker.StackFrame computeCodeLocation() {
+        var walker = StackWalker.getInstance(EnumSet.of(StackWalker.Option.RETAIN_CLASS_REFERENCE));
+        return walker.walk((stackStream) -> {
+            Optional<StackWalker.StackFrame> result = stackStream.filter(stackFrame -> {
+                var declaringClass = stackFrame.getDeclaringClass();
+                return declaringClass != CGlobalDataImpl.class && declaringClass != CGlobalDataFactory.class;
+            }).findFirst();
+            return result.get();
+        });
     }
 }

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/c/CGlobalDataNonConstantRegistry.java

@@ -24,9 +24,6 @@
  */
 package com.oracle.svm.core.c;
 
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
-
 import org.graalvm.collections.EconomicMap;
 import org.graalvm.collections.Equivalence;
 import org.graalvm.nativeimage.Platform;
@@ -45,9 +42,6 @@ public class CGlobalDataNonConstantRegistry {
 
     private final EconomicMap<CGlobalDataImpl<?>, CGlobalDataInfo> cGlobalDataInfos = ImageHeapMap.create(Equivalence.IDENTITY, "cGlobalDataInfos");
 
-    @Platforms(Platform.HOSTED_ONLY.class) //
-    private final Lock lock = new ReentrantLock();
-
     /**
      * Invoked at runtime via com.oracle.svm.hosted.c.CGlobalDataFeature#getCGlobalDataInfoMethod.
      */
@@ -62,11 +56,10 @@ public CGlobalDataInfo getCGlobalDataInfo(CGlobalDataImpl<?> cGlobalData) {
 
     @Platforms(Platform.HOSTED_ONLY.class)
     public void registerNonConstantSymbol(CGlobalDataInfo cGlobalDataInfo) {
-        lock.lock();
-        try {
-            cGlobalDataInfos.put(cGlobalDataInfo.getData(), cGlobalDataInfo);
-        } finally {
-            lock.unlock();
-        }
+        /*
+         * Note at build time ImageHeapMaps are backed by a concurrent hash map, so this code is
+         * thread safe.
+         */
+        cGlobalDataInfos.put(cGlobalDataInfo.getData(), cGlobalDataInfo);
     }
 }

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

@@ -37,6 +37,11 @@
 import com.oracle.svm.core.heap.UnknownPrimitiveField;
 import com.oracle.svm.core.util.VMError;
 
+/**
+ * Stores build-specific information about a CGlobal. Static information is stored in
+ * {@link CGlobalDataImpl}. This separation of data is used to facilitate storing
+ * {@link CGlobalDataImpl} within static fields.
+ */
 public final class CGlobalDataInfo {
     /**
      * Image heap object storing the base address of CGlobalData memory using a relocation. Before
@@ -47,7 +52,15 @@ public final class CGlobalDataInfo {
     private final CGlobalDataImpl<?> data;
     private final boolean isSymbolReference;
 
+    /**
+     * Denotes whether this CGlobal has been defined as a global in a prior layer. This also implies
+     * in a prior layer that the CGlobal was not a symbol reference.
+     */
+    @Platforms(Platform.HOSTED_ONLY.class) private final boolean definedAsGlobalInPriorLayer;
+
+    /** See CGlobalDataFeature#registerWithGlobalSymbol. */
     @UnknownPrimitiveField(availability = AfterHostedUniverse.class) private boolean isGlobalSymbol;
+    /** See CGlobalDataFeature#registerWithGlobalHiddenSymbol. */
     @UnknownPrimitiveField(availability = AfterHostedUniverse.class) private boolean isHiddenSymbol;
     @UnknownPrimitiveField(availability = AfterHeapLayout.class) private int offset = -1;
     @UnknownPrimitiveField(availability = AfterHeapLayout.class) private int size = -1;
@@ -56,11 +69,12 @@ public final class CGlobalDataInfo {
     @Platforms(HOSTED_ONLY.class) private byte[] bytes;
 
     @Platforms(Platform.HOSTED_ONLY.class)
-    public CGlobalDataInfo(CGlobalDataImpl<?> data) {
+    public CGlobalDataInfo(CGlobalDataImpl<?> data, boolean definedAsGlobalInPriorLayer) {
         assert data != null;
         this.data = data;
-        this.isSymbolReference = (data.bytesSupplier == null && data.sizeSupplier == null);
+        this.isSymbolReference = data.isSymbolReference();
         assert !this.isSymbolReference || data.symbolName != null;
+        this.definedAsGlobalInPriorLayer = definedAsGlobalInPriorLayer;
     }
 
     public CGlobalDataImpl<?> getData() {
@@ -100,7 +114,16 @@ public int getSize() {
     }
 
     public void makeGlobalSymbol() {
-        VMError.guarantee(!isSymbolReference && data.symbolName != null, "Cannot change the local/global status of a symbol reference");
+        if (isSymbolReference) {
+            /*
+             * Only CGlobals assigned to global data chunks allocated by native image can be defined
+             * as global symbols. If the symbol was defined as a global symbol in a prior layer,
+             * this request is a no-op.
+             */
+            VMError.guarantee(definedAsGlobalInPriorLayer, "Cannot change the local/global status of a symbol reference %s", data.symbolName);
+            return;
+        }
+        VMError.guarantee(data.symbolName != null, "No symbol name associated with data reference");
         isGlobalSymbol = true;
     }