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

[Andrew Hughes]

On 12/05/2020 15:14, Severin Gehwolf wrote:
> Hi Andrew,
> 
> Thanks for looking at this.
> 
> On Tue, 2020-05-12 at 14:07 +0100, Andrew Haley wrote:
>> On 5/7/20 4:09 PM, Severin Gehwolf wrote:
>>> 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.
>>
>> This is tricky. JVM_GetVersionInfo is a function exported by
>> libjvm.so, and the version is simply encoded as
>>
>> unsigned int Abstract_VM_Version::jvm_version() {
>>   return ((Abstract_VM_Version::vm_major_version() & 0xFF) << 24) |
>>          ((Abstract_VM_Version::vm_minor_version() & 0xFF) << 16) |
>>          (Abstract_VM_Version::vm_build_number() & 0xFF);
>> }
>>
>> I guess we could argue that this is for JVM-JDK internal use only, and
>> no-one else cares.
>>
>> Or we could encode it in a different way such that at least we have a
>> jvm_version that is monotonically increasing, perhaps by (ab)using the
>> lower 8 bits of the version, the vm_build_number. But I guess I'm
>> being paranoid, and no tools are going to care about minor versions
>> anyway,even if they do call JVM_GetVersionInfo.
> 
> Yes, this is indeed tricky. The trouble is that
> Abstract_VM_Version::vm_build_number() actually holds the configured
> JDK build number, we can't really use it. It's already being used.
> 
> (gdb)
> 99	
> 100	  _vm_major_version = atoi(vm_major_ver);
> 101	  _vm_minor_version = atoi(vm_minor_ver);
> 102	  _vm_build_number  = atoi(vm_build_num);
> 103	
> 104	  os::free(vm_version);
> 105	  _initialized = true;
> 106	}
> 107	
> 108	#if defined(_LP64)
> (gdb) p _vm_build_number 
> $1 = 2
> 
> The only bits which seem unused are bits 8 through 16 of this unsigned
> int. So even if JVM_GetVersionInfo *is* being used somewhere in the
> wild, it *should* continue to work. Hence, the proposal to now also use
> those unused bits for the minor version.
> 
> Thoughts?
> 
> Thanks,
> Severin
> 

The April release of OpenJDK 7u has the values:

Major version: 24, minor version: 5, micro version: 0, build number: 2

when it should be 24.261-b02. So we already have a situation that could
be potentially be misinterpreted as an old JDK version, 7u5 b02, using
the current interpretation of the structure. 8u will see similar issues
if we make no change in this cycle.

As the micro version appears to have always been zero, it seems pretty
safe to just hard-code this value and instead use all 16 bits of the
interpreted value for the micro version. In terms of backwards
compatibility, if a micro version greater than zero is read using the
old method, this will be an indicator that it needs to be
re-interpreted as a 16-bit value.

The current value for 7 is 402980866
(0001 1000 0000 0101 0000 0000 0000 0010)
which thus gets interpreted as above.

The new method encodes the same version numbers as 402720002
(0001 1000 0000 0001 0000 0101 0000 0010)

which can be read back as major 24, minor 261, micro 0 and build 2 using
the updated decoding.

If the old decoding is used, we get major 24, minor 1, micro 5 and build
2. As micro is usually set to zero, updated code should be able to use
this to instead use the new interpretation, while, in the worst case,
stale code will get a unique, if incorrect, version.

As 7u releases are oddly numbered and increment in tens, we won't hit a
case where the second byte (the old micro version) is zero. With 8u,
this will happen where the update version is a multiple of 256 (e.g.
8u512, 8u1792, ...)

Regarding the contract of this API, it is generally used only for
interactions between the JVM and the class library. jvm.h is not
included in the JDK image. Attempts to follow the API when CACAO & JamVM
were still trying to work with OpenJDK suggest it has been unstable even
across update releases [0] [1].

[0] https://icedtea.classpath.org/hg/icedtea7/rev/810d698ffdfb
[1] https://icedtea.classpath.org/hg/icedtea7/rev/139f2a8f085d
-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222