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