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

[Calvin Cheung]

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