Fill unused vtable slots with a stub that reports a fatal error

Author: Christian Wimmer

substratevm/src/com.oracle.svm.core.graal.amd64/src/com/oracle/svm/core/graal/amd64/AMD64CalleeSavedRegisters.java

@@ -36,14 +36,17 @@
 import org.graalvm.nativeimage.ImageSingletons;
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.Platforms;
+import org.graalvm.word.Pointer;
 
 import com.oracle.svm.core.CalleeSavedRegisters;
 import com.oracle.svm.core.FrameAccess;
+import com.oracle.svm.core.RegisterDumper;
 import com.oracle.svm.core.SubstrateOptions;
 import com.oracle.svm.core.SubstrateTargetDescription;
 import com.oracle.svm.core.amd64.AMD64CPUFeatureAccess;
 import com.oracle.svm.core.config.ConfigurationValues;
 import com.oracle.svm.core.graal.meta.SubstrateRegisterConfig;
+import com.oracle.svm.core.log.Log;
 import com.oracle.svm.core.util.VMError;
 
 import jdk.vm.ci.amd64.AMD64;
@@ -162,4 +165,48 @@ public void emitRestore(AMD64MacroAssembler asm, int frameSize, Register exclude
     private AMD64Address calleeSaveAddress(AMD64MacroAssembler asm, int frameSize, Register register) {
         return asm.makeAddress(frameRegister, frameSize + getOffsetInFrame(register));
     }
+
+    @Override
+    public void dumpRegisters(Log log, Pointer callerSP, boolean printLocationInfo, boolean allowJavaHeapAccess) {
+        log.string("Callee saved registers (sp=").zhex(callerSP).string(")").indent(true);
+        /*
+         * The loop to print all registers is manually unrolled so that the register order is
+         * defined, and also so that the lookup of the "offset in frame" can be constant folded at
+         * image build time using a @Fold method.
+         */
+        dumpReg(log, "RAX ", callerSP, offsetInFrameOrNull(AMD64.rax), printLocationInfo, allowJavaHeapAccess);
+        dumpReg(log, "RBX ", callerSP, offsetInFrameOrNull(AMD64.rbx), printLocationInfo, allowJavaHeapAccess);
+        dumpReg(log, "RCX ", callerSP, offsetInFrameOrNull(AMD64.rcx), printLocationInfo, allowJavaHeapAccess);
+        dumpReg(log, "RDX ", callerSP, offsetInFrameOrNull(AMD64.rdx), printLocationInfo, allowJavaHeapAccess);
+        dumpReg(log, "RBP ", callerSP, offsetInFrameOrNull(AMD64.rbp), printLocationInfo, allowJavaHeapAccess);
+        dumpReg(log, "RSI ", callerSP, offsetInFrameOrNull(AMD64.rsi), printLocationInfo, allowJavaHeapAccess);
+        dumpReg(log, "RDI ", callerSP, offsetInFrameOrNull(AMD64.rdi), printLocationInfo, allowJavaHeapAccess);
+        dumpReg(log, "RSP ", callerSP, offsetInFrameOrNull(AMD64.rsp), printLocationInfo, allowJavaHeapAccess);
+        dumpReg(log, "R8  ", callerSP, offsetInFrameOrNull(AMD64.r8), printLocationInfo, allowJavaHeapAccess);
+        dumpReg(log, "R9  ", callerSP, offsetInFrameOrNull(AMD64.r9), printLocationInfo, allowJavaHeapAccess);
+        dumpReg(log, "R10 ", callerSP, offsetInFrameOrNull(AMD64.r10), printLocationInfo, allowJavaHeapAccess);
+        dumpReg(log, "R11 ", callerSP, offsetInFrameOrNull(AMD64.r11), printLocationInfo, allowJavaHeapAccess);
+        dumpReg(log, "R12 ", callerSP, offsetInFrameOrNull(AMD64.r12), printLocationInfo, allowJavaHeapAccess);
+        dumpReg(log, "R13 ", callerSP, offsetInFrameOrNull(AMD64.r13), printLocationInfo, allowJavaHeapAccess);
+        dumpReg(log, "R14 ", callerSP, offsetInFrameOrNull(AMD64.r14), printLocationInfo, allowJavaHeapAccess);
+        dumpReg(log, "R15 ", callerSP, offsetInFrameOrNull(AMD64.r15), printLocationInfo, allowJavaHeapAccess);
+        log.indent(false);
+    }
+
+    private static void dumpReg(Log log, String label, Pointer callerSP, int offsetInFrameOrNull, boolean printLocationInfo, boolean allowJavaHeapAccess) {
+        if (offsetInFrameOrNull != 0) {
+            long value = callerSP.readLong(offsetInFrameOrNull);
+            RegisterDumper.dumpReg(log, label, value, printLocationInfo, allowJavaHeapAccess);
+        }
+    }
+
+    @Fold
+    static int offsetInFrameOrNull(Register register) {
+        AMD64CalleeSavedRegisters that = AMD64CalleeSavedRegisters.singleton();
+        if (that.calleeSavedRegisters.contains(register)) {
+            return that.getOffsetInFrame(register);
+        } else {
+            return 0;
+        }
+    }
 }

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

@@ -31,9 +31,11 @@
 import org.graalvm.nativeimage.ImageSingletons;
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.Platforms;
+import org.graalvm.word.Pointer;
 
 import com.oracle.svm.core.code.FrameInfoEncoder;
 import com.oracle.svm.core.heap.SubstrateReferenceMapBuilder;
+import com.oracle.svm.core.log.Log;
 import com.oracle.svm.core.util.VMError;
 
 import jdk.vm.ci.code.Register;
@@ -107,4 +109,13 @@ public int getOffsetInFrame(Register register) {
         assert result < 0 : "Note that the offset of a callee save register is negative, because it is located in the callee frame";
         return result;
     }
+
+    /**
+     * Optional method for subclasses to implement. It is called during diagnostic printing to print
+     * the values of saved registers of the provided stack frame. The caller must ensure that the
+     * provided frame really has callee saved registers, since that cannot be checked automatically.
+     */
+    @SuppressWarnings("unused")
+    public void dumpRegisters(Log log, Pointer callerSP, boolean printLocationInfo, boolean allowJavaHeapAccess) {
+    }
 }

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

@@ -0,0 +1,74 @@
+/*
+ * Copyright (c) 2021, 2021, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package com.oracle.svm.core;
+
+import java.lang.reflect.Method;
+
+import org.graalvm.nativeimage.ImageSingletons;
+import org.graalvm.nativeimage.LogHandler;
+import org.graalvm.nativeimage.c.function.CodePointer;
+import org.graalvm.word.Pointer;
+import org.graalvm.word.WordFactory;
+
+import com.oracle.svm.core.annotate.NeverInline;
+import com.oracle.svm.core.annotate.StubCallingConvention;
+import com.oracle.svm.core.log.Log;
+import com.oracle.svm.core.snippets.KnownIntrinsics;
+import com.oracle.svm.core.stack.StackOverflowCheck;
+import com.oracle.svm.core.thread.VMThreads;
+import com.oracle.svm.util.ReflectionUtil;
+
+/**
+ * Provides a stub method that can be used to fill otherwise unused vtable slots. Instead of a
+ * segfault, this method provides a full diagnostic output with a stack trace.
+ */
+public final class InvalidVTableEntryHandler {
+    public static final Method HANDLER_METHOD = ReflectionUtil.lookupMethod(InvalidVTableEntryHandler.class, "invalidVTableEntryHandler");
+    public static final String MSG = "Fatal error: Virtual method call used an illegal vtable entry that was seen as unused by the static analysis";
+
+    @StubCallingConvention
+    @NeverInline("We need a separate frame that stores all registers")
+    private static void invalidVTableEntryHandler() {
+        VMThreads.StatusSupport.setStatusIgnoreSafepoints();
+        StackOverflowCheck.singleton().disableStackOverflowChecksForFatalError();
+
+        /*
+         * Since this is so far the only use case we have for a fatal error with
+         * frameHasCalleeSavedRegisters=true, we inline the usual fatal error handling. Note that
+         * this has the added benefit that the instructions printed as part of the crash dump are
+         * from the method that has the illegal vtable call. That can be helpful when debugging the
+         * cause of the fatal error.
+         */
+        Pointer callerSP = KnownIntrinsics.readCallerStackPointer();
+        CodePointer callerIP = KnownIntrinsics.readReturnAddress();
+        LogHandler logHandler = ImageSingletons.lookup(LogHandler.class);
+        Log log = Log.enterFatalContext(logHandler, callerIP, MSG, null);
+        if (log != null) {
+            SubstrateDiagnostics.print(log, callerSP, callerIP, WordFactory.nullPointer(), true);
+            log.string(MSG).newline();
+        }
+        logHandler.fatalError();
+    }
+}

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

@@ -112,19 +112,19 @@ public static void printLocationInfo(Log log, UnsignedWord value, boolean allowJ
 
     /** Prints extensive diagnostic information to the given Log. */
     public static boolean print(Log log, Pointer sp, CodePointer ip) {
-        return print(log, sp, ip, WordFactory.nullPointer());
+        return print(log, sp, ip, WordFactory.nullPointer(), false);
     }
 
     /**
      * Print diagnostics for the various subsystems. If a fatal error occurs while printing
      * diagnostics, it can happen that the same thread enters this method multiple times.
      */
     @RestrictHeapAccess(access = RestrictHeapAccess.Access.NO_ALLOCATION, reason = "Must not allocate during printing diagnostics.")
-    static boolean print(Log log, Pointer sp, CodePointer ip, RegisterDumper.Context context) {
+    public static boolean print(Log log, Pointer sp, CodePointer ip, RegisterDumper.Context context, boolean frameHasCalleeSavedRegisters) {
         log.newline();
         // Save the state of the initial error so that this state is consistently used, even if
         // further errors occur while printing diagnostics.
-        if (!state.trySet(log, sp, ip, context) && !isInProgressByCurrentThread()) {
+        if (!state.trySet(log, sp, ip, context, frameHasCalleeSavedRegisters) && !isInProgressByCurrentThread()) {
             log.string("Error: printDiagnostics already in progress by another thread.").newline();
             log.newline();
             return false;
@@ -244,16 +244,18 @@ private static class PrintDiagnosticsState {
         Pointer sp;
         CodePointer ip;
         RegisterDumper.Context context;
+        boolean frameHasCalleeSavedRegisters;
 
         @SuppressWarnings("hiding")
-        public boolean trySet(Log log, Pointer sp, CodePointer ip, RegisterDumper.Context context) {
+        public boolean trySet(Log log, Pointer sp, CodePointer ip, RegisterDumper.Context context, boolean frameHasCalleeSavedRegisters) {
             if (diagnosticThread.compareAndSet(WordFactory.nullPointer(), CurrentIsolate.getCurrentThread())) {
                 assert diagnosticThunkIndex == 0;
                 assert invocationCount == 0;
                 this.log = log;
                 this.sp = sp;
                 this.ip = ip;
                 this.context = context;
+                this.frameHasCalleeSavedRegisters = frameHasCalleeSavedRegisters;
                 return true;
             }
             return false;
@@ -264,6 +266,7 @@ public void clear() {
             sp = WordFactory.nullPointer();
             ip = WordFactory.nullPointer();
             context = WordFactory.nullPointer();
+            frameHasCalleeSavedRegisters = false;
 
             diagnosticThunkIndex = 0;
             invocationCount = 0;
@@ -286,6 +289,9 @@ public void printDiagnostics(Log log, int invocationCount) {
                 log.string("General purpose register values:").indent(true);
                 RegisterDumper.singleton().dumpRegisters(log, context, invocationCount <= 2, invocationCount == 1);
                 log.indent(false);
+            } else if (CalleeSavedRegisters.supportedByPlatform() && state.frameHasCalleeSavedRegisters) {
+                CalleeSavedRegisters.singleton().dumpRegisters(log, state.sp, invocationCount <= 2, invocationCount == 1);
+
             }
         }
     }

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

@@ -155,7 +155,7 @@ private static void dumpInterruptibly(PointerBase signalInfo, RegisterDumper.Con
 
             PointerBase sp = RegisterDumper.singleton().getSP(context);
             PointerBase ip = RegisterDumper.singleton().getIP(context);
-            boolean printedDiagnostics = SubstrateDiagnostics.print(log, (Pointer) sp, (CodePointer) ip, context);
+            boolean printedDiagnostics = SubstrateDiagnostics.print(log, (Pointer) sp, (CodePointer) ip, context, false);
             if (printedDiagnostics) {
                 log.string("Segfault detected, aborting process. Use runtime option -R:-InstallSegfaultHandler if you don't want to use SubstrateSegfaultHandler.").newline();
                 log.newline();

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

@@ -254,7 +254,7 @@ public interface Thunk {
 
     /** Prints extensive diagnostic information to the given Log. */
     public static boolean printDiagnostics(Log log, Pointer sp, CodePointer ip) {
-        return SubstrateDiagnostics.print(log, sp, ip, WordFactory.nullPointer());
+        return SubstrateDiagnostics.print(log, sp, ip, WordFactory.nullPointer(), false);
     }
 
     /**

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/UniverseBuilder.java

@@ -48,6 +48,7 @@
 import org.graalvm.nativeimage.c.function.CEntryPointLiteral;
 import org.graalvm.nativeimage.c.function.CFunction;
 import org.graalvm.nativeimage.c.function.CFunctionPointer;
+import org.graalvm.nativeimage.hosted.Feature;
 
 import com.oracle.graal.pointsto.BigBang;
 import com.oracle.graal.pointsto.constraints.UnsupportedFeatures;
@@ -60,9 +61,11 @@
 import com.oracle.graal.pointsto.meta.AnalysisType;
 import com.oracle.graal.pointsto.meta.AnalysisUniverse;
 import com.oracle.graal.pointsto.results.AbstractAnalysisResultsBuilder;
+import com.oracle.svm.core.InvalidVTableEntryHandler;
 import com.oracle.svm.core.StaticFieldsSupport;
 import com.oracle.svm.core.SubstrateOptions;
 import com.oracle.svm.core.SubstrateUtil;
+import com.oracle.svm.core.annotate.AutomaticFeature;
 import com.oracle.svm.core.annotate.ExcludeFromReferenceMap;
 import com.oracle.svm.core.c.BoxedRelocatedPointer;
 import com.oracle.svm.core.c.function.CFunctionOptions;
@@ -80,6 +83,7 @@
 import com.oracle.svm.core.hub.DynamicHubSupport;
 import com.oracle.svm.core.hub.LayoutEncoding;
 import com.oracle.svm.core.util.VMError;
+import com.oracle.svm.hosted.FeatureImpl.BeforeAnalysisAccessImpl;
 import com.oracle.svm.hosted.HostedConfiguration;
 import com.oracle.svm.hosted.NativeImageOptions;
 import com.oracle.svm.hosted.config.HybridLayout;
@@ -651,6 +655,12 @@ private void buildVTables() {
          */
         buildVTable(objectClass, vtablesMap, usedSlotsMap, vtablesSlots);
 
+        /*
+         * To avoid segfaults when jumping to address 0, all unused vtable entries are filled with a
+         * stub that reports a fatal error.
+         */
+        HostedMethod invalidVTableEntryHandler = hMetaAccess.lookupJavaMethod(InvalidVTableEntryHandler.HANDLER_METHOD);
+
         for (HostedType type : hUniverse.getTypes()) {
             if (type.isArray()) {
                 type.vtable = objectClass.vtable;
@@ -659,13 +669,20 @@ private void buildVTables() {
                 assert type.isInterface() || type.isPrimitive();
                 type.vtable = new HostedMethod[0];
             }
+
+            HostedMethod[] vtableArray = type.vtable;
+            for (int i = 0; i < vtableArray.length; i++) {
+                if (vtableArray[i] == null) {
+                    vtableArray[i] = invalidVTableEntryHandler;
+                }
+            }
         }
 
         if (SubstrateUtil.assertionsEnabled()) {
             /* Check that all vtable entries are the correctly resolved methods. */
             for (HostedType type : hUniverse.getTypes()) {
                 for (HostedMethod m : type.vtable) {
-                    assert m == null || m.equals(hUniverse.lookup(type.wrapped.resolveConcreteMethod(m.wrapped, type.wrapped)));
+                    assert m == null || m.equals(invalidVTableEntryHandler) || m.equals(hUniverse.lookup(type.wrapped.resolveConcreteMethod(m.wrapped, type.wrapped)));
                 }
             }
         }
@@ -960,3 +977,13 @@ private void processFieldLocations() {
         }
     }
 }
+
+@AutomaticFeature
+final class InvalidVTableEntryFeature implements Feature {
+
+    @Override
+    public void beforeAnalysis(BeforeAnalysisAccess a) {
+        BeforeAnalysisAccessImpl access = (BeforeAnalysisAccessImpl) a;
+        access.registerAsCompiled(InvalidVTableEntryHandler.HANDLER_METHOD);
+    }
+}