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

[Thomas Stüfe]

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).

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.

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
> >