RFR(XS): 8060721: Test runtime/SharedArchiveFile/LimitSharedSizes.java fails in jdk 9 fcs new platforms/compiler

[Calvin Cheung]

Thanks - Ioi.

On 11/7/2014 12:44 PM, Ioi Lam wrote:
> Calvin, the new changes look good to me.
>
> - Ioi
>
> On 11/7/14, 11:28 AM, Calvin Cheung wrote:
>> On 11/6/2014 10:38 PM, David Holmes wrote:
>>> Hi Calvin,
>>>
>>> On 7/11/2014 11:06 AM, Calvin Cheung wrote:
>>>> I've updated the webrev at the same location:
>>>>      http://cr.openjdk.java.net/~ccheung/8060721/webrev/
>>>> I also re-ran the tests.
>>>>
>>>> Please take a look.
>>>
>>>  717         jio_snprintf(class_list_path_str + class_list_path_len,
>>>  718                      sizeof(class_list_path_str) - 
>>> class_list_path_len,
>>>  719                      "%slib", os::file_separator());
>>>  720       }
>>>  721     }
>>>  722     class_list_path_len = (int)strlen(class_list_path_str);
>>>
>>> The strlen recalculation at #722 should be moved inside the if-block 
>>> as that is the only time it is needed.
>> Agreed.
>>> Also can we not just do += 4 ?
>> I didn't want to use 4 to avoid another magic number but in this case 
>> I think it's obvious.
>>
>> I've updated webrev at the same location:
>>     http://cr.openjdk.java.net/~ccheung/8060721/webrev/
>>
>> thanks,
>> Calvin
>>>
>>> Thanks,
>>> David
>>>
>>>> thanks,
>>>> Calvin
>>>>
>>>> On 11/5/2014 8:28 PM, Calvin Cheung wrote:
>>>>> On 11/5/2014 4:50 PM, David Holmes wrote:
>>>>>> On 6/11/2014 5:14 AM, Calvin Cheung wrote:
>>>>>>> While upgrading the compiler on Mac for jdk9, we found this 
>>>>>>> compiler
>>>>>>> bug
>>>>>>> where it skips the following 2 lines of code in metaspaceShared.cpp
>>>>>>> when
>>>>>>> optimization is enable (set to -Os) for the fastdebug and product
>>>>>>> builds.
>>>>>>>      strcat(class_list_path_str, os::file_separator());
>>>>>>>      strcat(class_list_path_str, "classlist");
>>>>>>>
>>>>>>> The bug is reproducible with Xcode 5.1.1 and 6.1.
>>>>>>>
>>>>>>> A workaround fix is to rewrite an "if" block in the
>>>>>>> MetaspaceShared::preload_and_dump() method.
>>>>>>
>>>>>> Can't you simply replace the strcats with jio_snprintf and do away
>>>>>> with the sub_path array?
>>>>> The following works. I'll do more testing before sending an updated
>>>>> webrev.
>>>>>
>>>>> --- a/src/share/vm/memory/metaspaceShared.cpp
>>>>> +++ b/src/share/vm/memory/metaspaceShared.cpp
>>>>> @@ -713,12 +713,15 @@
>>>>>      int class_list_path_len = (int)strlen(class_list_path_str);
>>>>>      if (class_list_path_len >= 3) {
>>>>>        if (strcmp(class_list_path_str + class_list_path_len - 3,
>>>>> "lib") != 0) {
>>>>> -        strcat(class_list_path_str, os::file_separator());
>>>>> -        strcat(class_list_path_str, "lib");
>>>>> +        jio_snprintf(class_list_path_str + class_list_path_len,
>>>>> +                     sizeof(class_list_path_str) - 
>>>>> class_list_path_len,
>>>>> +                     "%slib", os::file_separator());
>>>>>        }
>>>>>      }
>>>>> -    strcat(class_list_path_str, os::file_separator());
>>>>> -    strcat(class_list_path_str, "classlist");
>>>>> +    class_list_path_len = (int)strlen(class_list_path_str);
>>>>> +    jio_snprintf(class_list_path_str + class_list_path_len,
>>>>> +                 sizeof(class_list_path_str) - class_list_path_len,
>>>>> +                 "%sclasslist", os::file_separator());
>>>>>      class_list_path = class_list_path_str;
>>>>>    } else {
>>>>>      class_list_path = SharedClassListFile;
>>>>>>
>>>>>> Or even try strncat instead of strcat?
>>>>> I think jio_snprintf is better because it null terminates the string.
>>>>> If I use strncat, I'll need to initialize the entire buffer to null.
>>>>>
>>>>> thanks,
>>>>> Calvin
>>>>>>
>>>>>> David
>>>>>>
>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8060721
>>>>>>>
>>>>>>> webrev: http://cr.openjdk.java.net/~ccheung/8060721/webrev/
>>>>>>>
>>>>>>> Testing:
>>>>>>>      JPRT
>>>>>>>      The affected testcase with product, fastdebug, and debug 
>>>>>>> builds
>>>>>>> built with Xcode 5.1.1 and 6.1.
>>>>>>>
>>>>>>> thanks,
>>>>>>> Calvin
>>>>>
>>>>
>>
>