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

Missing bounds checks for some String intrinsics

    Details

    • Type: Bug
    • Status: In Progress
    • Priority: P2
    • Resolution: Unresolved
    • Affects Version/s: 9
    • Fix Version/s: 9
    • Component/s: hotspot
    • Environment:

      -XX:-CompactStrings

    • Subcomponent:
    • Understanding:
      Fix Understood

      Description

      # SIGSEGV (0xb) at pc=0x0000007fa4726ec0, pid=3211, tid=3218
      #
      # JRE version: Java(TM) SE Runtime Environment (9.0) (build 9-internal+0-2016-05-26-211740.amurillo.jdk9-hs-2016-05-26-snapshot)
      # Java VM: Java HotSpot(TM) 64-Bit Server VM (9-internal+0-2016-05-26-211740.amurillo.jdk9-hs-2016-05-26-snapshot, compiled mode, tiered, compressed oops, g1 gc, linux-aarch64)
      # Problematic frame:
      # V [libjvm.so+0x3d2ec0] CollectedHeap::fill_with_objects(HeapWord*, unsigned long, bool)+0xa8
      #
      # Core dump will be written. Default location: Core dumps may be processed with "/usr/share/apport/apport %p %s %c %P" (or dumping to /export/local/aurora/sandbox/results/ResultDir/stressHierarchy009/core.3211)


      --------------- S U M M A R Y ------------

      Command Line: -XX:MaxMetaspaceSize=450m -Xss10m -Xbootclasspath/a:/export/local/aurora/CommonData/vm/lib/wb.jar -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -Xcomp -XX:+CreateCoredumpOnCrash -XX:+IgnoreUnrecognizedVMOptions -XX:ReservedCodeCacheSize=256M -Xcomp metaspace.stressHierarchy.common.StressHierarchy3 -treeDepth 70 -minLevelSize 10 -maxLevelSize 100 -hierarchyType INTERFACES -triggerUnloadingByFillingMetaspace

      Host: AArch64 Processor rev 0 (aarch64), 8 cores, 15G, Ubuntu 14.04.3 LTS
      Time: Fri May 27 10:15:09 2016 PDT elapsed time: 26 seconds (0d 0h 0m 26s)

      --------------- T H R E A D ---------------

      Current thread (0x0000007f9c029000): GCTaskThread "GC Thread#0" [stack: 0x0000007fa344a000,0x0000007fa354a000] [id=3218]

      Stack: [0x0000007fa344a000,0x0000007fa354a000], sp=0x0000007fa3548740, free space=1017k
      Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
      V [libjvm.so+0x3d2ec0] CollectedHeap::fill_with_objects(HeapWord*, unsigned long, bool)+0xa8
      V [libjvm.so+0x4dc5b8] RemoveSelfForwardPtrHRClosure::doHeapRegion(HeapRegion*)+0x190
      V [libjvm.so+0x4c2a54] G1CollectedHeap::collection_set_iterate_from(HeapRegion*, HeapRegionClosure*)+0x3c
      V [libjvm.so+0x4dc26c] G1ParRemoveSelfForwardPtrsTask::work(unsigned int)+0x9c
      V [libjvm.so+0x9db9ac] GangWorker::loop()+0x50
      V [libjvm.so+0x9db888] AbstractGangWorker::run()+0x2c
      V [libjvm.so+0x825fe4] thread_native_entry(Thread*)+0x104
      C [libpthread.so.0+0x7e2c] start_thread+0xb0


      siginfo: si_signo: 11 (SIGSEGV), si_code: 2 (SEGV_ACCERR), si_addr: 0x00000006d4c3093c
      1. 8158168_verifier.diff
        5 kB
        Sangheon Kim
      2. 8158168.diff
        4 kB
        Thomas Schatzl
      3. 8158168-reduced-verification
        1 kB
        Thomas Schatzl
      4. assert-to-guarantee.diff
        0.4 kB
        Sangheon Kim
      5. some-barreirs.diff
        7 kB
        Sangheon Kim

        Issue Links

          Activity

          Hide
          tschatzl Thomas Schatzl added a comment -
          Yes. The objects that are referenced are in the collection set, i.e. eden or some old regions (before gc, previous gc's survivors are relabelled as eden). Eden regions are only written by the mutator, so the safepointing provides memory consistency. G1 also does not write into any old region in the collection set, so the object's values must have been written during the last gc too.
          The GC also does not overwrite or reuse the length field in these areas due to some hacks (however the length field of the destination object for large j.l.O. arrays may not always contain the true length of the array). That one is synchronized across threads by the implicit memory barriers in the work steal protocol.
          Show
          tschatzl Thomas Schatzl added a comment - Yes. The objects that are referenced are in the collection set, i.e. eden or some old regions (before gc, previous gc's survivors are relabelled as eden). Eden regions are only written by the mutator, so the safepointing provides memory consistency. G1 also does not write into any old region in the collection set, so the object's values must have been written during the last gc too. The GC also does not overwrite or reuse the length field in these areas due to some hacks (however the length field of the destination object for large j.l.O. arrays may not always contain the true length of the array). That one is synchronized across threads by the implicit memory barriers in the work steal protocol.
          Hide
          dlong Dean Long added a comment -
          I've made some progress. I can reproduce the failure about 33% of the time now, with a fastdebug build, and with instrumentation to check for a bad array at arbitrary points in the Java code.
          Show
          dlong Dean Long added a comment - I've made some progress. I can reproduce the failure about 33% of the time now, with a fastdebug build, and with instrumentation to check for a bad array at arbitrary points in the Java code.
          Hide
          dlong Dean Long added a comment -
          The reason this happens on arm64 is because it doesn't implement compact strings. I would expect the same problem on other platforms if -XX:-CompactStrings is used.
          Show
          dlong Dean Long added a comment - The reason this happens on arm64 is because it doesn't implement compact strings. I would expect the same problem on other platforms if -XX:-CompactStrings is used.
          Hide
          dholmes David Holmes added a comment - - edited
          So the problem is the missing bounds check in the intrinsic? Unsynchronized access should never lead to a crash.

          How does CompactStrings avoid the problem?
          Show
          dholmes David Holmes added a comment - - edited So the problem is the missing bounds check in the intrinsic? Unsynchronized access should never lead to a crash. How does CompactStrings avoid the problem?
          Hide
          dholmes David Holmes added a comment - - edited
          incomplete or incorrect in the face of concurrent updates? I read it as the latter based on what is described above - in which case the intrinsic can not trust the caller to do the check and the whole arrangement is fundamentally broken. Seems AbstractStringBuilder can not use the intrinsic.
          Show
          dholmes David Holmes added a comment - - edited incomplete or incorrect in the face of concurrent updates? I read it as the latter based on what is described above - in which case the intrinsic can not trust the caller to do the check and the whole arrangement is fundamentally broken. Seems AbstractStringBuilder can not use the intrinsic.

            People

            • Assignee:
              dlong Dean Long
              Reporter:
              dfazunen Dmitry Fazunenko
            • Votes:
              0 Vote for this issue
              Watchers:
              13 Start watching this issue

              Dates

              • Created:
                Updated: