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