United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-7041100 The load in String.equals intrinsic executed before null check
JDK-7041100 : The load in String.equals intrinsic executed before null check

Details
Type:
Bug
Submit Date:
2011-05-02
Status:
Closed
Updated Date:
2013-04-20
Project Name:
JDK
Resolved Date:
2011-05-16
Component:
hotspot
OS:
generic
Sub-Component:
compiler
CPU:
generic
Priority:
P3
Resolution:
Fixed
Affected Versions:
hs21
Fixed Versions:
hs21 (b12)

Related Reports
Backport:
Backport:
Backport:
Backport:
Backport:
Relates:

Sub Tasks

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

                                    

Comments
EVALUATION

http://hg.openjdk.java.net/jdk7/hotspot-comp/hotspot/rev/e6d7eed3330c
                                     
2011-05-04
EVALUATION

Remove control from loads in String.equals intrinsic and cast argument to not-null.
                                     
2011-05-04



Hardware and Software, Engineered to Work Together