<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <p>This version looks good.  If you wanted you could also do the
      following:</p>
    <pre>
<span class="new">       CompiledMethod* nm = NULL;</span>
<span class="new">       JavaThread* thread = (JavaThread*)t;</span>
<span class="new">       if (in_java) {</span>
<span class="new">          CodeBlob* cb</span><span class="new"> = CodeCache::find_blob_unsafe(pc);</span></pre>
    <br>
    because "cb" is only used in that inner block.<br>
    <br>
    dl<br>
    <br>
    <div class="moz-cite-prefix">On 11/14/17 1:11 PM, jamsheed wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:2c16f1ff-57fc-9fed-2290-7e3a99c6a741@oracle.com">Hi
      Dean,
      <br>
      <br>
      revised webrev:
      <br>
      <br>
      <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jcm/6415680/webrev.01/">http://cr.openjdk.java.net/~jcm/6415680/webrev.01/</a>
      <br>
      <br>
      i agree to the comment that it is potentially unsafe to assume the
      implementation, and count can be in autoincrement mode. so with
      this bug i would like to deal with only
      <br>
      <br>
      the single value access error handling.
      <br>
      <br>
      revised webrev: <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jcm/6415680/webrev.01/">http://cr.openjdk.java.net/~jcm/6415680/webrev.01/</a>
      <br>
      <br>
      Best regards,
      <br>
      <br>
      Jamsheed
      <br>
      <br>
      <br>
      <br>
      On Tuesday 14 November 2017 11:57 PM, <a class="moz-txt-link-abbreviated" href="mailto:dean.long@oracle.com">dean.long@oracle.com</a> wrote:
      <br>
      <blockquote type="cite">Adding runtime alias...
        <br>
        <br>
        comments inlined below.
        <br>
        <br>
        <br>
        On 11/13/17 9:10 PM, jamsheed wrote:
        <br>
        <br>
        <blockquote type="cite">Hi, request for review, jbs:
          <a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-6415680">https://bugs.openjdk.java.net/browse/JDK-6415680</a> webrev:
          <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jcm/6415680/webrev.00/">http://cr.openjdk.java.net/~jcm/6415680/webrev.00/</a>
          Description: 1) changes equivalent to JDK-4454115 is done for
          windows.
          <br>
        </blockquote>
        <br>
        It looks like "nm" can be uninitialized if "in_java" is false.
        <br>
        <br>
        <blockquote type="cite">2) added guard to multiple value access
          sites, Unsafe_CopyMemory0, Unsafe_SetMemory0 and
          Unsafe_CopySwapMemory0.
          <br>
        </blockquote>
        <br>
        Can you narrow the scope of the unsafe access using something
        like GuardUnsafeAccess, instead of marking the whole function as
        doing unsafe access?
        <br>
        <br>
        There are some risks with trying to  abort a copy function.
        <br>
        <br>
        First, won't we get multiple exceptions until we finally hit the
        end of the range?  What if the bad range is very large?
        <br>
        <br>
        Second, what if the loop is using auto-increment instructions?
        Skipping to the next instruction would mean we loop forever if
        the increment never happens.
        <br>
        <br>
        I think if we are going to safely abort copy functions then we
        need to register them as a kind of CodeBlob that has a special
        abort entry point or exception handler we can redirect to, or
        maybe pop the whole frame and return.
        <br>
        <br>
        Is there really a problem with these copy functions?  I'm
        wondering why Mikael did not identify these as a problem in
        8154592.
        <br>
        <br>
        <blockquote type="cite">3) Unsafe_CopySwapMemory0 is JVM_LEAF so
          removed thread->thread_state() == _thread_in_vm checks from
          signal handler
          <br>
        </blockquote>
        <br>
        How about adding a check for _thread_in_native instead of
        removing the check entirely?
        <br>
        <br>
        dl
        <br>
        <br>
        <blockquote type="cite">Best regards, Jamsheed
          <br>
          <br>
        </blockquote>
        <br>
      </blockquote>
      <br>
    </blockquote>
    <br>
  </body>
</html>