Skip to content
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

Segmentation fault in glColor3ub on macOS #858

Closed
pftbest opened this issue Feb 15, 2023 · 8 comments
Closed

Segmentation fault in glColor3ub on macOS #858

pftbest opened this issue Feb 15, 2023 · 8 comments

Comments

@pftbest
Copy link

pftbest commented Feb 15, 2023

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
Copy link
Member

Spasi commented Feb 15, 2023

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
Copy link
Author

pftbest commented Feb 15, 2023

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.

JNIEXPORT void JNICALL Java_org_lwjgl_opengl_GL11_glColor3ub(JNIEnv *__env, jclass clazz, jbyte red, jbyte green, jbyte blue) {
glColor3ubPROC glColor3ub = (glColor3ubPROC)tlsGetFunction(24);
UNUSED_PARAM(clazz)
glColor3ub(red, green, blue);
}

pftbest added a commit to pftbest/lwjgl3ify that referenced this issue Feb 15, 2023
glColor3ub is causing SEGFAULT on negative values, use glColor3i instead.
See LWJGL/lwjgl3#858

This call is rarely used so performance impact should be minimal.
eigenraven pushed a commit to GTNewHorizons/lwjgl3ify that referenced this issue Feb 15, 2023
glColor3ub is causing SEGFAULT on negative values, use glColor3i instead.
See LWJGL/lwjgl3#858

This call is rarely used so performance impact should be minimal.
@pftbest
Copy link
Author

pftbest commented Feb 15, 2023

@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
Copy link
Author

pftbest commented Feb 15, 2023

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 Spasi closed this as completed in 9a5f62e Mar 2, 2023
@Spasi
Copy link
Member

Spasi commented Mar 2, 2023

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

@Spasi
Copy link
Member

Spasi commented Mar 4, 2023

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
Copy link
Author

pftbest commented Mar 5, 2023

Thanks, I'll try it tomorrow.

@pftbest
Copy link
Author

pftbest commented Mar 6, 2023

@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

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

2 participants