[8u] RFR(S): 8244548: JDK 8u: sun.misc.Version.jdkUpdateVersion() returns wrong result

[Simon Tooke]

(disclaimer: I am not a reviewer)


I believe it's a "good thing" to shrink the reserved1 bitfield, as I 
suspect the entire struct grows by 4 bytes otherwise.


Original:

644 unsigned int jdk_version; /* Consists of major, minor, micro (n.n.n) 
*/ 1645 /* and build number (xx) */ 1646 unsigned int update_version : 
8; /* Update release version (uu) */ 1647 unsigned int 
special_update_version : 8; /* Special update release version (c)*/ 1648 
unsigned int reserved1 : 16; 1649 unsigned int reserved2; The proposed 
patch: 44 unsigned int jdk_version; /* Consists of major, minor, micro 
(n.n.n) */ 1645 /* and build number (xx) */ 1646 unsigned int 
update_version : 16; /* Update release version (uu) */ 1647 unsigned int 
special_update_version : 8; /* Special update release version (c)*/ 1648 
unsigned int reserved1 : 16; 1649 unsigned int reserved2; Note that the 
original sum of bitfields is 32 bits, so it will fit into a 4 byte int 
(which it is surrounded by) The proposed sum of bitfields is 40 bits, 
which will probably cause a new 4 byte bitfield to be generated under 
the hood.

-Simon


On 2020-05-07 11:09 a.m., Severin Gehwolf wrote:
> Hi,
>
> Please review this OpenJDK 8u fix for an issue where the update version
> as configured via --with-update-version=XXX might overflow in internal
> JDK structures and thus, will get reported wrong. In particular, only 1
> byte is being reserved for the update versions internally. That is, it
> works fine up to a configured update version of 255 (2^8 - 1). We've
> passed that in OpenJDK 8u with the 8u262 cycle currently in EA. Hence,
> we are now seeing this issue.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8244548
> webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8244548/02/
> testing: Linux tier 1
>
> The proposed fix is to extend the update_version field in
> jdk_version_info from 8 to 16 bit and to use 16 bit of the jvm_version
> integer in jvm_version_info. Thus, the new upper bound for update
> version number is 2^16-1 => 65535 which should be sufficient. I don't
> think OpenJDK 8u will live that long ;-)
>
> jvm_version_info.jvm_version currently holds this quadruplet:
>
> Most significant 8 bits => major version, followed by 8 bits => minor
> version, followed by 8 bits => micro version, followed by 8 bits =>
> build version. Note that JVM minor version represents the update
> version as passed in via configure and the micro version is currently
> not used (always 0). See vm_version.cpp lines 100-102 where only major,
> minor and build number are ever been set. Knowing this, we can still
> preserve the same behavior after patch by defining JVM_VERSION_MICRO to
> 0 for any version.
>
> For jdk_version_info the fix is simpler, since the update_version is a
> separate field for which I've extended it to 16 bit. Andrew Brygin
> suggested to reduce the reserved1 field from currently 16 bit to 8 bit
> since we are extending update_version by 8 bits, thus making the whole
> structure grow. I'm not sure reducing reserved1 to 8 bits so as to not
> grow the structure would be necessary, but I'd be happy to do so if
> there is such consensus.
>
> Thoughts?
>
> Thanks,
> Severin
>