On Mon, 15 Nov 2021 09:59:44 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> jmm_GetDiagnosticCommandArgumentsInfo and jmm_GetDiagnosticCommandInfo are >> used to query the hotspot about diagnostic commands. They provide output >> arrays for the information: >> >> >> void jmm_GetDiagnosticCommandArgumentsInfo(JNIEnv *env, >> jstring command, dcmdArgInfo* infoArray) >> >> >> but array size is implicitly assumed to be known to both caller and callee. >> Caller and callee negotiate those sizes in prior steps, but things can go >> wrong. E.g. I recently hunted a bug where `DCmd::number_arguments()` was off >> - did not reflect the real number of its jcmd parameters - which led to a >> hidden memory overwriter. >> >> Thankfully, JDK-8264565 rewrote the dcmd framework to deal with this >> particular issue (The VM I analyzed was older). Still, it would be good if >> we had additional safety measures here. >> >> ------------- >> >> Testing: >> - manual tests with artificially induced error in one dcmd for debug, release >> - GHAs (which include tier1 serviceability jcmd tests which use JMX and >> exercise these APIs) > > src/hotspot/share/services/management.cpp line 1968: > >> 1966: >> 1967: JVM_ENTRY(void, jmm_GetDiagnosticCommandInfo(JNIEnv *env, jobjectArray >> cmds, >> 1968: dcmdInfo* infoArray, jint count)) > > I do not see the point of the change in this case. This doesn't check for a > mismatch between an "external" and "internal" value but between two external > values. If you don't trust the caller to size infoArray correctly then how > can you trust them to pass in the right "count" ? Well, it warns in case of programming errors. I agree, that programming error is very unlikely since the caller has all the information he needs. And he could pass in the wrong count. Idk. Mostly I changed this interface to follow the established pattern of always passing in an explicit output buffer size. And to keep it symmetric to``jmm_GetDiagnosticCommandArgumentsInfo` and `jmm_GetGCExtAttributeInfo`. If you object, I'll remove this part. ------------- PR: https://git.openjdk.java.net/jdk/pull/6363
