[GR-33600] Espresso svm offset recomputation

ansalond · ansalond · commit 66970229cc4a · 2021-09-09T21:48:50.000Z
PullRequest: graal/9709
diff --git a/substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/TruffleBaseFeature.java b/substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/TruffleBaseFeature.java
@@ -44,6 +44,8 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.BooleanSupplier;
 
+import com.oracle.svm.core.config.ConfigurationValues;
+import com.oracle.truffle.api.staticobject.StaticProperty;
 import org.graalvm.compiler.api.replacements.SnippetReflectionProvider;
 import org.graalvm.compiler.nodes.ConstantNode;
 import org.graalvm.compiler.nodes.ValueNode;
@@ -107,6 +109,7 @@
 import jdk.vm.ci.meta.MetaAccessProvider;
 import jdk.vm.ci.meta.ResolvedJavaField;
 import jdk.vm.ci.meta.ResolvedJavaMethod;
+import sun.misc.Unsafe;
 
 /**
  * Base feature for using Truffle in the SVM. If only this feature is used (not included through
@@ -219,8 +222,9 @@ public void setProfilingEnabled(boolean profilingEnabled) {
 
     @Override
     public void cleanup() {
-        // clean the cached call target nodes to prevent them from keeping application
-        // classes alive
+        /*
+         * Clean the cached call target nodes to prevent them from keeping application classes alive
+         */
         TruffleRuntime runtime = Truffle.getRuntime();
         if (!(runtime instanceof DefaultTruffleRuntime) && !(runtime instanceof SubstrateTruffleRuntime)) {
             throw VMError.shouldNotReachHere("Only SubstrateTruffleRuntime and DefaultTruffleRuntime supported");
@@ -512,16 +516,14 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod,
 
         static void duringAnalysis(DuringAnalysisAccess access) {
             boolean requiresIteration = false;
-            // We need to register as unsafe-accessed the primitive, object, and shape
-            // fields of
-            // generated storage classes. However, these classes do not share a common super
-            // type, and their fields are not annotated. Plus, the invocation plugin does
-            // not
-            // intercept calls to `StaticShape.Builder.build()` that happen during the
-            // analysis,
-            // for example because of context pre-initialization. Therefore, we inspect the
-            // generator cache in ArrayBasedShapeGenerator, which contains references to all
-            // generated storage classes.
+            /*
+             * We need to register as unsafe-accessed the primitive, object, and shape fields of
+             * generated storage classes. However, these classes do not share a common super type,
+             * and their fields are not annotated. Plus, the invocation plugin does not intercept
+             * calls to `StaticShape.Builder.build()` that happen during the analysis, for example
+             * because of context pre-initialization. Therefore, we inspect the generator cache in
+             * ArrayBasedShapeGenerator, which contains references to all generated storage classes.
+             */
             ConcurrentHashMap<?, ?> generatorCache = ReflectionUtil.readStaticField(SHAPE_GENERATOR, "generatorCache");
             for (Map.Entry<?, ?> entry : generatorCache.entrySet()) {
                 Object shapeGenerator = entry.getValue();
@@ -625,6 +627,82 @@ private static void registerReflectionAccessesForRuntimeValidation(Class<?> stor
     }
 }
 
+@TargetClass(className = "com.oracle.truffle.api.staticobject.StaticProperty", onlyWith = TruffleBaseFeature.IsEnabled.class)
+final class Target_com_oracle_truffle_api_staticobject_StaticProperty {
+
+    @Alias @RecomputeFieldValue(kind = Kind.Custom, declClass = Target_com_oracle_truffle_api_staticobject_StaticProperty.OffsetTransformer.class) //
+    int offset;
+
+    public static final class OffsetTransformer implements RecomputeFieldValue.CustomFieldValueTransformer {
+        /*
+         * We have to use reflection to access private members instead of aliasing them in the
+         * substitution class since substitutions are present only at runtime
+         */
+        private static final Method staticPropertyGetInternalKind;
+
+        static {
+            // Checkstyle: stop
+            staticPropertyGetInternalKind = ReflectionUtil.lookupMethod(StaticProperty.class, "getInternalKindName");
+            // Checkstyle: resume
+        }
+
+        @Override
+        public Object transform(MetaAccessProvider metaAccess, ResolvedJavaField original, ResolvedJavaField annotated,
+                        Object receiver, Object originalValue) {
+            int offset = (int) originalValue;
+            if (offset == 0) {
+                /*
+                 * The offset is not yet initialized, probably because no shape was built for the
+                 * receiver static property
+                 */
+                return offset;
+            }
+
+            StaticProperty receiverStaticProperty = (StaticProperty) receiver;
+
+            String internalKindName;
+            try {
+                // Checkstyle: stop
+                internalKindName = (String) staticPropertyGetInternalKind.invoke(receiverStaticProperty);
+                // Checkstyle: resume
+            } catch (IllegalAccessException | InvocationTargetException e) {
+                throw VMError.shouldNotReachHere(e);
+            }
+
+            int baseOffset;
+            int indexScale;
+            JavaKind javaKind;
+            if (internalKindName.equals("Object")) {
+                javaKind = JavaKind.Object;
+                baseOffset = Unsafe.ARRAY_OBJECT_BASE_OFFSET;
+                indexScale = Unsafe.ARRAY_OBJECT_INDEX_SCALE;
+            } else {
+                javaKind = JavaKind.Byte;
+                baseOffset = Unsafe.ARRAY_BYTE_BASE_OFFSET;
+                indexScale = Unsafe.ARRAY_BYTE_INDEX_SCALE;
+            }
+
+            assert offset >= baseOffset && (offset - baseOffset) % indexScale == 0;
+
+            /*
+             * Reverse the offset computation to find the index
+             */
+            int index = (offset - baseOffset) / indexScale;
+
+            /*
+             * Find SVM array base offset and array index scale for this JavaKind
+             */
+            int svmArrayBaseOffset = ConfigurationValues.getObjectLayout().getArrayBaseOffset(javaKind);
+            int svmArrayIndexScaleOffset = ConfigurationValues.getObjectLayout().getArrayIndexScale(javaKind);
+
+            /*
+             * Redo the offset computation with the SVM array base offset and array index scale
+             */
+            return svmArrayBaseOffset + svmArrayIndexScaleOffset * index;
+        }
+    }
+}
+
 @TargetClass(className = "com.oracle.truffle.api.staticobject.ArrayBasedShapeGenerator", onlyWith = TruffleBaseFeature.IsEnabled.class)
 final class Target_com_oracle_truffle_api_staticobject_ArrayBasedShapeGenerator {
 
@@ -676,22 +754,22 @@ public Object transform(MetaAccessProvider metaAccess, ResolvedJavaField origina
 
 // Checkstyle: stop
 
-// If allowProcess() is disabled at build time, then we ensure that
-// ProcessBuilder is not reachable.
-// The main purpose of this is to test that ProcessBuilder is not part of the
-// image when building
-// language images with allowProcess() disabled, which we interpret as "forbid
-// shelling out to
-// external processes" (GR-14041).
+/*
+ * If allowProcess() is disabled at build time, then we ensure that ProcessBuilder is not reachable.
+ * The main purpose of this is to test that ProcessBuilder is not part of the image when building
+ * language images with allowProcess() disabled, which we interpret as
+ * "forbid shelling out to external processes" (GR-14041).
+ */
 @Delete
 @TargetClass(className = "java.lang.ProcessBuilder", onlyWith = {TruffleBaseFeature.IsEnabled.class,
                 TruffleBaseFeature.IsCreateProcessDisabled.class})
 final class Target_java_lang_ProcessBuilder {
 }
 
-// If allowProcess() is disabled at build time, then we ensure
-// ObjdumpDisassemblerProvider does not
-// try to invoke the nonexistent ProcessBuilder.
+/*
+ * If allowProcess() is disabled at build time, then we ensure ObjdumpDisassemblerProvider does not
+ * try to invoke the nonexistent ProcessBuilder.
+ */
 @TargetClass(className = "org.graalvm.compiler.code.ObjdumpDisassemblerProvider", onlyWith = {
                 TruffleBaseFeature.IsEnabled.class, TruffleBaseFeature.IsCreateProcessDisabled.class})
 final class Target_org_graalvm_compiler_code_ObjdumpDisassemblerProvider {
diff --git a/truffle/src/com.oracle.truffle.api.staticobject.test/src/com/oracle/truffle/api/staticobject/test/StaticTest.java b/truffle/src/com.oracle.truffle.api.staticobject.test/src/com/oracle/truffle/api/staticobject/test/StaticTest.java
@@ -53,22 +53,112 @@
  * built time for context pre-initialization.
  */
 public class StaticTest {
-    private static final StaticProperty property;
+    private static final StaticProperty propertyB;
+    private static final StaticProperty propertyS;
+    private static final StaticProperty propertyC;
+    private static final StaticProperty propertyI;
+    private static final StaticProperty propertyL;
+    private static final StaticProperty propertyF;
+    private static final StaticProperty propertyD;
+    private static final StaticProperty propertyTF;
+    private static final StaticProperty propertyO1;
+    private static final StaticProperty propertyO2;
     private static final Object staticObject;
+    private static final byte testValueB = 1;
+    private static final short testValueS = 2;
+    private static final char testValueC = 'a';
+    private static final int testValueI = 3;
+    private static final long testValueL = 4;
+    private static final float testValueF = 5.0f;
+    private static final double testValueD = 6.0;
+    private static final boolean testValueTF = true;
+    private static final Object testValueO1 = "object1";
+    private static final Object testValueO2 = "object2";
 
     static {
         TestEnvironment environment = new TestEnvironment(new TestConfiguration(true, false));
         StaticShape.Builder builder = StaticShape.newBuilder(environment.testLanguage);
-        property = new DefaultStaticProperty("property");
-        builder.property(property, int.class, false);
+        propertyB = new DefaultStaticProperty("propertyB");
+        propertyS = new DefaultStaticProperty("propertyS");
+        propertyC = new DefaultStaticProperty("propertyC");
+        propertyI = new DefaultStaticProperty("propertyI");
+        propertyL = new DefaultStaticProperty("propertyL");
+        propertyF = new DefaultStaticProperty("propertyF");
+        propertyD = new DefaultStaticProperty("propertyD");
+        propertyTF = new DefaultStaticProperty("propertyTF");
+        propertyO1 = new DefaultStaticProperty("propertyO1");
+        propertyO2 = new DefaultStaticProperty("propertyO2");
+        builder.property(propertyB, byte.class, false);
+        builder.property(propertyS, short.class, false);
+        builder.property(propertyC, char.class, false);
+        builder.property(propertyI, int.class, false);
+        builder.property(propertyL, long.class, false);
+        builder.property(propertyF, float.class, false);
+        builder.property(propertyD, double.class, false);
+        builder.property(propertyTF, boolean.class, false);
+        builder.property(propertyO1, Object.class, false);
+        builder.property(propertyO2, Object.class, false);
         staticObject = builder.build().getFactory().create();
-        property.setInt(staticObject, 42);
+        propertyB.setByte(staticObject, testValueB);
+        propertyS.setShort(staticObject, testValueS);
+        propertyC.setChar(staticObject, testValueC);
+        propertyI.setInt(staticObject, testValueI);
+        propertyL.setLong(staticObject, testValueL);
+        propertyF.setFloat(staticObject, testValueF);
+        propertyD.setDouble(staticObject, testValueD);
+        propertyTF.setBoolean(staticObject, testValueTF);
+        propertyO1.setObject(staticObject, testValueO1);
+        propertyO2.setObject(staticObject, testValueO2);
     }
 
     @Test
     public void staticallyDeclaredStaticObject() {
-        Assert.assertEquals(42, property.getInt(staticObject));
-        property.setInt(staticObject, 24);
-        Assert.assertEquals(24, property.getInt(staticObject));
+        Assert.assertEquals(testValueB, propertyB.getByte(staticObject));
+        Assert.assertEquals(testValueS, propertyS.getShort(staticObject));
+        Assert.assertEquals(testValueC, propertyC.getChar(staticObject));
+        Assert.assertEquals(testValueI, propertyI.getInt(staticObject));
+        Assert.assertEquals(testValueL, propertyL.getLong(staticObject));
+        Assert.assertEquals(testValueF, propertyF.getFloat(staticObject), 1e-6f);
+        Assert.assertEquals(testValueD, propertyD.getDouble(staticObject), 1e-6);
+        Assert.assertEquals(testValueTF, propertyTF.getBoolean(staticObject));
+        Assert.assertEquals(testValueO1, propertyO1.getObject(staticObject));
+        Assert.assertEquals(testValueO2, propertyO2.getObject(staticObject));
+
+        byte newTestValueB = 11;
+        propertyB.setByte(staticObject, newTestValueB);
+        Assert.assertEquals(newTestValueB, propertyB.getByte(staticObject));
+
+        short newTestValueS = 22;
+        propertyS.setShort(staticObject, newTestValueS);
+        Assert.assertEquals(newTestValueS, propertyS.getShort(staticObject));
+
+        char newTestValueC = 'b';
+        propertyC.setChar(staticObject, newTestValueC);
+        Assert.assertEquals(newTestValueC, propertyC.getChar(staticObject));
+
+        int newTestValueI = 33;
+        propertyI.setInt(staticObject, newTestValueI);
+        Assert.assertEquals(newTestValueI, propertyI.getInt(staticObject));
+
+        long newTestValueL = 44;
+        propertyL.setLong(staticObject, newTestValueL);
+        Assert.assertEquals(newTestValueL, propertyL.getLong(staticObject));
+
+        float newTestValueF = 55.0f;
+        propertyF.setFloat(staticObject, newTestValueF);
+        Assert.assertEquals(newTestValueF, propertyF.getFloat(staticObject), 1e-6f);
+
+        double newTestValueD = 66.0;
+        propertyD.setDouble(staticObject, newTestValueD);
+        Assert.assertEquals(newTestValueD, propertyD.getDouble(staticObject), 1e-6);
+
+        propertyTF.setBoolean(staticObject, false);
+        Assert.assertFalse(propertyTF.getBoolean(staticObject));
+
+        Object newTestValueO = "object3";
+        propertyO1.setObject(staticObject, newTestValueO);
+        Assert.assertEquals(newTestValueO, propertyO1.getObject(staticObject));
+        propertyO2.setObject(staticObject, newTestValueO);
+        Assert.assertEquals(newTestValueO, propertyO2.getObject(staticObject));
     }
 }
diff --git a/truffle/src/com.oracle.truffle.api.staticobject/src/com/oracle/truffle/api/staticobject/ArrayBasedStaticShape.java b/truffle/src/com.oracle.truffle.api.staticobject/src/com/oracle/truffle/api/staticobject/ArrayBasedStaticShape.java
@@ -225,6 +225,8 @@ static class ArrayBasedPropertyLayout {
                 byte propertyKind = staticProperty.getInternalKind();
                 int index;
                 if (propertyKind == StaticPropertyKind.Object.toByte()) {
+                    // These offsets are re-computed for SVM:
+                    // TruffleBaseFeature.Target_com_oracle_truffle_api_staticobject_StaticProperty
                     index = Unsafe.ARRAY_OBJECT_BASE_OFFSET + Unsafe.ARRAY_OBJECT_INDEX_SCALE * objArraySize++;
                 } else {
                     index = primitiveFieldIndexes.getIndex(propertyKind);
