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

The load in String.equals intrinsic executed before null check

    Details

    • Subcomponent:
    • Resolved In Build:
      b12
    • CPU:
      generic
    • OS:
      generic
    • Verification:
      Not verified

      Backports

        Description

        Tom Rodriguez wrote:
        > It's possible. Even if it is, I suspect there might be some way to modify the intrinsic guards to protect against it.
        >
        > tom
        >
        > On Apr 29, 2011, at 10:53 AM, Vladimir Kozlov wrote:
        >
        >> It looks like the bug I have:
        >>
        >> 6831314: C2 may incorrectly change control of type nodes
        >>
        >> Vladimir
        >>
        >> Tom Rodriguez wrote:
        >>> This appears to be a bug with the String.equals intrinsic. For some reason the control for the null check is wrong allowing the load to float into the same block. It seems to require a very particular set of profile data to get the bad schedule. Normally the frequencies are different enough to keep it scheduled in a later block. It appears that if you change the code to this:
        >>> public static boolean stringEQ(String a, String b) {
        >>> return a == null ? false : a.equals(b);
        >>> }
        >>> then it doesn't happen. You can also disable the intrinsic using -XX:UnlockDiagnosticVMOptions -XX:DisableIntrinsic=_equals.
        >>> I reproduced the crash with this test:
        >>> public class StringEQ {
        >>> static String n = null;
        >>> public static void main(String[] args) throws Exception {
        >>> for (int i = 0; i < 5000; i++) {
        >>> stringEQ(args[0], args[1]);
        >>> stringEQ(args[0], n);
        >>> stringEQ(args[0], args[0]);
        >>> stringEQ(n, args[0]);
        >>> }
        >>> }
        >>> public static boolean stringEQ(String a, String b) {
        >>> if (a == b)
        >>> return true;
        >>> if (a == null || b == null)
        >>> return false;
        >>> else
        >>> return a.equals(b);
        >>> }
        >>> }
        >>> after modifying gcm.cpp to always schedule in the earliest legal block. Please file a bug.
        >>> tom
        >>> On Apr 26, 2011, at 7:53 PM, David Holmes wrote:
        >>>> Hi,
        >>>>
        >>>> I've cc'ed the compiler team and bcc'ed the runtime team.
        >>>>
        >>>> Cheers,
        >>>> David
        >>>>
        >>>> Fui Shien Choong said the following on 04/27/11 11:43:
        >>>>> Hi
        >>>>> I have a few crashes in compiled code doing string comparisons.
        >>>>> The method looks like this.
        >>>>> public static boolean stringEQ(String a, String b)
        >>>>> {
        >>>>> if(a == b)
        >>>>> return true;
        >>>>> if(a == null || b == null)
        >>>>> return false;
        >>>>> else
        >>>>> return a.equals(b);
        >>>>> }
        >>>>> (dbx) frame 8
        >>>>> 0xffffffff76bce228: ld [%i1 + 20], %l0
        >>>>> (dbx) dis 0xffffffff76bce200, 0xffffffff76bce228
        >>>>> 0xffffffff76bce200: save %sp, -144, %sp
        >>>>> 0xffffffff76bce204: cmp %i0, %i1 (a == b)
        >>>>> 0xffffffff76bce208: be,pn %xcc,0xffffffff76bce338 ! 0xffffffff76bce338
        >>>>> 0xffffffff76bce20c: nop
        >>>>> 0xffffffff76bce210: cmp %i0, 0 (a == null)
        >>>>> 0xffffffff76bce214: be,pn %xcc,0xffffffff76bce340 ! 0xffffffff76bce340
        >>>>> 0xffffffff76bce218: nop
        >>>>> 0xffffffff76bce21c: ld [%i0 + 20], %g3 (load count of 1st string)
        >>>>> 0xffffffff76bce220: cmp %i1, 0
        >>>>> 0xffffffff76bce224: be,pn %xcc,0xffffffff76bce340 ! 0xffffffff76bce340
        >>>>> 0xffffffff76bce228: ld [%i1 + 20], %l0 << crash here
        >>>>> (dbx) x $i1
        >>>>> 0x0000000000000000: 0x0000000000000000
        >>>>> Shouldnt we insert a nop at 0xffffffff76bce228? This gets evaluated anyway
        >>>>> prior to the branch.
        >>>>> Is there a known bug for this?
        >>>>> Thanks.
        >>>>> Fui Shien

          Attachments

            Issue Links

              Activity

                People

                • Assignee:
                  kvn Vladimir Kozlov
                  Reporter:
                  kvn Vladimir Kozlov
                • Votes:
                  0 Vote for this issue
                  Watchers:
                  3 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Imported:
                    Indexed: