On Tue, 15 Apr 2025 15:09:16 GMT, Ferenc Rakoczi <d...@openjdk.org> wrote:
>> @adinn Hi, Andrew, >> I think I addressed all of your comment improvement comments, in most cases >> I just changed them as you suggested. Thanks a lot for the thorough review! > >> @ferakocz >> >> Hi Ferenc, >> >> Sorry, but I still had a few comments to add to the KyberNTTMult routine to >> clarify exactly how the load, compute and store operations relate to the >> original Java source. That's the only remaining code that I felt needed >> further clarification for maintainers. So, after you work through them I can >> approve the PR. > > No problem , it was easy to make the changes. Thanks again! @ferakocz I reran test jtreg:test/jdk/sun/security/provider/acvp/Launcher.java and hit a Java assertion: >> ML-KEM-512 encapsulation 1 STDERR: java.lang.AssertionError at java.base/com.sun.crypto.provider.ML_KEM.twelve2Sixteen(ML_KEM.java:1371) at java.base/com.sun.crypto.provider.ML_KEM.decodePoly(ML_KEM.java:1408) at java.base/com.sun.crypto.provider.ML_KEM.decodeVector(ML_KEM.java:1337) at java.base/com.sun.crypto.provider.ML_KEM.kPkeEncrypt(ML_KEM.java:712) at java.base/com.sun.crypto.provider.ML_KEM.encapsulate(ML_KEM.java:555) at java.base/com.sun.crypto.provider.ML_KEM_Impls$K.implEncapsulate(ML_KEM_Impls.java:134) at java.base/sun.security.provider.NamedKEM$KeyConsumerImpl.engineEncapsulate(NamedKEM.java:124) at java.base/javax.crypto.KEM$Encapsulator.encapsulate(KEM.java:265) at java.base/javax.crypto.KEM$Encapsulator.encapsulate(KEM.java:225) at ML_KEM_Test.encapDecapTest(ML_KEM_Test.java:98) at ML_KEM_Test.run(ML_KEM_Test.java:41) at Launcher.run(Launcher.java:160) at Launcher.main(Launcher.java:122) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104) at java.base/java.lang.reflect.Method.invoke(Method.java:565) at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:335) at java.base/java.lang.Thread.run(Thread.java:1447) JavaTest Message: Test threw exception: java.lang.AssertionError JavaTest Message: shutting down test The offending code is this: private void twelve2Sixteen(byte[] condensed, int index, short[] parsed, int parsedLength) { int i = parsedLength / 64; int remainder = parsedLength - i * 64; if (remainder != 0) { i++; } assert (((remainder != 0) && (remainder != 48)) || <== assert here index + i * 96 > condensed.length); I believe the logic is reversed here i.e. it should be: assert ((remainder == 0) || (remainder == 48)) && index + i * 96 <= condensed.length); Does that sound right? ------------- PR Comment: https://git.openjdk.org/jdk/pull/23663#issuecomment-2806628550