RFR: 8290705: StringConcat::validate_mem_flow asserts with "unexpected user: StoreI"

Tobias Hartmann thartmann at openjdk.org
Thu Jul 21 11:01:52 UTC 2022


C2's string concatenation optimization (`OptimizeStringConcat`) does not correctly handle side effecting instructions between  StringBuffer Allocate/Initialize and the call to the constructor. In the failing test, see `SideEffectBeforeConstructor::test`, a `result` field is incremented just before the constructor is invoked. The string concatenation optimization still merges the allocation, constructor and `toString` calls and incorrectly re-wires the store to before the concatenation. As a result, passing `null` to the constructor will incorrectly increment the field before throwing a NullPointerException. With a debug build, we hit an assert in `StringConcat::validate_mem_flow` due to the unexpected field store. This is an old bug and an extreme edge case as javac would not generate such code.

The following comment suggests that this case should be covered by `StringConcat::validate_control_flow()`:
https://github.com/openjdk/jdk/blob/3582fd9e93d9733c6fdf1f3848e0a093d44f6865/src/hotspot/share/opto/stringopts.cpp#L834-L838

However, the control flow analysis does not catch this case. I added the missing check.

Thanks,
Tobias

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

Commit messages:
 - 8290705: StringConcat::validate_mem_flow asserts with "unexpected user: StoreI"

Changes: https://git.openjdk.org/jdk/pull/9589/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9589&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8290705
  Stats: 121 lines in 3 files changed: 121 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/9589.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9589/head:pull/9589

PR: https://git.openjdk.org/jdk/pull/9589


More information about the hotspot-compiler-dev mailing list