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

On x86, assert on unbound assembler Labels used as branch targets

    Details

    • Subcomponent:
    • Resolved In Build:
      b04
    • OS:
      generic

      Backports

        Description

        Originally reported by Jim Roskind & Wade Hennesey of Amazon. Analysis and patch provided by XIn Liu (xxinliu@amazon.com).

        The Label class instances (used to define pseudo-assembly code) can be abused in both the C1 and Interpreter. The most common abuse is being "branched to" but never defined as a location in code via bind(). Adding an assert to catch these caused 106 jtreg/hotspot and 17 jtreg/jdk test failures.

        All were caused by the new assertion in the template interpreter for both x86_32 and x86_64. The label backedge_counter_overflow was not bound when UseLoopCounter was True but UseOnStackReplacement was False.

        A short reproducer is to invoke the debug version of java containing the new assertion like this:

        java -XX:-UseOnStackReplacement

        Here is our jdk8u x64 patch. The bug exists in jdk tip too.

        diff --git a/src/hotspot/src/cpu/x86/vm/templateTable_x86_64.cpp b/src/hotspot/src/cpu/x86/vm/templateTable_x86_64.cpp
        index 301acb44..21814259 100644
        --- a/src/hotspot/src/cpu/x86/vm/templateTable_x86_64.cpp
        +++ b/src/hotspot/src/cpu/x86/vm/templateTable_x86_64.cpp
        @@ -1666,14 +1666,14 @@ void TemplateTable::branch(bool is_jsr, bool is_wide) {
                 const Address mdo_backedge_counter(rbx, in_bytes(MethodData::backedge_counter_offset()) +
                                                    in_bytes(InvocationCounter::counter_offset()));
                 __ increment_mask_and_jump(mdo_backedge_counter, increment, mask,
        - rax, false, Assembler::zero, &backedge_counter_overflow);
        + rax, false, Assembler::zero, UseOnStackReplacement ? &backedge_counter_overflow : &dispatch);
                 __ jmp(dispatch);
               }
               __ bind(no_mdo);
               // Increment backedge counter in MethodCounters*
               __ movptr(rcx, Address(rcx, Method::method_counters_offset()));
               __ increment_mask_and_jump(Address(rcx, be_offset), increment, mask,
        - rax, false, Assembler::zero, &backedge_counter_overflow);
        + rax, false, Assembler::zero, UseOnStackReplacement ? &backedge_counter_overflow : &dispatch);
             } else {
               // increment counter
               __ movptr(rcx, Address(rcx, Method::method_counters_offset()));
        diff --git a/src/hotspot/src/share/vm/asm/assembler.hpp b/src/hotspot/src/share/vm/asm/assembler.hpp
        index ec8ec5eb..d117e073 100644
        --- a/src/hotspot/src/share/vm/asm/assembler.hpp
        +++ b/src/hotspot/src/share/vm/asm/assembler.hpp
        @@ -169,6 +169,10 @@ class Label VALUE_OBJ_CLASS_SPEC {
           Label() {
             init();
           }
        +
        + ~Label() {
        + assert(is_bound() || is_unused(), "Label was never bound to a location, but it was used as a jmp target");
        + }
         };

        // A union type for code which has to assemble both constant and

          Attachments

            Issue Links

              Activity

                People

                • Assignee:
                  phh Paul Hohensee
                  Reporter:
                  phh Paul Hohensee
                • Votes:
                  0 Vote for this issue
                  Watchers:
                  3 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved: