[GR-70641] Use LibC.free(...) when freeing native memory that was allocated by the libc or other native code.

PullRequest: graal/22329

Author: christianhaeubl

substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/PosixProcessPropertiesSupport.java

@@ -41,7 +41,7 @@
 import com.oracle.svm.core.BaseProcessPropertiesSupport;
 import com.oracle.svm.core.c.locale.LocaleSupport;
 import com.oracle.svm.core.graal.stackvalue.UnsafeStackValue;
-import com.oracle.svm.core.memory.UntrackedNullableNativeMemory;
+import com.oracle.svm.core.headers.LibC;
 import com.oracle.svm.core.posix.headers.Dlfcn;
 import com.oracle.svm.core.posix.headers.Signal;
 import com.oracle.svm.core.posix.headers.Stdlib;
@@ -105,7 +105,7 @@ public String getObjectFile(PointerBase symbolAddress) {
         try {
             return CTypeConversion.toJavaString(realpath);
         } finally {
-            UntrackedNullableNativeMemory.free(realpath);
+            LibC.free(realpath);
         }
     }
 
@@ -165,11 +165,13 @@ protected static String realpath(String path) {
             if (realpath.isNull()) {
                 /* Failure to find a real path. */
                 return null;
-            } else {
+            }
+
+            try {
                 /* Success */
-                String result = CTypeConversion.toJavaString(realpath);
-                UntrackedNullableNativeMemory.free(realpath);
-                return result;
+                return CTypeConversion.toJavaString(realpath);
+            } finally {
+                LibC.free(realpath);
             }
         }
     }

substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/darwin/DarwinSystemPropertiesSupport.java

@@ -24,7 +24,6 @@
  */
 package com.oracle.svm.core.posix.darwin;
 
-import jdk.graal.compiler.word.Word;
 import org.graalvm.nativeimage.ImageSingletons;
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.c.function.CLibrary;
@@ -37,14 +36,16 @@
 import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature;
 import com.oracle.svm.core.feature.InternalFeature;
 import com.oracle.svm.core.graal.stackvalue.UnsafeStackValue;
+import com.oracle.svm.core.headers.LibC;
 import com.oracle.svm.core.jdk.SystemPropertiesSupport;
-import com.oracle.svm.core.memory.UntrackedNullableNativeMemory;
 import com.oracle.svm.core.posix.PosixSystemPropertiesSupport;
 import com.oracle.svm.core.posix.headers.Limits;
 import com.oracle.svm.core.posix.headers.Stdlib;
 import com.oracle.svm.core.posix.headers.Unistd;
 import com.oracle.svm.core.posix.headers.darwin.Foundation;
 
