[9] RFR(L): 8037816 : Fix for 8036122 breaks build with Xcode5/clang

[David Chase]

On 2014-04-18, at 5:12 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

> Hi David,
> 
> In general I agree with changes.
> 
> So for older gcc the warning will be ignored for the rest of file or only for the following method body after pragma. Right?

This is correct, though unfortunate.  (Right now, we're ignoring it all the time for all the files).

> Why you need pragmas around outputStream::vprint_cr()? There is no pragma for vprint().

I am not 100% sure.  I will check this.  (I.e, I will remove this and test it to see if anything breaks).

> In heapInspection.hpp empty line in wring place (should be before pragma).
> 
> Spacint is off (in globalDefinitions.hpp):
> + #ifndef  ATTRIBUTE_PRINTF
> + #define ATTRIBUTE_PRINTF(fmt,vargs)

I'll get these.

As you surely know from the compiler chat (but non-compiler-team might not) there is more to come.  The rough estimate of what's coming:

1) printf-attribute annotation of all our printf-like methods, enabled for clang >= 3.1 and gcc >= 4.2.
2) 275 files touched, 121 of those with a more-work-needed-for-picky-gcc pragma macro.
3) 6700 line patch file.

Here's my notes on what's coming, which is still being tested and needs a round of porting to non-x86 gcc platforms, both of which should take at least till late Tuesday.

Changes to fix:

Decorated several more printf-like internal routines with ATTRIBUTE_PRINTF(x,y)

Left ATTRIBUTE_PRINTF(x,y) enabled for all clang and gcc compilations.
(This causes substantial amounts of noise, but ensures that there shall be no backsliding on these improvements in the future.)

Added some additional format-string macros to allow repair of some of our formats.  The intent for all changes was to not modify the output at all unless it was almost certainly correct (e.g., printing a default case for unknown constant type with a %f format, or printing a known-integer with %s).

#define UINT64_FORMAT_X        "%" PRIx64
#define PTR_FORMAT_W(width)   "%" #width PRIxPTR
#define SIZE_FORMAT_XW(width) "%" #width PRIxPTR
#define SIZE_FORMAT_X         "%" PRIxPTR

At least two very-recently-fixed bugs were detected in this automated sweep, and I see quite a few cases of could-be-a-bug but lacked the time to construct a test case to prove it, so I just fixed it as the warning specified.

Replace const char * fmt = string with #define fmt string to work around literal-minded format-checking.  Rename two occurrences of “fmt” to be different, more descriptive, less likely to clash.

Test push/pop with older versions of clang (works, as far as 3.1).

Add _Pragma("GCC diagnostic ignored \"-Wformat-security\"") to check-disabling macro for older clang.

P2I inline method, not macro.
static inline intptr_t p2i(void * p) { return (intptr_t) p; }

Tested print(const char * s) overloading.
test.cpp:30:3: error: call to 'my_print' is ambiguous
  my_print(s);
  ^~~~~~~~
test.cpp:12:6: note: candidate function
void my_print(const char * fmt, ...) {
     ^
test.cpp:18:6: note: candidate function
void my_print(const char * s) {
     ^

Instead, add print_cr() for the most common use.
Constructors taking format and args are unfortunately forced to take (“%s”, “”) for an empty string.

To tone down gcc’s hypersensitivity to format mismatches, add a use of this macro after header inclusions for source files whose format strings cannot be easily and surely corrected (e.g., through addition of p2i() not too often):
#define PRAGMA_FORMAT_MUTE_WARNINGS_FOR_GCC _Pragma("GCC diagnostic ignored \"-Wformat\"") _Pragma("GCC diagnostic error \"-Wformat-nonliteral\"") _Pragma("GCC diagnostic error \"-Wformat-security\"")
Use of this macros does not silence warnings for nonliteral, non-constant, empty, or mismatched arg length parameter strings.

For two files,
src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp
the macro ATTRIBUTE_PRINTF(x,y)
is defined to be empty above the header inclusions; the format strings are wrong in peculiar ways, and they are buried in macros.

David