RFR(XS): 8212883: Setting a double manageable flag with jcmd/jinfo crashes the JVM

David Holmes david.holmes at oracle.com
Wed Oct 24 06:56:51 UTC 2018


Hi Thomas,

On 24/10/2018 4:32 PM, Thomas Stüfe wrote:
> Hi,
> 
> I am really not sure about the implicit boolean comparison of the
> sscanf return value (not only for this but the other flag variants
> too).

It's checking for 0 or 1 matches. But yes hotspot style says it should 
be an explicit check against 0 or 1.

> sscanf may return 0-n (for 0-n matched items) and EOF in case of an
> error. Is EOF always 0? Otherwise, to be sure, I would compare the
> return value with 1 since we expect 1 item to match.

That's probably technically safer, though for sscanf I don't see how you 
can return EOF unless the arg is an empty string - even then that may 
simply constitute 0 matches.

Cheers,
David

> Just my 5c.
> 
> Cheers, Thomas
> 
> 
> 
> On Wed, Oct 24, 2018 at 1:16 AM David Holmes <david.holmes at oracle.com> wrote:
>>
>> Hi Tony,
>>
>> On 24/10/2018 8:13 AM, Tony Printezis wrote:
>>> Webrev here:
>>>
>>> http://cr.openjdk.java.net/~tonyp/8212883/webrev.0/
>>>
>>> Currently, HotSpot doesn’t actually have any double manageable flags, which
>>> is why I think no-one has hit this before. I recently added a couple to our
>>> own builds and I noticed that setting them is not handled correctly in the VM.
>>> The fix is pretty trivial (mostly cut-and-paste from what the code does for
>>> the other types).
>>
>> I agree the fix is pretty obvious and straight-forward.
>>
>>> I tested it by introducing a dummy double manageable flag and I can set it
>>> with jinfo/jcmd and jconsole (these cover all the various paths in the
>>> changes). Is it worth expanding the
>>> serviceability/attach/AttachSetGetFlag.java test to also get/set a double
>>> flag (I’d need to introduce a dummy double manageable flag to do that
>>> though)?
>>
>> I hate to see new code untested ... but then it seems we don't have
>> tests for all the existing types of flags anyway.
>>
>> Reviewed.
>>
>> Thanks,
>> David
>>
>>> Regards,
>>>
>>> Tony
>>>
>>>
>>> —————
>>> Tony Printezis | @TonyPrintezis | tprintezis at twitter.com
>>>


More information about the hotspot-runtime-dev mailing list