JDK-8321466 : SIGSEGV when reading from a MemorySegment when address is 0
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.lang.foreign
  • Affected Version: 22
  • Priority: P4
  • Status: Closed
  • Resolution: Not an Issue
  • OS: os_x
  • CPU: aarch64
  • Submitted: 2023-12-06
  • Updated: 2023-12-11
  • Resolved: 2023-12-07
Related Reports
Relates :  
Description
On operations like MemorySegment::get, the operation should probably fail with an `IllegalStateException` or equivalent. Otherwise this leads to segfault.


Typically I was refactoring some code and also plying with the API, and stumble on this crash.

Clearly my code is wrong, yet I don't thin this should crash the JVM

```
package io.github.bric3.panama.b.memory;

import java.lang.foreign.Arena;
import java.lang.foreign.MemoryLayout;
import java.lang.foreign.MemoryLayout.PathElement;
import java.lang.foreign.MemorySegment;

import static java.lang.foreign.ValueLayout.ADDRESS;
import static java.lang.foreign.ValueLayout.JAVA_BYTE;

public class AccessingData {
  public static void main(String[] args) {
    var basic_struct_LAYOUT = MemoryLayout.structLayout(
            MemoryLayout.sequenceLayout(
                    64,
                    JAVA_BYTE
            ).withName("buf"),
            JAVA_BYTE.withName("buf_len"),
            JAVA_BYTE.withName("flags")
    ).withName("basic_struct");

    var pathToFlags = PathElement.groupElement("flags");
    var flags = basic_struct_LAYOUT.varHandle(
            pathToFlags
    );

    try (var arena = Arena.ofConfined()) {
      var memorySegment = arena.allocate(basic_struct_LAYOUT);
      flags.set(memorySegment, 0L, (byte) 0b0001_0001);

      // the bad code
      var retargeted = memorySegment.get(ADDRESS.withTargetLayout(basic_struct_LAYOUT), 0);
      byte flagsValue_JVMCrash = retargeted.get(JAVA_BYTE, basic_struct_LAYOUT.byteOffset(pathToFlags)); // this crashes
    }
  }
}
```

And the summary printed on stdout does not help much if you're a inexperienced with JVM crashes.

```
# Problematic frame:
# V  [libjvm.dylib+0x9b69d8]  Unsafe_GetByte(JNIEnv_*, _jobject*, _jobject*, long)+0x14c
```

----
MacOS 14.1.2 (Sonoma)
CPU: M1
OpenJDK Runtime Environment (22.0+26) (build 22-ea+26-2112)
Comments
I like that idea !
11-12-2023

That's an interesting idea. But, I think it would be better to have a dedicated system property in that case, rather than relying on assertions. i.e. something like '-Djava.lang.foreign.CHECK=true', similar to have we have `-Xcheck:jni`.
11-12-2023

> every access (that is not to a constant address) would pay for that. Ah that makes sense. Maybe something could be bone here with the _enable assertion_ flag ? The assert check will skipped if it isn't enabled on the CLI.
10-12-2023

If I use this code: MemorySegment ptr = MemorySegment.NULL.reinterpret(16); int x = ptr.getAtIndex(ValueLayout.JAVA_INT, 3); I also see a hard VM crash. The documentation you're referencing seems to be from this snippet: MemorySegment ptr = null; try (Arena arena = Arena.ofConfined()) { MemorySegment z = segment.get(ValueLayout.ADDRESS, ...); // size = 0, scope = always alive ptr = z.reinterpret(16, arena, null); // size = 4, scope = arena.scope() int x = ptr.getAtIndex(ValueLayout.JAVA_INT, 3); // ok } int x = ptr.getAtIndex(ValueLayout.JAVA_INT, 3); // throws IllegalStateException But in that case the ISE is thrown since the scope of 'ptr' has already been closed. An extra check to see that the base memory segment is non NULL would be possible, but at the same time, every access (that is not to a constant address) would pay for that. And, the check would work only for addresses that are exactly '0', so the use seems limited.
07-12-2023

This is not an issue. Note that the code is relying on a restricted method, namely `AddressLayout::withTargetLayout`. This method allows to automatically resize a memory segment that is read from some other memory location, so that the returned segment is immediately usable in a dereference operation. If this method is not used, the returned segment will have size zero, and accessing the segment will result in an IndexOutOfBoundsException. This is the general strategy by which the FFM API enforces safety: if no restricted methods are used, no JVM crashes are possible. But restricted methods, while powerful, allow developers to create situations which might lead to JVM crashes (as in this cases). Please see this section: https://download.java.net/java/early_access/jdk22/docs/api/java.base/java/lang/foreign/MemorySegment.html#wrapping-addresses Also, please note that running a program with restricted methods will result in runtime warnings: ``` WARNING: A restricted method in java.lang.foreign.AddressLayout has been called WARNING: java.lang.foreign.AddressLayout::withTargetLayout has been called by AccessingData in an unnamed module WARNING: Use --enable-native-access=ALL-UNNAMED to avoid a warning for callers in this module WARNING: Restricted methods will be blocked in a future release unless native access is enabled ``` There's also an optional javac warning that can be enabled using the `-Xlint:restricted` flag: ``` $ javac -Xlint:restricted AccessingData.java AccessingData.java:30: warning: [restricted] AddressLayout.withTargetLayout(MemoryLayout) is a restricted method. var retargeted = memorySegment.get(ADDRESS.withTargetLayout(basic_struct_LAYOUT), 0); ^ (Restricted methods are unsafe and, if used incorrectly, might crash the Java runtime or corrupt memory) 1 warning ```
07-12-2023

Hi I understand the restriction and that restricted method could crash the VM, but I believe in this case that `MemorySegment::get` method could avoid that crash by throwing IllegalStateException when the segment address is `0`, as is the case in the incorrect code above. This would typically align the existing behavior of `getAtIndex`, as documented in the javadoc ``` int x = ptr.getAtIndex(ValueLayout.JAVA_INT, 3); // throws IllegalStateException ``` So basically, I believe this is what should happen ``` retargeted.get(JAVA_BYTE, basic_struct_LAYOUT.byteOffset(pathToFlags)) // throws IllegalStateException ```
07-12-2023