RFR: JDK-8157141 & JDK-8166454: Solaris getisax(2) and meminfo(2) cleanup

[Kim Barrett]

> On Sep 29, 2016, at 1:42 PM, Alan Burlison <Alan.Burlison at oracle.com> wrote:
> 
> On 29/09/2016 00:15, Kim Barrett wrote:
> 
>> ------------------------------------------------------------------------------
>> src/os_cpu/solaris_sparc/vm/vm_version_solaris_sparc.cpp
>> 377   uint_t av1 = avs[AV_HW1_IDX];
>> 378   if (av1 & AV_SPARC_MUL32)        features |= hardware_mul32_m;
>> ...
>> 
>> Good to use the symbolic index AV_HW1_IDX. Bad form to assume we have
>> that many entries.  That should be
>> 
>>  if (avn > AV_HW1_IDX) {
>>    uint_t av1 = avs[AV_HW1_IDX];
>>    if (av1 & AV_SPARC_MUL32)        features |= hardware_mul32_m;
>>    ...
>>  }
> 
> There is always at least one entry returned so the condition above will always be true - we will never reduce the number of HW capabilities bits so I believe the test is unnecessary. It's not there in the existing version of the code either. However if you want me to add it I'll do so.

The old code used literal integer sizes and indices.  The new code assumes avn > AV_HW1_IDX.  I don’t see anything that guarantees that to be true (other than, perhaps, the source code for getisax).  If the array was allocated with a size of the larger of AV_HW1_IDX+1 and AV_HW2_IDX+1 then we’d be guaranteed safe to access av[AV_HW{1,2}_IDX].

> 
>>> A JPRT hotspot testset run is clean.
>> 
>> Have you used logging to hand-verify the expected features are being set?
> 
> I've run it under the debugger and single stepped through the code on a T5 and T7. The features are set as expected.

That works too.

Just that JPRT by itself isn’t a sufficient test here.