Skip to content

Bus Error on CMPXCHG in Scala 3 new lazy vals #5863

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
szymon-rd opened this issue Jan 31, 2023 · 3 comments
Closed

Bus Error on CMPXCHG in Scala 3 new lazy vals #5863

szymon-rd opened this issue Jan 31, 2023 · 3 comments
Assignees

Comments

@szymon-rd
Copy link

szymon-rd commented Jan 31, 2023

Describe the issue
The problem is occurring in the current implementation of LazyVals in Scala 3.3.0-RC2. I managed to minimize it into a snippet of Java code:

import sun.misc.Unsafe;
import java.lang.reflect.Field;

class Evaluating {
}

class FoooBar {
   volatile static Object foo;
}

class FoooBarMain {
   public static void main(String... args) throws Exception { 
      Field f = Unsafe.class.getDeclaredField("theUnsafe");
      f.setAccessible(true);
      Unsafe unsafe = (Unsafe) f.get(null);
      
      Evaluating evaluating = new Evaluating();
      long offset = unsafe.staticFieldOffset(FoooBar.class.getDeclaredField("foo"));
      System.out.println(unsafe.compareAndSwapObject(FoooBar.class, offset, null, evaluating));
      System.out.println(FoooBar.foo);
    }
}

After running it, a segfault occurs. When run with GDB, it shows a Bus error. I investigated it a bit, and I found that the failing instruction is:

lock cmpxchg %esi,0x1b71e0(%r14)

Stepping after this instruction results in Thread 2 received signal SIGBUS, Bus error.
I am providing a minimized example that generates a slightly different assembly. But it is compiled from a similar bytecode and results in the same error. In our case, I investigated it a bit further and found that the address of the destination of cmpxchg is wrong and does not correspond to the storage we want to operate on.

It only fails for static fields - therefore, only objects in Scala suffer from this. Classes generate non-static fields that use Unsafe functions for non-static fields, and all addresses are correct.

Steps to reproduce the issue
Please include both build steps as well as run steps

  1. Create a java class like the above one and call it ReproJava.java,
  2. Compile it: javac ReproJava.java -d javaout,
  3. Generate the native image: native-image -g -cp javaout FoooBarMain,
  4. Execute the native image: ./foobarmain

Describe GraalVM and your environment:

  • java --version:
java 11.0.18 2023-01-17 LTS
Java(TM) SE Runtime Environment GraalVM EE 22.3.1 (build 11.0.18+9-LTS-jvmci-22.3-b11)
Java HotSpot(TM) 64-Bit Server VM GraalVM EE 22.3.1 (build 11.0.18+9-LTS-jvmci-22.3-b11, mixed mode, sharing)
  • native-image --version:
GraalVM 22.3.1 Java 11 EE (Java Version 11.0.18+9-LTS-jvmci-22.3-b11)
  • OS: macOS 11 with Intel. Reproduced also on macOS with ARM and Ubuntu.
szymon-rd added a commit to scala/scala3 that referenced this issue Feb 6, 2023
The new lazy vals implementation was not working in graalvm's native
image. The generated holder for the lazy value was static
post-compilation. It caused the generated code to perform CAS operate on
bad addresses and throw Bus Error.
Issue on oracle/graalvm: oracle/graal#5863
This PR removes the static modifier from the lazy val holder and makes
it accessible via `MODULE$` instead. It fixes the CAS on GraalVM and
does not have a performance impact, at least judging by the benchmark
I've added to this PR.
@oubidar-Abderrahim oubidar-Abderrahim self-assigned this Feb 7, 2023
@oubidar-Abderrahim
Copy link
Member

Thank you for reporting this, we'll take a look into it shortly.

@oubidar-Abderrahim
Copy link
Member

Tracked internally on GR 44061

@christianwimmer
Copy link

christianwimmer commented Feb 8, 2023

The example code is wrong, and happens to just accidentally work when running on the HotSpot VM: when accessing a static field with Unsafe, it is required that the accessed object is coming from staticFieldBase.

So the correct code is

      Evaluating evaluating = new Evaluating();
      long offset = unsafe.staticFieldOffset(FoooBar.class.getDeclaredField("foo"));
      System.out.println(unsafe.compareAndSwapObject(unsafe.staticFieldBase(FoooBar.class), offset, null, evaluating));
      System.out.println(FoooBar.foo);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants