fix(core): invalid size calculation in struct array malloc methods

LWJGL · Oct 25, 2017 · c7c9434 · c7c9434
1 parent 3ba1080
commit c7c9434
Show file tree

Hide file tree

Showing 10 changed files with 46 additions and 25 deletions.
diff --git a/doc/notes/3.1.3.md b/doc/notes/3.1.3.md
@@ -77,4 +77,9 @@ This build includes the following changes:
 - glfw: Signature of `glfwGetError` has changed.
 - jemalloc: Removed `JEmacros` and moved its functionality to `JEmalloc`.
 - jemalloc: `ChunkHooks` has been replaced by `ExtentHooks`.
-- OpenCL: Removed two confusing `clCompileProgram` overloads.
+- OpenCL: Removed two confusing `clCompileProgram` overloads.
+
+#### Known Issues
+
+- The `<StructType>.malloc(capacity)` methods allocate memory with invalid size.
+    * Workaround: `<StructType>.calloc(capacity)` or `<StructType>.create(nmemAlloc(capacity * <StructType>.SIZEOF))`
diff --git a/doc/notes/3.1.4.md b/doc/notes/3.1.4.md
@@ -22,4 +22,5 @@ This build includes the following changes:
 #### Fixes
 
 - EGL: Fixed nullability of `eglMakeCurrent` arguments.
-- Fixed native library resource discovery when running LWJGL as JPMS modules.
+- Fixed native library resource discovery when running LWJGL as JPMS modules.
+- Fixed invalid size calculation in `<StructType>.malloc(capacity)` methods.
diff --git a/doc/notes/full.md b/doc/notes/full.md
@@ -79,6 +79,11 @@ This build includes the following changes:
 - jemalloc: `ChunkHooks` has been replaced by `ExtentHooks`.
 - OpenCL: Removed two confusing `clCompileProgram` overloads.
 
+#### Known Issues
+
+- The `<StructType>.malloc(capacity)` methods allocate memory with invalid size.
+    * Workaround: `<StructType>.calloc(capacity)` or `<StructType>.create(nmemAlloc(capacity * <StructType>.SIZEOF))`
+
 ### 3.1.2
 
 _Released 2017 May 15_

diff --git a/doc/notes/latest.md b/doc/notes/latest.md
@@ -77,4 +77,9 @@ This build includes the following changes:
 - glfw: Signature of `glfwGetError` has changed.
 - jemalloc: Removed `JEmacros` and moved its functionality to `JEmalloc`.
 - jemalloc: `ChunkHooks` has been replaced by `ExtentHooks`.
-- OpenCL: Removed two confusing `clCompileProgram` overloads.
+- OpenCL: Removed two confusing `clCompileProgram` overloads.
+
+#### Known Issues
+
+- The `<StructType>.malloc(capacity)` methods allocate memory with invalid size.
+    * Workaround: `<StructType>.calloc(capacity)` or `<StructType>.create(nmemAlloc(capacity * <StructType>.SIZEOF))`
diff --git a/modules/core/src/main/java/org/lwjgl/system/Struct.java b/modules/core/src/main/java/org/lwjgl/system/Struct.java
@@ -55,14 +55,18 @@ protected static ByteBuffer checkContainer(ByteBuffer container, int sizeof) {
         return container;
     }
 
+    private static long getBytes(int elements, int elementSize) {
+        return Integer.toUnsignedLong(elements) * elementSize;
+    }
+
     protected static long __malloc(int elements, int elementSize) {
-        long bytes = apiGetBytes(elements, elementSize);
+        long bytes = getBytes(elements, elementSize);
         apiCheckAllocation(elements, bytes, BITS64 ? Long.MAX_VALUE : 0xFFFFFFFFL);
         return nmemAlloc(bytes);
     }
 
     protected static ByteBuffer __create(int elements, int elementSize) {
-        apiCheckAllocation(elements, apiGetBytes(elements, elementSize), 0x7FFFFFFFL);
+        apiCheckAllocation(elements, getBytes(elements, elementSize), 0x7FFFFFFFL);
         return BufferUtils.createByteBuffer(elements * elementSize);
     }
 

diff --git a/modules/core/src/test/java/org/lwjgl/opencl/CLTest.java b/modules/core/src/test/java/org/lwjgl/opencl/CLTest.java
@@ -20,7 +20,7 @@
 import static org.lwjgl.system.Pointer.*;
 import static org.testng.Assert.*;
 
