On Thu, 30 Oct 2025 09:12:41 GMT, Andrew Haley <[email protected]> wrote:
> We shouldn't leave such fragile code in once we've noticed it. IMO it's a > false economy to avoid SafeFetch on efficiency grounds. If needs be, there > are ways to make it faster. @theRealAph, I agree - there should be a mechanism to ensure the function can't cause a crash connected to reading outside the code blob. Would it be suitable if this is handled as a separate issue and PR dedicated to resolving it? One aspect I'm still unsure about, when considering the `SafeFetch`-based approach: as far as I'm aware, there is no guarantee that a code outside a code blob can't ever have a NOP+MOVK sequence (though of course chances of that are very low) and, certainly, there is no guarantee that an arbitrary data outside code blob would not match the pattern. As an example, https://github.com/openjdk/jdk/blob/87e5341d78d206fa9e987340861cd5f1c0858891/src/hotspot/share/runtime/frame.inline.hpp#L112 assumes that once the `check` is successful, it can find the CodeBlob object based on the information stored in the perceived post-call NOP sequence. If it is possible for the `check` to happen for a location outside code blob and that would happen to result in a false-positive match, then the retrieved information would be unreliable and might lead to further issues including assumption that an arbitrary instruction sequence or data is interpreted as a `CodeBlob`. There is a special case of potentially having a call instruction right at the end of the code blob. As far as I understand, it is not currently possible for platforms for which continuations are enabled because every call is followed by the post-call NOP sequence. In case it actually might happen in some case, the `SafeFetch` would guarantee there is no crash, however in my understanding the code might still be fragile: if there instead happens to be another `CodeBlob` next to the current one, the `check` will try to interpret the header data as a post-call NOP sequence. There can be a false-positive match - as there is no guarantee the data in the header will never match with the post-call NOP sequence pattern. In that case, similarly to the more generic case, an arbitrary data or code could be interpreted as a `CodeBlob` leading to an unpredictable behaviour. I'd like to solve the issue with fragility of the post-call NOP check, however it does appear to me that only adding `SafeFetch` might not fully resolve the concern: it would prevent faults but wouldn't address the false positive matches. If, however, another guarantee is added that post-call NOP check never looks beyond the code blob of the call site, then `SafeFetch` would not be required. What do you think about this perspective? ------------- PR Comment: https://git.openjdk.org/jdk/pull/26678#issuecomment-3467154836
