On Sat, 22 Nov 2025 00:17:51 GMT, Sandhya Viswanathan <[email protected]> wrote:
>> Jiangli Zhou has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Change to just create a byte array for 'nonce' without generating random >> data in gcmDecrypt. Suggested by AI. > > src/hotspot/cpu/x86/stubGenerator_x86_64_aes.cpp line 3531: > >> 3529: __ subl(len, 16 * 16); >> 3530: __ cmpl(len, 16 * 16); >> 3531: __ jcc(Assembler::lessEqual, ENC_DEC_DONE); > > I think the fix should instead be to just move the addl to pos before the > MESG_BELOW_32_BLKS, as below: > > + __ addl(pos, 16 * 16); > __ bind(MESG_BELOW_32_BLKS); > __ subl(len, 16 * 16); > - __ addl(pos, 16 * 16); > > This is because on fall through path addl is needed but not while coming from > line 3479 via jcc. For the latter, the addl has already been done on line > 3477. Hmmm, I think you are correct. Examining the entire flow today, I see `ENCRYPT_16_BLKS` doesn't increment `pos`. Thanks for pointing out that! Before my change, add `pos` was done when it fell through to `MESG_BELOW_32_BLKS`. Removed the `cmpl/jcc` change from `MESG_BELOW_32_BLKS` and moved `addl` to above `MESG_BELOW_32_BLKS`, as suggested. I did a bit debugging for the rare failures occurring to the new test case. One of the failures had message with length `1048833`. That would eventually go through `ENCRYPT_16_BLKS` then fall through `MESG_BELOW_32_BLKS`. With the updated fix, 200 runs all pass on AVX512 machines. So the rare failures were also related to this then. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28363#discussion_r2553794067
