On Thu, 1 Oct 2026 14:39:39 GMT, Matthias Baesken <[email protected]> wrote:
>> That sill sounds like a bug that should be caught with an assert. I don't >> see how GCC can make any assumptions about a string returned by >> GetMethodName(). It has to have a SIGNATURE_END_ARGS in it. GCC can't know >> that, but that shouldn't lead to it to thinking strchr can return NULL in >> this case. > > In util.c > https://github.com/openjdk/jdk/blob/46fbea9c4b9b628e43dd8474bde2bd8e984bf857/src/jdk.jdwp.agent/share/native/libjdwp/util.c#L756 > > we null-check after strchr, so being more consistent here and do the > null-check too in production code would make sense. I would argue that too is an uncessary check, and in fact the caller ends up asserting that JVMTI_ERROR_NONE is returned, which implies that it should always be returned. But there is a bug here. methodReturnType() calls JVMTI GetMethodName, which can return an error for various reasons, such as out of memory. If it doesn't return an error, we can trust the method signature returned, but if it does return an error it should be handled properly but is not. Generally speaking in the debug agent, something like an out of memory results in EXIT_ERROR, so someone in the call chain needs to make the error check and do the EXIT_ERROR. But that doesn't mean methodReturnType() should be doing any verification on the signature other than with asserts. So I think consistency here means doing the assert rather than returning an error. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32929#discussion_r4158143277
