RFR: 8328404: RISC-V: Fix potential crash in C2_MacroAssembler::arrays_equals [v2]
Gui Cao
gcao at openjdk.org
Sat Mar 23 08:44:53 UTC 2024
> Hi, The current behavior of C2_MacroAssembler::arrays_equals always load longword before comparison.
> When array[0] is aligned to 32-bit (especially after JDK-8139457 which tries to relax alignment
> of array elements), the last longword load will exceed the array limit and may touch the next
> word beyond object layout in heap memory. So this should bear a similar problem as JDK-8328138.
>
> Proposed fix changes this behavior and aligns with handling in C2_MacroAssembler::string_equals,
> which will check the number of remaining array elements before loading the next longword.
> No obvious changes witnessed from the JMH numbers or benchmarks like SPECjbb2015.
>
> Patch also removed the AvoidUnalignedAccesses check in C2_MacroAssembler::string_equals as we
> don't see extra performance gain when setting AvoidUnalignedAccesses to false when testing the
> JMH tests or benchmarks like SPECjbb2015 on three popular RISC-V hardware platforms. We can
> consider adding it back if it turns out to be usefull on future new hardwares.
>
>
> ### Correctness test:
> - [x] Run tier1-3, hotspot:tier4 tests on LicheePi 4A (release)
> - [x] Run tier1-3, hotspot:tier4 tests on SOPHON SG2042 (release)
>
>
> ### JMH test:
>
> #### 1. test/micro/org/openjdk/bench/java/util/ArraysEquals.java
> 1. SiFive unmatched
>
> Before:
> Benchmark Mode Cnt Score Error Units
> ArraysEquals.testByteFalseBeginning avgt 12 37.804 ± 7.292 ns/op
> ArraysEquals.testByteFalseEnd avgt 12 77.972 ± 3.208 ns/op
> ArraysEquals.testByteFalseMid avgt 12 54.427 ± 6.436 ns/op
> ArraysEquals.testByteTrue avgt 12 75.121 ± 5.172 ns/op
> ArraysEquals.testCharFalseBeginning avgt 12 42.486 ± 6.526 ns/op
> ArraysEquals.testCharFalseEnd avgt 12 122.208 ± 2.533 ns/op
> ArraysEquals.testCharFalseMid avgt 12 83.891 ± 3.680 ns/op
> ArraysEquals.testCharTrue avgt 12 122.096 ± 5.519 ns/op
>
> After:
> Benchmark Mode Cnt Score Error Units
> ArraysEquals.testByteFalseBeginning avgt 12 32.638 ± 7.279 ns/op
> ArraysEquals.testByteFalseEnd avgt 12 73.013 ± 8.081 ns/op
> ArraysEquals.testByteFalseMid avgt 12 43.619 ± 6.104 ns/op
> ArraysEquals.testByteTrue avgt 12 83.044 ± 8.207 ns/op
> ArraysEquals.testCharFalseBeginning avgt 12 39.154 ± 5.233 ns/op
> ArraysEquals.testCharFalseEnd avgt 12 122.072 ± 7.784 ns/op
> ArraysEquals.testCharFalseMid avgt 12 67.831 ± 9.218 ns/op
> Ar...
Gui Cao 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 two additional commits since the last revision:
- Merge remote-tracking branch 'upstream/master' into JDK-8328404
- 8328404: RISC-V: Fix potential crash in C2_MacroAssembler::arrays_equals
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/18370/files
- new: https://git.openjdk.org/jdk/pull/18370/files/8824e1c6..baaae2cc
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=18370&range=01
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=18370&range=00-01
Stats: 60167 lines in 2421 files changed: 10903 ins; 7502 del; 41762 mod
Patch: https://git.openjdk.org/jdk/pull/18370.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/18370/head:pull/18370
PR: https://git.openjdk.org/jdk/pull/18370
More information about the hotspot-compiler-dev
mailing list