Repository navigation
Conversation
|
Before you submit for review:
If you did not complete any of these, then please explain below. |
The native paths passed v1.length() to native code without checking v2, so a shorter v2 could cause out-of-bounds native reads. Reject unequal lengths at the Java/native boundary, matching the Panama implementations.
|
|
||
| import static org.junit.jupiter.api.Assertions.*; | ||
|
|
||
| class InPlaceLengthCheckTest { |
There was a problem hiding this comment.
All these tests should probably be in NativeVectorUtilSupportTest.java instead of creating a new test class. The existing test class already has methods like subtractionRejectsShorterRightHandSide, so these would fit in nicely there.
| return NativeSimdOps.assemble_and_sum_f32(((MemorySegmentVectorFloat) data).get(), dataBase, ((MemorySegmentByteSequence) baseOffsets).get(), baseOffsetsOffset, (long) baseOffsetsLength); | ||
| } | ||
|
|
||
| private static void checkSameLength(VectorFloat<?> v1, VectorFloat<?> v2) { |
There was a problem hiding this comment.
There are other operations in this class which look like they might cause UB if called with unexpected vector sizes, but which themselves have no checks. For example pq_decoded_cosine_similarity. I haven't scrutinized the native code to verify this, but is this something we should be concerned about?
The difference is that those other functions are on the critical path for graph search, so even if we do think checks are warranted there it's probably out of scope for this PR.
|
|
||
| @Override | ||
| public void addInPlace(VectorFloat<?> v1, VectorFloat<?> v2) { | ||
| checkSameLength(v1, v2); |
There was a problem hiding this comment.
The native functions have HWY_RESTRICT on them. Should we also check for seg1.address() != seg2.address()?
Fixes #760.
The native paths passed v1.length() to native code without checking v2, so a shorter v2 could cause out-of-bounds native reads. Reject unequal lengths at the Java/native boundary, matching the Panama implementations.