+import jdk.graal.compiler.word.Word;
+
 @CLibrary(value = "darwin", requireStatic = true)
 public class DarwinSystemPropertiesSupport extends PosixSystemPropertiesSupport {
 
@@ -105,7 +106,7 @@ protected String osVersionValue() {
                 CCharPointer osVersionStr = Foundation.systemVersionPlatform();
                 if (osVersionStr.isNonNull()) {
                     osVersionValue = CTypeConversion.toJavaString(osVersionStr);
-                    UntrackedNullableNativeMemory.free(osVersionStr);
+                    LibC.free(osVersionStr);
                     return osVersionValue;
                 }
             } else {
@@ -120,7 +121,7 @@ protected String osVersionValue() {
         CCharPointer osVersionStr = Foundation.systemVersionPlatformFallback();
         if (osVersionStr.isNonNull()) {
             osVersionValue = CTypeConversion.toJavaString(osVersionStr);
-            UntrackedNullableNativeMemory.free(osVersionStr);
+            LibC.free(osVersionStr);
             return osVersionValue;
         }
         return osVersionValue = "Unknown";

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/headers/LibC.java

@@ -24,7 +24,10 @@
  */
 package com.oracle.svm.core.headers;
 
+import static com.oracle.svm.core.Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE;
+
 import org.graalvm.nativeimage.ImageSingletons;
+import org.graalvm.nativeimage.UnmanagedMemory;
 import org.graalvm.nativeimage.c.type.CCharPointer;
 import org.graalvm.nativeimage.c.type.CCharPointerPointer;
 import org.graalvm.word.PointerBase;
@@ -34,6 +37,9 @@
 import com.oracle.svm.core.Uninterruptible;
 import com.oracle.svm.core.imagelayer.ImageLayerBuildingSupport;
 import com.oracle.svm.core.layeredimagesingleton.LayeredImageSingletonSupport;
+import com.oracle.svm.core.memory.NativeMemory;
+import com.oracle.svm.core.memory.NullableNativeMemory;
+import com.oracle.svm.core.memory.UntrackedNullableNativeMemory;
 import com.oracle.svm.core.traits.SingletonLayeredInstallationKind;
 import com.oracle.svm.core.traits.SingletonTraitKind;
 
@@ -88,6 +94,17 @@ public static void abort() {
         exit(EXIT_CODE_ABORT);
     }
 
+    /**
+     * May only be used to free memory that was allocated by the libc or other native code. Do
+     * <b>NOT</b> use this method when freeing memory that was allocated via internal APIs such as
+     * {@link NativeMemory}, {@link NullableNativeMemory}, {@link UntrackedNullableNativeMemory}, or
+     * {@link UnmanagedMemory}.
+     */
+    @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
+    public static void free(PointerBase ptr) {
+        libc().free(ptr);
+    }
+
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
     public static UnsignedWord strlen(CCharPointer str) {
         return libc().strlen(str);

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

@@ -31,7 +31,6 @@
 import java.time.ZoneId;
 import java.util.TimeZone;
 
-import jdk.graal.compiler.word.Word;
 import org.graalvm.collections.EconomicMap;
 import org.graalvm.nativeimage.ImageSingletons;
 import org.graalvm.nativeimage.Platforms;
@@ -48,12 +47,13 @@
 import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature;
 import com.oracle.svm.core.feature.InternalFeature;
 import com.oracle.svm.core.handles.PrimitiveArrayView;
-import com.oracle.svm.core.memory.UntrackedNullableNativeMemory;
+import com.oracle.svm.core.headers.LibC;
 import com.oracle.svm.core.option.HostedOptionKey;
 import com.oracle.svm.core.util.VMError;
 
 import jdk.graal.compiler.options.Option;
 import jdk.graal.compiler.options.OptionKey;
+import jdk.graal.compiler.word.Word;
 
 /**
  * The following classes aim to provide full support for time zones for native-image. This
@@ -101,7 +101,7 @@ private static String getSystemTimeZoneID(String javaHome) {
             CCharPointer tzId = LibCHelper.SVM_FindJavaTZmd(tzMappingsPtr, contentLen);
             String result = CTypeConversion.toJavaString(tzId);
             // SVM_FindJavaTZmd returns a newly allocated string
-            UntrackedNullableNativeMemory.free(tzId);
+            LibC.free(tzId);
             return result;
         } finally {
             if (refContent != null) {

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/memory/NativeMemory.java

@@ -27,7 +27,6 @@
 
 import static com.oracle.svm.core.Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE;
 
-import jdk.graal.compiler.word.Word;
 import org.graalvm.nativeimage.UnmanagedMemory;
 import org.graalvm.nativeimage.impl.UnmanagedMemorySupport;
 import org.graalvm.word.PointerBase;
@@ -36,24 +35,16 @@
 import com.oracle.svm.core.Uninterruptible;
 import com.oracle.svm.core.nmt.NmtCategory;
 
+import jdk.graal.compiler.word.Word;
+
 /**
  * Internal API for managing native memory. This class supports native memory tracking (NMT) and is
  * therefore preferred over the public API class {@link UnmanagedMemory} and its
  * {@link UnmanagedMemorySupport implementations}.
- *
+ * <p>
  * All methods that allocate native memory throw an {@link OutOfMemoryError} if the memory
  * allocation fails. If native memory needs to be allocated from uninterruptible code, use
  * {@link NullableNativeMemory} instead.
- *
- * Note that NMT may cause segfaults in certain scenarios:
- * <ul>
- * <li>Native memory that was allocated outside of Java (e.g., in a C library) or via some API that
- * does not support NMT (e.g., {@link UnmanagedMemory}) may only be freed with
- * {@link UntrackedNullableNativeMemory#free}. This is necessary because NMT assumes that each
- * allocation has a custom header, which isn't true for such memory.</li>
- * <li>NMT accesses the image heap. If native memory needs to be allocated before the image heap is
- * mapped or after it was unmapped, {@link UntrackedNullableNativeMemory} must be used.</li>
- * </ul>
  */
 public class NativeMemory {
     /**
@@ -139,7 +130,8 @@ public static <T extends PointerBase> T realloc(T ptr, int size, NmtCategory cat
      * <p>
      * Note that this method must <b>NOT</b> be used to free memory that was allocated via other
      * classes (e.g., {@link UnmanagedMemorySupport}) or outside of Native Image code (e.g., in a C
-     * library).
+     * library). Otherwise, segfaults can occur because NMT assumes that each allocation has a
+     * custom header, which isn't true for such memory.
      */
     @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
     public static void free(PointerBase ptr) {

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/nmt/NmtCategory.java

@@ -41,6 +41,8 @@ public enum NmtCategory {
     HeapDump("Heap Dump"),
     /** Image heap (may include GC-specific data). */
     ImageHeap("Image Heap"),
+    /** Interpreter. */
+    Interpreter("Interpreter"),
     /** Collected Java heap (may include GC-specific data). */
     JavaHeap("Java Heap"),
     /** Java Flight Recorder. */

substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/InterpreterStubSection.java

@@ -40,7 +40,6 @@
 import org.graalvm.nativeimage.Platforms;
 import org.graalvm.nativeimage.StackValue;
 import org.graalvm.nativeimage.c.function.CFunctionPointer;
-import org.graalvm.nativeimage.impl.UnmanagedMemorySupport;
 import org.graalvm.word.Pointer;
 
 import com.oracle.objectfile.BasicProgbitsSectionImpl;
@@ -59,6 +58,8 @@
 import com.oracle.svm.core.graal.code.SubstrateCallingConventionType;
 import com.oracle.svm.core.hub.DynamicHub;
 import com.oracle.svm.core.jdk.InternalVMMethod;
+import com.oracle.svm.core.memory.NativeMemory;
+import com.oracle.svm.core.nmt.NmtCategory;
 import com.oracle.svm.core.util.VMError;
 import com.oracle.svm.hosted.image.AbstractImage;
 import com.oracle.svm.hosted.image.NativeImage;
@@ -356,7 +357,7 @@ public static Object leaveInterpreter(CFunctionPointer compiledEntryPoint, Inter
 
         Pointer stackBuffer = Word.nullPointer();
         if (stackSize > 0) {
-            stackBuffer = ImageSingletons.lookup(UnmanagedMemorySupport.class).malloc(Word.unsigned(stackSize));
+            stackBuffer = NativeMemory.malloc(Word.unsigned(stackSize), NmtCategory.Interpreter);
             accessHelper.setSp(leaveData, stackSize, stackBuffer);
         }
 
@@ -421,7 +422,7 @@ public static Object leaveInterpreter(CFunctionPointer compiledEntryPoint, Inter
         } finally {
             if (stackSize > 0) {
                 VMError.guarantee(stackBuffer.isNonNull());
-                ImageSingletons.lookup(UnmanagedMemorySupport.class).free(stackBuffer);
+                NativeMemory.free(stackBuffer);
             }
         }