LWJGL / lwjgl3

LWJGL is a Java library that enables cross-platform access to popular native APIs useful in the development of graphics (OpenGL, Vulkan, bgfx), audio (OpenAL, Opus), parallel computing (OpenCL, CUDA) and XR (OpenVR, LibOVR, OpenXR) applications.
https://www.lwjgl.org
BSD 3-Clause "New" or "Revised" License
4.67k stars 628 forks source link

Segmentation fault in glColor3ub on macOS #858

Closed pftbest closed 1 year ago

pftbest commented 1 year ago

Version

3.3.2 (nightly)

Platform

macOS x64, macOS arm64

JDK

OpenJDK Runtime Environment (19.0.2+7) (build 19.0.2+7-44)

Module

OpenGL

Bug description

Passing negative value to glColor3ub causes SIGSEGV in opengl.

Steps to reproduce:

  1. Clone lwjgl demos
  2. Add -XstartOnFirstThread VM flag
  3. Add a call to glColor3ub:

    --- a/src/org/lwjgl/demo/opengl/SimpleDrawElements.java
    +++ b/src/org/lwjgl/demo/opengl/SimpleDrawElements.java
    @@ -137,6 +137,9 @@ public class SimpleDrawElements {
             float aspect = (float)width/height;
             glLoadIdentity();
             glOrtho(-aspect, aspect, -1, 1, -1, 1);
    +
    +            byte color = -128;
    +            glColor3ub(color, color, color);
             glDrawElements(GL_TRIANGLES, 3, GL_UNSIGNED_INT, 0L);
    
             glfwSwapBuffers(window); // swap the color buffers

Reproduced both in rosetta emulated x86_64 jdk and with native aarch64 jdk.

JDK Versions:

OpenJDK 64-Bit Server VM Zulu17.40+19-CA (17.0.6+10-LTS, mixed mode, sharing, tiered, compressed oops, compressed class ptrs, g1 gc, bsd-aarch64)
OpenJDK 64-Bit Server VM (19.0.2+7-44, mixed mode, sharing, tiered, compressed oops, compressed class ptrs, g1 gc, bsd-amd64)

Full crash logs: hs_err_pid37760.log hs_err_pid37929.log

Stacktrace or crash log output

No response

Spasi commented 1 year ago

Hey @pftbest,

Are you saying it doesn't crash with values >= 0? LWJGL does not do anything special here, just passes the values to the native function. Based on the logs, it's crashing inside GLEngine, so might be a driver bug.

pftbest commented 1 year ago

It's working fine for values >= 0 and <= 127. My guess that in native part of lwjgl the jbyte value is sign extended to integer where value of -1 is passed as negative to the macOS glColor3ub which expects 255 instead.

https://github.com/LWJGL/lwjgl3/blob/b44ba7af8030200bee34d7dfb77edf5617589328/modules/lwjgl/opengl/src/generated/c/org_lwjgl_opengl_GL11.c#L387-L391

pftbest commented 1 year ago

@Spasi If you look carefully at the register values in the crash log:

 x0=0x0000000178090000 points into unknown readable memory: 0x0000000000000000 | 00 00 00 00 00 00 00 00
 x1=0x00000000ffffff80 is an unknown value
 x2=0x00000000ffffff80 is an unknown value
 x3=0x00000000ffffff80 is an unknown value
 x4=0x00000001f5e8fcc0: glColor3ub_Exec+0 in /System/Library/Frameworks/OpenGL.framework/Versions/A/Resources/GLEngine.bundle/GLEngine at 0x00000001f5e57000
 x5=0x0 is NULL

You can clearly see that the value of x1, x2 and x3 is set to 0xffffff80 instead of expected 0x80. So the calling convention was broken somewhere up the call chain.

pftbest commented 1 year ago

Found the root cause:

On macOS the Caller is responsible for truncating or sign extending the value before passing it to Callee. See Pass-arguments-to-functions-correctly.

In this case the value must be truncated because jbyte is a signed type (which will be sign-extended to 32 bits as per calling convention) but glColor3ub expects unsigned value of type GLubyte which should be zero-extended instead. See example on godbolt.org.

But native code in lwjgl does not truncate the values properly because the function is declared as if it expects signed arguments:

typedef void (APIENTRY *glColor3ubPROC) (jbyte, jbyte, jbyte);

instead of unsigned:

typedef void (APIENTRY *glColor3ubPROC) (GLubyte, GLubyte, GLubyte);
or
typedef void (APIENTRY *glColor3ubPROC) (uint8_t, uint8_t, uint8_t);
or
typedef void (APIENTRY *glColor3ubPROC) (unsigned char, unsigned char, unsigned char);

So compiler doesn't know that it should convert the values.

Specifying the correct data types in the glColor3ubPROC function declaration should fix the issue.

Spasi commented 1 year ago

Thank you @pftbest for the information above (especially the godbolt repro!), this issue should hopefully be fixed in the next snapshot.

Spasi commented 1 year ago

Hey @pftbest, LWJGL 3.3.2-snapshot+10 is now available with the above fix. Could you please verify that the issue has been resolved?

pftbest commented 1 year ago

Thanks, I'll try it tomorrow.

pftbest commented 1 year ago

@Spasi The issue is resolved now, my demo app runs just file. I also see new AND instructions in the disassembly, so it's all good, thank you.

Screenshot 2023-03-06 at 17 46 17