SIGBUS in Access<1572864UL>::store_at<int> on Solaris/SPARC

[jesper.wilhelmsson at oracle.com]

Awesome!

Thanks,
/Jesper

> On 23 Feb 2018, at 12:54, David Holmes <david.holmes at oracle.com> wrote:
> 
> Fix has been pushed.
> 
> David
> 
> On 23/02/2018 9:49 PM, Roman Kennke wrote:
>> I think it will.
>> Roman
>> On Fri, Feb 23, 2018 at 12:45 PM,  <jesper.wilhelmsson at oracle.com> wrote:
>>> Hi Roman,
>>> 
>>> Will this fix be pushed today?  If not I would prefer to back out JDK-8197999 to get clean results in the Friday nightly.
>>> 
>>> Thanks,
>>> /Jesper
>>> 
>>> 
>>>> On 22 Feb 2018, at 22:17, Roman Kennke <rkennke at redhat.com> wrote:
>>>> 
>>>> I took the bug.
>>>> I tried with casting similar to what you suggested, but that failed my
>>>> test. Maybe I had the parenthesis differently? In any case, I made it
>>>> so that it matches what is in oop.inline.hpp boolean accessor.
>>>> 
>>>> I'll move the test to a new file and post an RFR separately.
>>>> 
>>>> Thanks, Roman
>>>> 
>>>> 
>>>> On Thu, Feb 22, 2018 at 10:04 PM, Stefan Karlsson
>>>> <stefan.karlsson at oracle.com> wrote:
>>>>> On 2018-02-22 21:41, Roman Kennke wrote:
>>>>>> 
>>>>>> Ok thank you.
>>>>>> I tried your patch and can confirm that it works/passes. :-) It also
>>>>>> gives me some ideas how gtest works.
>>>>>> 
>>>>>> I modified the test so that it fails without the fix, and passes with the
>>>>>> fix:
>>>>>> 
>>>>>> http://cr.openjdk.java.net/~rkennke/8198564/webrev.00/
>>>>>> 
>>>>>> If you think that's good, then I can post a formal RFR and take over the
>>>>>> bug.
>>>>> 
>>>>> 
>>>>> Yes, this seems good. A similar patch using (jboolean)(((jint)contents) & 1)
>>>>> passes tests that used to fail on sparc.
>>>>> 
>>>>> You might want to consider moving the test to a test_typeArrayOop.cpp file.
>>>>> 
>>>>> Thanks,
>>>>> StefanK
>>>>> 
>>>>> 
>>>>>> 
>>>>>> Roman
>>>>>> 
>>>>>> On Thu, Feb 22, 2018 at 9:00 PM, Stefan Karlsson
>>>>>> <stefan.karlsson at oracle.com> wrote:
>>>>>>> 
>>>>>>> On 2018-02-22 20:47, Roman Kennke wrote:
>>>>>>>> 
>>>>>>>> On Thu, Feb 22, 2018 at 8:22 PM, Stefan Karlsson
>>>>>>>> <stefan.karlsson at oracle.com> wrote:
>>>>>>>>> 
>>>>>>>>> On 2018-02-22 20:14, Roman Kennke wrote:
>>>>>>>>>> 
>>>>>>>>>> Right. This looks like possible and likely cause of the problem. And
>>>>>>>>>> it worked before because of implicit conversion back to jboolean:
>>>>>>>>>> 
>>>>>>>>>> -  void bool_at_put(int which, jboolean contents)  {
>>>>>>>>>> *bool_at_addr(which) = (((jint)contents) & 1); }
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> Can you test it? Because, I can't ;-)
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> Yes. I'm kicking of some testing on sparc. Could you write a gtest for
>>>>>>>>> this?
>>>>>>>> 
>>>>>>>> I can try. I never wrote a gtest before ;-) Is there an existing one
>>>>>>>> that I could use as template, and/or pointers how to start?
>>>>>>> 
>>>>>>> 
>>>>>>> You can look at the existing tests in test/hotspot/gtest. I suggest you
>>>>>>> read
>>>>>>> the official googletest doc to get started. There might be some other
>>>>>>> document about our adaption of googletest, but I don't know where it is.
>>>>>>> 
>>>>>>> Maybe something like this would work:
>>>>>>> 
>>>>>>> diff --git a/test/hotspot/gtest/oops/test_arrayOop.cpp
>>>>>>> b/test/hotspot/gtest/oops/test_arrayOop.cpp
>>>>>>> --- a/test/hotspot/gtest/oops/test_arrayOop.cpp
>>>>>>> +++ b/test/hotspot/gtest/oops/test_arrayOop.cpp
>>>>>>> @@ -22,6 +22,7 @@
>>>>>>>   */
>>>>>>> 
>>>>>>>  #include "precompiled.hpp"
>>>>>>> +#include "memory/universe.hpp"
>>>>>>>  #include "oops/arrayOop.hpp"
>>>>>>>  #include "oops/oop.inline.hpp"
>>>>>>>  #include "unittest.hpp"
>>>>>>> @@ -86,4 +87,37 @@
>>>>>>>  TEST_VM(arrayOopDesc, narrowOop) {
>>>>>>>    ASSERT_PRED1(check_max_length_overflow, T_NARROWOOP);
>>>>>>>  }
>>>>>>> +
>>>>>>> +TEST_VM(arrayOopDesc, bool_at_put) {
>>>>>>> +  char mem[100];
>>>>>>> +  memset(mem, 0, ARRAY_SIZE(mem));
>>>>>>> +
>>>>>>> +  char* addr = align_up(mem, 16);
>>>>>>> +
>>>>>>> +  typeArrayOop o = (typeArrayOop) addr;
>>>>>>> +  o->set_klass(Universe::boolArrayKlassObj());
>>>>>>> +  o->set_length(10);
>>>>>>> +
>>>>>>> +
>>>>>>> +  ASSERT_EQ((jboolean)0, o->bool_at(0));
>>>>>>> +  ASSERT_EQ((jboolean)0, o->bool_at(1));
>>>>>>> +  ASSERT_EQ((jboolean)0, o->bool_at(2));
>>>>>>> +  ASSERT_EQ((jboolean)0, o->bool_at(3));
>>>>>>> +  ASSERT_EQ((jboolean)0, o->bool_at(4));
>>>>>>> +  ASSERT_EQ((jboolean)0, o->bool_at(5));
>>>>>>> +  ASSERT_EQ((jboolean)0, o->bool_at(6));
>>>>>>> +  ASSERT_EQ((jboolean)0, o->bool_at(7));
>>>>>>> +
>>>>>>> +  o->bool_at_put(0, 1);
>>>>>>> +
>>>>>>> +  ASSERT_EQ((jboolean)1, o->bool_at(0));
>>>>>>> +  ASSERT_EQ((jboolean)0, o->bool_at(1));
>>>>>>> +  ASSERT_EQ((jboolean)0, o->bool_at(2));
>>>>>>> +  ASSERT_EQ((jboolean)0, o->bool_at(3));
>>>>>>> +  ASSERT_EQ((jboolean)0, o->bool_at(4));
>>>>>>> +  ASSERT_EQ((jboolean)0, o->bool_at(5));
>>>>>>> +  ASSERT_EQ((jboolean)0, o->bool_at(6));
>>>>>>> +  ASSERT_EQ((jboolean)0, o->bool_at(7));
>>>>>>> +}
>>>>>>> +
>>>>>>>  // T_VOID and T_ADDRESS are not supported by max_array_length()
>>>>>>> 
>>>>>>> And then run with:
>>>>>>> ../build/fastdebug/hotspot/variant-server/libjvm/gtest/gtestLauncher -jdk
>>>>>>> ../build/fastdebug/jdk --gtest_filter="arrayOopDesc*"
>>>>>>> 
>>>>>>> StefanK
>>>>>>> 
>>>>>>>> Roman
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>> 
>>>