RFR: 8341471: Reversed field layout caused by unstable sorting [v2]

[Fei Gao]

On Mon, 7 Oct 2024 09:23:37 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> Fei Gao has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
>> 
>>  - Extend the comparison function to all platforms
>>  - Merge branch 'master' into field_layout_fix
>>  - 8341471: [macOS_aarch64] Reversed field layout caused by unstable sorting
>>    
>>    For class Test:
>>    ```
>>        public class Test {
>>            char a000;
>>            char a001;
>>            char a002;
>>            char a003;
>>            char a004;
>>            char a005;
>>            char a006;
>>            char a007;
>>            char a008;
>>            char a009;
>>            char a00a;
>>            char a00b;
>>        }
>>    ```
>>    
>>    We found its field layout on macOS was:
>>    
>>    Layout of class Test
>>      Instance fields:
>>       @0 12/- RESERVED
>>       @12 "a00b" C 2/2 REGULAR
>>       @14 "a001" C 2/2 REGULAR
>>       @16 "a002" C 2/2 REGULAR
>>       @18 "a003" C 2/2 REGULAR
>>       @20 "a004" C 2/2 REGULAR
>>       @22 "a005" C 2/2 REGULAR
>>       @24 "a006" C 2/2 REGULAR
>>       @26 "a007" C 2/2 REGULAR
>>       @28 "a008" C 2/2 REGULAR
>>       @30 "a009" C 2/2 REGULAR
>>       @32 "a00a" C 2/2 REGULAR
>>       @34 "a000" C 2/2 REGULAR
>>    
>>    `a000` was put in the end while `a00b` was put in the beginning.
>>    
>>    Fields get sorted according to size in [1]. `qsort()` on macOS
>>    reverses the order of fields with the same size. We should extend
>>    the comparison function to preserve the order on macOS, as we
>>    did on Windows.
>>    
>>    Tier 1-3 passed on macOS.
>>    
>>    [1] https://github.com/openjdk/jdk/blob/3ee94e040a7395d11a294a6b660d707c97f188f8/src/hotspot/share/classfile/fieldLayoutBuilder.cpp#L102
>
> src/hotspot/share/classfile/fieldLayoutBuilder.hpp line 106:
> 
>> 104: #if defined (_WINDOWS) || defined (__APPLE__)
>> 105:     // qsort() on Windows/macOS reverse the order of fields with the same size
>> 106:     // the extension of the comparison function below preserves this order
> 
> `qsort()` isn't defined to be stable anywhere. I can't see any reason to restrict thus bug fix to Windows and MacOS.

@theRealAph thanks for your review. The new commit removed the macro and applied the extension of the comparison function to all platforms. Tier 1 - 3 passed on both linux-aarch64 and x86 platforms.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21382#discussion_r1791619589