On Tue, 28 May 2024 20:29:38 GMT, Sandhya Viswanathan <sviswanat...@openjdk.org> wrote:
>> No. This is checking for a zero length haystack. The following compare >> checks for needle length longer than haystack, regardless of the value in >> each. The comparison is signed, so a haystack length of 0 with a needle >> length of -1 will pass the following test and assume validity. > > But we have already checked for needle length to be greater than 0 in the > following lines: > __ cmpq(needle_len_p, 0); > __ jg_b(L_nextCheck); OK >> Again, I think we ought to leave this in. Although it executes ~3 >> instructions that may not be necessary in some cases I think it's best to >> perform the check. Once we have a good enough test to check reading past >> the end of the haystack we can change it. > > In this particular case, we are returning -1 (NoMatch), so no need to do > L_checkRangeAndReturn here, we could directly jump to L_returnError. OK. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1617876757 PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1617874637
