RFR(L): 8037816 : Fix for 8036122 breaks build with Xcode5/clang
david.r.chase at oracle.com
Thu Apr 17 20:16:24 UTC 2014
BUG: recent changes to enabled warnings cause clang builds on Mavericks to fail.
Format checking now complains when a non-constant string is used as a format.
FIX: This patch is adapted from Gerard Ziemski’s patch for XCode5 on Mavericks.
It seemed to me that the "right" way to do this was not to disable the checking with
pragmas that we had just enabled on the command line, but rather to correct the
problems as much as possible. I attempted to even enhance the checking by
decorating some internal printf-like methods with the format-printf attribute,
but was not able to deal with the spew of errors from newer versions of gcc.
I did, however, manage to get it clean on Mavericks with those attributes enabled,
so the problem has been reduced to dealing with one compiler's warnings.
The patch includes four sorts of changes:
(1) printf-like functions and methods that are marked as printf-like will no longer trigger complaints about their internal uses of format strings. A macro conditional on platform was introduced for this.
(2) The warning can be disabled on a case-by-case basis; a family of macros conditional on platform were introduced for this.
The last two are for use with older versions of gcc that do not support PUSH and POP, in combination with large methods containing many formatting calls -- newer gcc compilers will get sharper checking.
(3) In many cases, the best fix is to replace a call to a method expecting a format with one to a method expecting only a string. That is, replace “print(s)” with “print_raw(s)”. Note that when there is a single parameter this cannot possibly be a mistake.
(4) Use new P2I macro to clean up some of the Linux format warnings; this was feasible for some of the printf-like internal methods that were not called too often with technically-mismatched types and format specifiers.
#define P2I(p) ((intptr_t)(p))
#define SIZE_FORMAT_XW(width) "%" #width PRIxPTR
The fix is conditional for compilation on other platforms where warnings and pragmas differ.
There is a potentially conflicting fix from Coleen Phillimore also in flight through the repositories; those two calls are left unchanged in this fix to avoid conflicts.
Copyright changes deferred till later, because those will just inflate the size of the diffs; I have a script for fixing those quickly.
NON-FIX: because recent versions of gcc also complain about incorrect integer size/signedness and pointer conversions in format strings, I had to retract the printf attribute from two frequently-called methods; that would have required an even larger and more invasive patch.
I considered making this conditional on compilation with clang, but
(1) without Coleen’s patch, this will fail on Mavericks+XCode 5.
I can wait, and introduce a more-conditional ATTRIBUTE_PRINTF macro.
(2) until it can be checked on more than just that platform or we have a Mavericks+clang machine in the JPRT stable, the build risks being semi-broken for up-to-date Mac users with future changes.
A next RFE would be to make progress towards that goal; the attributes to enable are present in two comments in header files (debug.hpp and ostream.hpp), and formatting can be cleaned up to deal with the recent-version gcc complaints.
The remaining gcc warnings fall into several categories:
1) printing pointer with an integer format (fix this with P2I and macros for pointer format).
2) printing size_t with a “wrong” format (on some platforms).
3) printing intptr_t with a “wrong” format (on some platforms).
One complication is use of archaic tools that don’t support c99; this forces the use of more cumbersome methods for specifying format strings.
MountainLion + XCode 4.6.3
Mavericks + XCode 5.1.1
Ubuntu (64-bit) + gcc 4.7
Ubuntu (64-bit) + gcc 4.8
Ubuntu (64-bit) + gcc 4.4
(mixture of open and closed builds above; Mavericks was built both open and closed).
JPRT built clean for all the usual suspects
Run (fastdebug) on:
Note that most of the changes are not tested because many of them involve code that is only executed with particular flags or in error cases. I hand-verified that after preprocessing, the one formatting macro inserted into ostream.cpp expanded into something sensible. On a 64-bit OS, this:
indent().print("%" "07" "lx"":", i);
vs the old
(i is size_t, so lx is the appropriate format specifier)
I also hand-checked the changes using two different ways of viewing the diffs (BBEdit diff, and webrev-new).
More information about the hotspot-dev