RFR: 8149591 - Prepare hotspot for GTest

Igor Ignatyev igor.ignatyev at oracle.com
Wed May 4 19:29:17 UTC 2016


http://cr.openjdk.java.net/~iignatyev/8149591/webrev.02

Kim,

thank you for review, I've added a comment.

Working on the fix, I also noticed that there are platforms which use gcc < 4.8, and reports warnings ‘cuz of 'report_assert_msg(“”)’, so I guarded report_assert_msg w/ ATTRIBUTE_PRINTF with corresponding version check and added a declaration w/o ATTRIBUTE_PRINTF.

Stefan,
thanks for answering while I was out. 

Thanks,
— Igor

> On May 4, 2016, at 12:43 AM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
> 
> On 2016-05-03 23:24, Kim Barrett wrote:
>>> On May 3, 2016, at 3:57 PM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
>>> 
>>> Hi Kim,
>>> 
>>> On 2016-05-03 21:20, Kim Barrett wrote:
>>>>> On Apr 29, 2016, at 9:47 PM, Igor Ignatyev <igor.ignatyev at oracle.com> wrote:
>>>>> 
>>>>> Hi,
>>>>> 
>>>>> I’d like to renew this RFR.
>>>>> 
>>>>> besides updated previous changes, new webrev contains two new things:
>>>>> - new vm-flag — ExecutingUnitTests, we use it in debug jvm to print a clean error message (w/o assert, line, file, etc). it is needed to simplify comparing error message in “death” tests (the tests which intentionally cause crashes/asserts)
>>>>> - if CreateCoredumpOnCrash is false, we don't determine default core path
>>>>> 
>>>>> webrev: http://cr.openjdk.java.net/~iignatyev/8149591/webrev.00/
>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8149591
>>>>> 
>>>>> Thanks,
>>>>> — Igor
>>>> Not a full review yet, but a question before I go on.
>>>> 
>>>> I don’t understand the purpose of FormatBufferDummy.  I’m guessing it is to disambiguate the new
>>>> FormatBuffer constructor taking a va_list argument.
>>> Yes, that was the purpose.
>>> 
>>>>   But with these two overloads:
>>>> 
>>>>   FormatBuffer(const char* format, …);
>>>>   FormatBuffer(const char* format, va_list ap);
>>>> 
>>>> the latter should always be a better match when the second argument is a va_list, and should never
>>>> be a match at all otherwise.  (An overload containing ellipsis is always the least good match.)  So I
>>>> think we shouldn’t need FormatBufferDummy.
>>>> 
>>> I started with that implementation but got the following error:
>>> 
>>> src/share/vm/gc/shared/genCollectedHeap.cpp: In member function 'void GenCollectedHeap::do_collection(bool, bool, size_t, bool, GenCollectedHeap::GenerationType)':
>>> src/share/vm/gc/shared/genCollectedHeap.cpp:435:44: error: deprecated conversion from string constant to 'va_list {aka char*}' [-Werror=write-strings]
>>>     FormatBuffer<> gc_string("%s", "Pause ");
>>> 
>>> So, adding FormatBufferDummy was a quick-n-dirty solution to fix this.
>> va_list on this platform seems to be just a typedef for char*.
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=33588
>> which is “won’t fix”.  I hadn’t realized that overloading on va_list isn’t really useful;
>> I guess that’s why there are lots of “v”-prefixed/suffixed operations around.  I can’t
>> think of a better solution.  A comment would be nice.
> 
> Yes.
> 
> StefanK



More information about the hotspot-dev mailing list