On Wed, 7 May 2025 11:17:19 GMT, Volkan Yazici <vyaz...@openjdk.org> wrote:
>> src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java line 302: >> >>> 300: * <b>WARNING: This method does not perform any bound checks.</b> >>> 301: */ >>> 302: int countPositives(byte[] ba, int off, int len); >> >> Maybe we should change countPositives to do a bounds check and have a >> private method in StringCoding be the `@IntrinsicCandidate`. This would make >> it easier to audit. Right now it's hard to audit because there are usages of >> JLA.countPositives in faraway classes. I'm not suggested we do this as part >> of this PR of course but it would mean the warning comment could be removed. > > Submitted [8356380](https://bugs.openjdk.org/browse/JDK-8356380) to address > this. See also pending PR https://github.com/openjdk/jdk/pull/24777 It documents the expectations of how an intrinsic method must be kept private. An intrinsic is a private "back door" in the VM, and must not be accessed (except perhaps in carefully documented unusual circumstances) outside of its own source file. There should be a non-private "front door" in the same source file which applies all relevant checks before calling the intrinsic. Such checks need to be at least as strong as the built-in JVM range checks, null checks, and type checks that would be performed, if the intrinsic were implemented in 100% pure Java bytecodes. In performance evaluations, such checks may appear to be redundant. But they must not be removed. Instead, whatever other checks are redundant with them (there must be some!) should be brought into the same C2 compilation task as the "front door" checks. This requires making the two sets of checks (if there are two) look similar enough that C2 (or another JIT) can recognize their redundance. Bringing the checks into the same task might require a ForceInline annotation (which is allowed only inside the JDK). Recognizing the checks as redundant might require a C2 bug fix or RFE. (The javadoc for IntrinsicCandidate says stuff like this.) ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24982#discussion_r2087493446
