RFR: 8373059: Test sun/security/provider/acvp/ML_DSA_Intrinsic_Test.java should pass on Aarch64
Ferenc Rakoczi
duke at openjdk.org
Fri Dec 12 13:29:59 UTC 2025
On Fri, 12 Dec 2025 12:30:23 GMT, Weijun Wang <weijun at openjdk.org> wrote:
>> …hould pass on Aarch64
>>
>> The test used to fail because it had checked a stronger equivalence of the results of the Java method and its intrinsified version.
>> Other then fixing that, I did some formatting and corrected a comment.
>
> test/jdk/sun/security/provider/pqc/ML_DSA_Intrinsic_Test.java line 49:
>
>> 47: // To run manually:
>> 48: // java --add-opens java.base/sun.security.provider=ALL-UNNAMED
>> 49: // --add-exports java.base/sun.security.provider=ALL-UNNAMED
>
> Please indent one space to align with lines below.
Done.
> test/jdk/sun/security/provider/pqc/ML_DSA_Intrinsic_Test.java line 51:
>
>> 49: // --add-exports java.base/sun.security.provider=ALL-UNNAMED
>> 50: // -XX:+UnlockDiagnosticVMOptions -XX:+UseDilithiumIntrinsics
>> 51: // test/jdk/sun/security/provider/acvp/ML_DSA_Intrinsic_Test.java
>
> You've modified the test path above.
You are right. I have changed it.
> test/jdk/sun/security/provider/pqc/ML_DSA_Intrinsic_Test.java line 140:
>
>> 138: for (int j = 0; j < ML_DSA_N; j++) {
>> 139: coeffs1[j] = rnd.nextInt(2 * ML_DSA_Q) - ML_DSA_Q;
>> 140: coeffs2[j] = rnd.nextInt(2 * ML_DSA_Q) - ML_DSA_Q;
>
> Why are both so small? Maybe only one is enough?
This is purely for performance reasons. The test works for any case that satisfies the montMul() preconditions, but since this particular method that is under test here, these stricter conditions are always satisfied. If we allow a bigger range, the probability that the whole vector fails the equality test is higher, so the for loop will be executed more frequently and that takes time. I added a comment to the code.
> test/jdk/sun/security/provider/pqc/ML_DSA_Intrinsic_Test.java line 147:
>
>> 145:
>> 146: if (!Arrays.equals(prod1, prod2)) {
>> 147: boolean modQequal = true;
>
> You can copy the "the result is greater than -MONT_Q and less than MONT_Q" comment here.
I added some comments.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28722#discussion_r2614213946
PR Review Comment: https://git.openjdk.org/jdk/pull/28722#discussion_r2614214105
PR Review Comment: https://git.openjdk.org/jdk/pull/28722#discussion_r2614214403
PR Review Comment: https://git.openjdk.org/jdk/pull/28722#discussion_r2614214261
More information about the security-dev
mailing list