JDK-7041100 : The load in String.equals intrinsic executed before null check
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: hs21
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2011-05-02
  • Updated: 2013-09-12
  • Resolved: 2011-05-16
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availabitlity Release.

To download the current JDK release, click here.
JDK 6 JDK 7 Other
6u27-revFixed 7Fixed hs20.5Fixed
Related Reports
Relates :  
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 Remove control from loads in String.equals intrinsic and cast argument to not-null.
2011-05-04

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