search

capnproto / capnproto-java

Cap'n Proto in pure Java
Other
391 stars 86 forks source link

Integer overflow when allocating large ByteBuffer in BuilderArena #100

Closed clavin-xlnx closed 3 years ago

clavin-xlnx commented 3 years ago

Thanks for the rapid fixes on the previous issues (#97 and #99). We've encountered another integer overflow situation in Xilinx/RapidWright#176 when the BuilderArena attempts to allocate a new ByteBuffer, in our setup, we get the following stack trace:

Exception in thread "main" java.lang.IndexOutOfBoundsException
    at java.base/java.nio.Buffer.checkIndex(Buffer.java:688)
    at java.base/java.nio.HeapByteBuffer.putInt(HeapByteBuffer.java:408)
    at org.capnproto.WirePointer.setKindAndTarget(WirePointer.java:49)
    at org.capnproto.WireHelpers.allocate(WireHelpers.java:85)
    at org.capnproto.WireHelpers.initStructListPointer(WireHelpers.java:506)
    at org.capnproto.StructList$Factory.initFromPointerBuilder(StructList.java:81)
    at org.capnproto.StructList$Factory.initFromPointerBuilder(StructList.java:1)
    at org.capnproto.StructBuilder._initPointerField(StructBuilder.java:194)
    at com.xilinx.rapidwright.interchange.DeviceResources$Device$Builder.initWires(DeviceResources.java:718)
    at com.xilinx.rapidwright.interchange.DeviceResourcesWriter.writeAllWiresAndNodesToBuilder(DeviceResourcesWriter.java:755)
    at com.xilinx.rapidwright.interchange.DeviceResourcesWriter.writeDeviceResourcesFile(DeviceResourcesWriter.java:269)
    at com.xilinx.rapidwright.interchange.DeviceResourcesExample.main(DeviceResourcesExample.java:29)

BuilderArena.allocate() is called with amount=346319666, however on line 101, this is multiplied by Constants.BYTES_PER_WORD=8, causing it to overflow Integer.MAX_VALUE.

dwrensha commented 3 years ago

Thanks for the report.

We should catch this problem sooner and present a better error to the user.

We could in theory support allocations this large (between 2^31 and 2^32 bytes), but it would be nontrivial, because an individual java.nio.ByteBuffer can store at most 2^31 bytes. We would need to add an extra layer of indirection, and it would only buy us a factor of 2 in supported allocation size, because the capnproto format does not support allocations larger than 2^32 bytes.

Is there a way you can split up your message so that it does not need to contiguously allocate such a big chunk of memory?

dwrensha commented 3 years ago

d310db1e8895cbc4d65d5e7cfef7cdcbfa21edd1 should prevent the overflow and throw a better exception.

clavin-xlnx commented 3 years ago

Thanks for the overflow check, I've confirmed it throws correctly for our situation. We'll try changing how the data is being stored to avoid such a large singular array.

dwrensha commented 3 years ago

Closing, because the overflow has been fixed.