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