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

[Severin Gehwolf]

Hi,

Thanks for looking at this!

On Mon, 2020-05-11 at 19:57 +0100, Andrew Hughes wrote:
> 
> On 11/05/2020 19:22, Simon Tooke wrote:
> > That's horrible.  I will try again.  My wife claims I tell anecdotes
> > like that too.
> > 
> > Original:
> > 
> > 1644     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:
> > 
> > 1644     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
> 
> Thanks. That's a lot more readable :-)
> 
> Do we have any idea how much the three later fields are used in the
> wild?

I, for one, have no idea :-(

> In particular, could we not just steal those eight bits from
> 'special_update_version' (whatever that is) and keep the sum the same?

See below.

> This would only apply to versions where update_version is broken
> already, so reinterpretation of a structure of the existing size seems
> safer than increasing the size by eight bits.

Sure. webrev 03 should preserve the size.

> In code,
> 
> unsigned int jdk_version;
> unsigned int update_version : 16;
> unsigned int reserved1 : 16;
> unsigned int reserved2;
> 
> or, if we're wary about losing the name special_update_version, we could

special_update_version seems at least be referenced in code. So we
would have to change those references. Given that, I'd go with reducing
reserved1 as that doesn't seem to be used.

> shrink reserved1:
> 
> unsigned int jdk_version;
> unsigned int update_version : 16;
> unsigned int special_update_version : 8;
> unsigned int reserved1 : 8;
> unsigned int reserved2;

This is what webrev 03 does:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8244548/03/

Does that look OK to you?

Thanks,
Severin