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

Thomas Stüfe thomas.stuefe at gmail.com
Wed Oct 24 08:04:57 UTC 2018

On Wed, Oct 24, 2018 at 8:57 AM David Holmes <david.holmes at oracle.com> wrote:

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.

Posix: " If the input ends before the first matching failure or conversion, EOF shall be returned. "

Seems, with glibc at least, EOF is -1, and it is returned if you never get the chance to at least match one item because of missing input.

3 int main(int argc, char* argv) { 4 void* p; 5 int result = sscanf("", "%p", &p); 6 printf("%d ", result); 7 if (result == EOF) { 8 printf("EOF"); 9 } 10 return 0;

return -1 EOF

Cheers, Thomas

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