RFR: 8290705: StringConcat::validate_mem_flow asserts with "unexpected user: StoreI" [v2]

[Tobias Hartmann]

> 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

Tobias Hartmann has updated the pull request incrementally with one additional commit since the last revision:

  Modified debug printing code

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/9589/files
  - new: https://git.openjdk.org/jdk/pull/9589/files/e1de6874..d299e2ea

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=9589&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9589&range=00-01

  Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 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