8288761: SegmentAllocator:allocate(long bytesSize) not throwing IAEx …

…when bytesSize < 0 Reviewed-by: psandoz
openjdk · Jun 21, 2022 · d7b43af · d7b43af
1 parent 834d92d
commit d7b43af
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 16 deletions.
diff --git a/src/java.base/share/classes/java/lang/foreign/MemorySegment.java b/src/java.base/share/classes/java/lang/foreign/MemorySegment.java
@@ -883,9 +883,7 @@ static MemorySegment ofAddress(MemoryAddress address, long bytesSize, MemorySess
         Reflection.ensureNativeAccess(Reflection.getCallerClass(), MemorySegment.class, "ofAddress");
         Objects.requireNonNull(address);
         Objects.requireNonNull(session);
-        if (bytesSize < 0) {
-            throw new IllegalArgumentException("Invalid size : " + bytesSize);
-        }
+        Utils.checkAllocationSizeAndAlign(bytesSize, 1);
         return NativeMemorySegmentImpl.makeNativeSegmentUnchecked(address, bytesSize, session);
     }
 
@@ -957,15 +955,7 @@ static MemorySegment allocateNative(long bytesSize, MemorySession session) {
      */
     static MemorySegment allocateNative(long bytesSize, long alignmentBytes, MemorySession session) {
         Objects.requireNonNull(session);
-        if (bytesSize < 0) {
-            throw new IllegalArgumentException("Invalid allocation size : " + bytesSize);
-        }
-
-        if (alignmentBytes <= 0 ||
-                ((alignmentBytes & (alignmentBytes - 1)) != 0L)) {
-            throw new IllegalArgumentException("Invalid alignment constraint : " + alignmentBytes);
-        }
-
+        Utils.checkAllocationSizeAndAlign(bytesSize, alignmentBytes);
         return NativeMemorySegmentImpl.makeNativeSegment(bytesSize, alignmentBytes, session);
     }
 

diff --git a/src/java.base/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java b/src/java.base/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java
@@ -153,10 +153,7 @@ public final MemorySegment fill(byte value){
 
     @Override
     public MemorySegment allocate(long bytesSize, long bytesAlignment) {
-        if (bytesAlignment <= 0 ||
-                ((bytesAlignment & (bytesAlignment - 1)) != 0L)) {
-            throw new IllegalArgumentException("Invalid alignment constraint : " + bytesAlignment);
-        }
+        Utils.checkAllocationSizeAndAlign(bytesSize, bytesAlignment);
         return asSlice(0, bytesSize);
     }
 

diff --git a/src/java.base/share/classes/jdk/internal/foreign/ArenaAllocator.java b/src/java.base/share/classes/jdk/internal/foreign/ArenaAllocator.java
@@ -71,6 +71,7 @@ private MemorySegment newSegment(long bytesSize, long bytesAlignment) {
 
     @Override
     public MemorySegment allocate(long bytesSize, long bytesAlignment) {
+        Utils.checkAllocationSizeAndAlign(bytesSize, bytesAlignment);
         // try to slice from current segment first...
         MemorySegment slice = trySlice(bytesSize, bytesAlignment);
         if (slice != null) {

diff --git a/src/java.base/share/classes/jdk/internal/foreign/Utils.java b/src/java.base/share/classes/jdk/internal/foreign/Utils.java
@@ -162,4 +162,17 @@ public static void checkElementAlignment(MemoryLayout layout, String msg) {
             throw new IllegalArgumentException(msg);
         }
     }
+
+    public static void checkAllocationSizeAndAlign(long bytesSize, long alignmentBytes) {
+        // size should be >= 0
+        if (bytesSize < 0) {
+            throw new IllegalArgumentException("Invalid allocation size : " + bytesSize);
+        }
+
+        // alignment should be > 0, and power of two
+        if (alignmentBytes <= 0 ||
+                ((alignmentBytes & (alignmentBytes - 1)) != 0L)) {
+            throw new IllegalArgumentException("Invalid alignment constraint : " + alignmentBytes);
+        }
+    }
 }
diff --git a/test/jdk/java/foreign/TestSegmentAllocators.java b/test/jdk/java/foreign/TestSegmentAllocators.java
@@ -139,6 +139,26 @@ public void testBadUnboundedArenaSize() {
         SegmentAllocator.newNativeArena( -1, MemorySession.global());
     }
 
+    @Test(dataProvider = "allocators", expectedExceptions = IllegalArgumentException.class)
+    public void testBadAllocationSize(SegmentAllocator allocator) {
+        allocator.allocate(-1);
+    }
+
+    @Test(dataProvider = "allocators", expectedExceptions = IllegalArgumentException.class)
+    public void testBadAllocationAlignZero(SegmentAllocator allocator) {
+        allocator.allocate(1, 0);
+    }
+
+    @Test(dataProvider = "allocators", expectedExceptions = IllegalArgumentException.class)
+    public void testBadAllocationAlignNeg(SegmentAllocator allocator) {
+        allocator.allocate(1, -1);
+    }
+
+    @Test(dataProvider = "allocators", expectedExceptions = IllegalArgumentException.class)
+    public void testBadAllocationAlignNotPowerTwo(SegmentAllocator allocator) {
+        allocator.allocate(1, 3);
+    }
+
     @Test(dataProvider = "arrayAllocations")
     public <Z> void testArray(AllocationFactory allocationFactory, ValueLayout layout, AllocationFunction<Object, ValueLayout> allocationFunction, ToArrayHelper<Z> arrayHelper) {
         Z arr = arrayHelper.array();
@@ -444,4 +464,13 @@ private MemoryAddress[] wrap(long[] ints) {
             }
         };
     }
+
+    @DataProvider(name = "allocators")
+    static Object[][] allocators() {
+        return new Object[][] {
+                { SegmentAllocator.implicitAllocator() },
+                { SegmentAllocator.newNativeArena(MemorySession.global()) },
+                { SegmentAllocator.prefixAllocator(MemorySegment.allocateNative(10, MemorySession.global())) },
+        };
+    }
 }