Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8280283

dead compiler code found during the JDK-8272058 code review



    • Enhancement
    • Status: Open
    • P4
    • Resolution: Unresolved
    • 18
    • tbd
    • hotspot


      During the code review for the following fix:

          JDK-8272058 25 Null pointer dereference defect groups in 4 files

      some dead compiler code was found.

      Details are found here:


      Here's a paste of those comments:

       phedlin 22 hours ago •

      I believe this code is dead, in both MacroAssembler::pd_patch_instruction() and MacroAssembler::target_addr_for_insn(), since the poll read is PIC, we are not patching and should not need to acquire any target to update.

      The associated fix_relocation_after_move() will not invoke any of the above on the poll read since the maybe_cpool_ref() condition is used:

      void poll_Relocation::fix_relocation_after_move(const CodeBuffer* src, CodeBuffer* dest) {
        if (NativeInstruction::maybe_cpool_ref(addr())) {
          address old_addr = old_addr_for(addr(), src, dest);
          MacroAssembler::pd_patch_instruction(addr(), MacroAssembler::target_addr_for_insn(old_addr));

      Should be sufficient to simply remove these cases, possibly you could move an assert into the "unreachable" branch (for "precision").

      } else {
        assert(!NativeInstruction::is_ldrw_to_zr(<addr>), "Unexpected poll read");

      (The code above in fix_relocation_after_move() might also be dead...)

      Chip in @theRealAph ?

      Member Author
      @dcubed-ojdk dcubed-ojdk 20 hours ago

      @phedlin - Thanks for chiming in on this review thread.

      @bulasevich bulasevich 8 hours ago

      I wondered if target_addr_for_insn ever gets the ldrw_to_zr instruction.
      Anyway, I am Ok to move it to target_addr_for_insn_allow_null_ret (target_addr_for_insn_or_null?).
      Thank you.

      @theRealAph theRealAph 7 hours ago

      Since we moved to using per-thread safepoints, I don't think we'll ever see a reloc against a safepoint poll. I've just looked through the AArch64 back end, and as far as I can see the old safepoint code is gone.

      @theRealAph theRealAph 7 hours ago

          @bulasevich - I've made another pass through the code and checked the calls to MacroAssembler::target_addr_for_insn() and I don't see any of those calls in areas related to safepoint polling. From the other POV, I took a look at the aarch64 safepoint polling code and I don't see a way into MacroAssembler::target_addr_for_insn() .

          It is entirely possible that I'm missing something here and would appreciate it if you could sanity check my conclusions here.

      I think you're right. As far as I can see, this code is dead since
      8253180: ZGC: Implementation of JEP 376: ZGC: Concurrent Thread-Stack Processing

      which took out the code that generated safepoint relocs, but not the code here that relocated them.


        Issue Links



              Unassigned Unassigned
              dcubed Daniel Daugherty
              0 Vote for this issue
              4 Start watching this issue