RFR: JDK-8277029: JMM GetDiagnosticXXXInfo APIs should verify output array sizes

[Thomas Stuefe]

On Mon, 15 Nov 2021 09:59:44 GMT, David Holmes <dholmes at 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