RFR: 8080775: Better argument formatting for assert() and friends

[Christian Thalinger]

> On Sep 22, 2015, at 1:38 AM, David Lindholm <david.lindholm at oracle.com> wrote:
> 
> Hi,
> 
> Please review the following patch which removes the need to use err_msg() or err_msg_res() to format strings that are sent to assert(), guarantee(), fatal() and vm_exit_out_of_memory(). These are now variadic macros. So,
> 
> guarantee(_saved_index == _index, err_msg("saved index: %d index: %d", _saved_index, _index));
> 
> is replaced by
> 
> guarantee(_saved_index == _index, "saved index: %d index: %d", _saved_index, _index);

Excellent change!

> 
> This change also eliminates the need to choose if the buffer for the formatted string should be allocated on the stack (with err_msg()) or in the resource area (with err_msg_res()). There is no longer any need to allocate this extra buffer, since the va_list is sent all the way down to the functions writing the error to screen or file.

I’m not exactly sure but we might have ResourceMarks in the code just to have err_msg_res.  Did you see any cases like this?

Also, does this mean there are no buffers allocated on the stack to format the message anymore?  The reason we introduced err_msg_res was because Solaris Studio produced some bad code on SPARC which blew up the stack.

> 
> To accommodate for this, VMError is now AllStatic. All patterns of usages of this class was to create a VMError and immediately call report_and_die() on it.
> 
> Now, instead of having multiple constructors, there are multiple static VMError::report_and_die().
> 
> I have also removed all usages of err_msg() and err_msg_res() in assert(), guarantee(), fatal() and vm_exit_out_of_memory() in this patch. err_msg_res() is completely removed, err_msg() is still used by other functions.
> 
> Since we have about 1000 asserts with "" as detailed message, I had to disable the gcc-only warning for empty format strings.
> 
> (As a side note, there is also a JBS bug that describes a name change from assert to vmassert in our code. This changeset does not include such a name change, since we have over 19000 asserts in our code. We still have a "#define assert vmassert" just like before).
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8080775
> Webrev: http://cr.openjdk.java.net/~david/JDK-8080775/webrev.00/
> 
> 
> Testing: Passed JPRT
> 
> 
> Thanks,
> David
>