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

[Ioi Lam]

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
>>>>
>>>
>