RFR: 8336999: Verification for resource area allocated data structures in C2 [v2]

Tobias Hartmann thartmann at openjdk.org
Thu Jul 25 09:13:07 UTC 2024


> Similar to [GrowableArrayNestingCheck](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/utilities/growableArray.cpp#L60), we should implement a check for C2's resource allocated data structures that verifies that reallocation happens under the same `ResourceMark` as the original allocation. Otherwise, use-after-free bugs like [JDK-8336095](https://bugs.openjdk.org/browse/JDK-8336095) will lead to memory corruption.
> 
> This change adds a [ReallocMark](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/memory/allocation.cpp#L233) to all resource allocated data structures used by C2. I slightly modified it such that it checks the arena and skips verification if the data is not allocated in the resource arena. I also modified the grow methods such that we perform verification even if no reallocation is required. In addition, I changed a few `Unique_Node_List` allocations in vector.cpp from `comp_arena` to resource area allocations because they only have a short lifetime.
> 
> While testing, I hit the verification code from:
> 
> V  [libjvm.so+0x5c1ceb]  ReallocMark::check(Arena*)+0x7b  (allocation.cpp:244)
> V  [libjvm.so+0x6df2da]  Block_Array::grow(unsigned int)+0x1a  (block.cpp:43)
> V  [libjvm.so+0xb88679]  PhaseCFG::do_DFS(Tarjan*, unsigned int)+0x159  (block.hpp:72)
> V  [libjvm.so+0xb88b6b]  PhaseCFG::build_dominator_tree()+0xab  (domgraph.cpp:74)
> V  [libjvm.so+0xd75791]  PhaseCFG::do_global_code_motion()+0x11  (gcm.cpp:1635)
> V  [libjvm.so+0x9f4fd4]  Compile::Code_Gen()+0x2a4  (compile.cpp:2949)
> V  [libjvm.so+0x9f5f16]  Compile::Compile(ciEnv*, TypeFunc const* (*)(), unsigned char*, char const*, int, bool, bool, DirectiveSet*)+0xba6  (compile.cpp:991)
> 
> 
> It's a false positive because the code in `PhaseCFG::build_dominator_tree` pre-grows `PhaseCFG::_blocks` to prevent reallocation before entering the scope of a nested ResourceMark. I think that's bad practice and should be avoided. I changed the code to allocate `_blocks` in a separate arena and removed the pre-growing.
> 
> This detects [JDK-8336095](https://bugs.openjdk.org/browse/JDK-8336095) right away, even with `java -Xcomp -version`.
> 
> We should revisit the footprint impact of arena allocations in C2 with [JDK-8337015](https://bugs.openjdk.org/browse/JDK-8337015).
> 
> Thanks,
> Tobias

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

  Update src/hotspot/share/opto/block.cpp
  
  Co-authored-by: Christian Hagedorn <christian.hagedorn at oracle.com>

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/20311/files
  - new: https://git.openjdk.org/jdk/pull/20311/files/b0f839b8..391dd920

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

  Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/20311.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/20311/head:pull/20311

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


More information about the hotspot-dev mailing list