-@Test
+@Test(singleThreaded = true)
 public class CLTest {
 
     private static CLContextCallback CONTEXT_CALLBACK;
@@ -112,15 +112,15 @@ public void testContext() {
 
             long context = clCreateContext(ctxProps, device, CONTEXT_CALLBACK, NULL, errcode_ret);
             checkCLError(errcode_ret);
-            assertNotNull(context);
+            assertNotEquals(context, NULL);
 
             long queue = clCreateCommandQueue(context, device, 0, errcode_ret);
             checkCLError(errcode_ret);
-            assertNotNull(queue);
+            assertNotEquals(queue, NULL);
 
             long buffer = clCreateBuffer(context, CL_MEM_READ_ONLY, 128, errcode_ret);
             checkCLError(errcode_ret);
-            assertNotNull(buffer);
+            assertNotEquals(buffer, NULL);
 
             checkCLError(clReleaseCommandQueue(queue));
             checkCLError(clReleaseMemObject(buffer));
@@ -197,7 +197,7 @@ public void testNativeKernel() {
             );
 
             long e = eventOut.get(0);
-            assertNotNull(e);
+            assertNotEquals(e, NULL);
 
             CountDownLatch  latch = new CountDownLatch(1);
             CLEventCallback eventCallback;
@@ -230,7 +230,7 @@ public void testMemObjectDestructor() {
 
             long buffer = clCreateBuffer(context, CL_MEM_READ_ONLY, 128, errcode_ret);
             checkCLError(errcode_ret);
-            assertNotNull(buffer);
+            assertNotEquals(buffer, NULL);
 
             CountDownLatch eventLatch = new CountDownLatch(1);
 
@@ -271,11 +271,11 @@ public void testEventCallback() {
 
             long context = clCreateContext(ctxProps, device, CONTEXT_CALLBACK, NULL, errcode_ret);
             checkCLError(errcode_ret);
-            assertNotNull(context);
+            assertNotEquals(context, NULL);
 
             long e = clCreateUserEvent(context, errcode_ret);
             checkCLError(errcode_ret);
-            assertNotNull(e);
+            assertNotEquals(e, NULL);
 
             CountDownLatch eventLatch = new CountDownLatch(1);
 
@@ -284,9 +284,10 @@ public void testEventCallback() {
                 clSetEventCallback(e, CL_COMPLETE, eventCallback = CLEventCallback.create((event, event_command_exec_status, user_data) -> {
                     assertEquals(event, e);
                     assertEquals(event_command_exec_status, CL_COMPLETE);
+                    assertEquals(user_data, 0xDEADBEEF);
 
                     eventLatch.countDown();
-                }), NULL)
+                }), 0xDEADBEEF)
             );
 
             checkCLError(clSetUserEventStatus(e, CL_COMPLETE));
@@ -311,11 +312,11 @@ public void testSubBuffer() {
 
             long context = clCreateContext(ctxProps, device, CONTEXT_CALLBACK, NULL, errcode_ret);
             checkCLError(errcode_ret);
-            assertNotNull(context);
+            assertNotEquals(context, NULL);
 
             long buffer = clCreateBuffer(context, CL_MEM_READ_ONLY, 128, errcode_ret);
             checkCLError(errcode_ret);
-            assertNotNull(buffer);
+            assertNotEquals(buffer, NULL);
 
             long subbuffer;
             try (CLBufferRegion bufferRegion = CLBufferRegion.malloc()) {
@@ -325,7 +326,7 @@ public void testSubBuffer() {
                 subbuffer = nclCreateSubBuffer(buffer, CL_MEM_READ_ONLY, CL_BUFFER_CREATE_TYPE_REGION, bufferRegion.address(), memAddress(errcode_ret));
                 checkCLError(errcode_ret);
             }
-            assertNotNull(subbuffer);
+            assertNotEquals(subbuffer, NULL);
 
             checkCLError(clReleaseMemObject(buffer));
             checkCLError(clReleaseMemObject(subbuffer));

diff --git a/modules/core/src/test/java/org/lwjgl/system/BufferTest.java b/modules/core/src/test/java/org/lwjgl/system/BufferTest.java
@@ -18,12 +18,12 @@
 public class BufferTest {
 
     @Test(expectedExceptions = IllegalArgumentException.class)
-    public static void testLargeBufferNIO() {
+    public void testLargeBufferNIO() {
         // ByteBuffer.allocateDirect supports up to Integer.MAX_VALUE bytes
         BufferUtils.createShortBuffer(0x3FFFFFFF + 1);
     }
 
-    public static void testLargeBuffer() {
+    public void testLargeBuffer() {
         ShortBuffer buffer = memCallocShort(0x3FFFFFFF + 1);
         if (buffer == null) {
             throw new SkipException("Large buffer allocation failed."); // 32-bit JVM
@@ -45,7 +45,7 @@ private static long fillBuffer(JNINativeMethod.Buffer buffer) {
     }
 
     public void testStructBufferIterable() {
-        JNINativeMethod.Buffer buffer = JNINativeMethod.calloc(10);
+        JNINativeMethod.Buffer buffer = JNINativeMethod.malloc(10);
 
         long expected = fillBuffer(buffer);
         long actual   = 0;
@@ -59,7 +59,7 @@ public void testStructBufferIterable() {
     }
 
     public void testStructBufferStream() {
-        JNINativeMethod.Buffer buffer = JNINativeMethod.calloc(10);
+        JNINativeMethod.Buffer buffer = JNINativeMethod.malloc(10);
 
         long expected = fillBuffer(buffer);
         long actual = buffer
@@ -73,7 +73,7 @@ public void testStructBufferStream() {
     }
 
     public void testStructBufferParallelStream() {
-        JNINativeMethod.Buffer buffer = JNINativeMethod.calloc(10 * 1024);
+        JNINativeMethod.Buffer buffer = JNINativeMethod.malloc(10 * 1024);
 
         long expected = fillBuffer(buffer);
         long actual = buffer

diff --git a/modules/templates/src/main/kotlin/org/lwjgl/opencl/CLTypes.kt b/modules/templates/src/main/kotlin/org/lwjgl/opencl/CLTypes.kt
@@ -321,7 +321,7 @@ val cl_event_callback = "cl_event_callback".callback(
     OPENCL_PACKAGE, void, "CLEventCallback",
     """
     Will be called when the execution status of the command associated with {@code event} changes to an execution status equal or past the status specified by
-    {@code command_exec_status}."
+    {@code command_exec_status}.
     """,
     cl_event.IN("event", "the event"),
     cl_int.IN(

diff --git a/modules/templates/src/main/kotlin/org/lwjgl/opengl/templates/EXT_polygon_offset_clamp.kt b/modules/templates/src/main/kotlin/org/lwjgl/opengl/templates/EXT_polygon_offset_clamp.kt
@@ -35,7 +35,7 @@ val EXT_polygon_offset_clamp = "EXTPolygonOffsetClamp".nativeClassGL("EXT_polygo
         {@code factor} scales the maximum depth slope of the polygon, and {@code units} scales an implementation-dependent constant that relates to the usable
         resolution of the depth buffer. The resulting values are summed to produce the polygon offset value, which may then be clamped to a minimum or maximum
         value specified by {@code clamp}. The values {@code factor}, {@code units}, and {@code clamp} may each be positive, negative, or zero. Calling the
-        command #PolygonOffset() is equivalent to calling the command PolygonOffsetClampEXT with {@code clamp} equal to zero."
+        command #PolygonOffset() is equivalent to calling the command PolygonOffsetClampEXT with {@code clamp} equal to zero.
         """,
 
         GLfloat.IN("factor", "scales the maximum depth slope of the polygon"),

diff --git a/modules/templates/src/main/kotlin/org/lwjgl/opengles/templates/EXT_polygon_offset_clamp.kt b/modules/templates/src/main/kotlin/org/lwjgl/opengles/templates/EXT_polygon_offset_clamp.kt
@@ -33,7 +33,7 @@ val EXT_polygon_offset_clamp = "EXTPolygonOffsetClamp".nativeClassGLES("EXT_poly
         {@code factor} scales the maximum depth slope of the polygon, and {@code units} scales an implementation-dependent constant that relates to the usable
         resolution of the depth buffer. The resulting values are summed to produce the polygon offset value, which may then be clamped to a minimum or maximum
         value specified by {@code clamp}. The values {@code factor}, {@code units}, and {@code clamp} may each be positive, negative, or zero. Calling the
-        command #PolygonOffset() is equivalent to calling the command PolygonOffsetClampEXT with {@code clamp} equal to zero."
+        command #PolygonOffset() is equivalent to calling the command PolygonOffsetClampEXT with {@code clamp} equal to zero.
         """,
 
         GLfloat.IN("factor", "scales the maximum depth slope of the polygon"),