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

Christian Thalinger christian.thalinger at oracle.com
Wed Sep 23 19:02:21 UTC 2015

On Sep 22, 2015, at 10:19 PM, David Lindholm <david.lindholm at oracle.com> wrote:

Christian, On 2015-09-22 18:31, Christian Thalinger wrote:

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 errmsg() or errmsgres() to format strings that are sent to assert(), guarantee(), fatal() and vmexitoutofmemory(). These are now variadic macros. So, guarantee(savedindex == index, errmsg("saved index: %d index: %d", savedindex, index)); is replaced by guarantee(savedindex == index, "saved index: %d index: %d", savedindex, index); Excellent change! Thank you! This change also eliminates the need to choose if the buffer for the formatted string should be allocated on the stack (with errmsg()) or in the resource area (with errmsgres()). There is no longer any need to allocate this extra buffer, since the valist 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 errmsgres. Did you see any cases like this? I did not, but Kim Barrett have spotted one so far (I'll fix this). Also, does this mean there are no buffers allocated on the stack to format the message anymore? Yes, that is correct. The reason we introduced errmsgres was because Solaris Studio produced some bad code on SPARC which blew up the stack. Yes, and this should not be needed any more.

I might be repeating myself but… excellent!

Thanks, David

To accommodate for this, VMError is now AllStatic. All patterns of usages of this class was to create a VMError and immediately call reportanddie() on it. Now, instead of having multiple constructors, there are multiple static VMError::reportanddie(). I have also removed all usages of errmsg() and errmsgres() in assert(), guarantee(), fatal() and vmexitoutofmemory() in this patch. errmsgres() is completely removed, errmsg() 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

More information about the hotspot-dev mailing list