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

[Vladimir Kozlov]

Looks good to me.

Thanks,
Vladimir

On 5/8/14 2:08 PM, David Chase wrote:
>
> On 2014-05-07, at 6:38 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>
>> I see several cases of new print_raw_cr(). Why not print_cr("%s", msg)? It is in ciInstanceKlass.cpp, javaClasses.cpp, codeBlob.cpp, compilerOracle.cpp, g1CollectedHeap.cpp, psPromotionManager.cpp, metaspaceShared.cpp, constMethod.cpp, constantPool.cpp, cpCache.cpp, instanceKlass.cpp, method.cpp, loopnode.cpp, jniCheck.cpp, java.cpp, os.cpp, sweeper.cpp, thread.cpp, attachListener.cpp, diagnosticCommand.cpp, nmtDCmd.cpp, ostream.cpp, vmError.cpp.
>
> Fixed.
>
>> In ciEnv.cpp  it can be merged with previous line:
>>
>>    tty->print("# Compiler replay data is saved as: %s", buffer);
>>    tty->print("# Compiler inline data is saved as: %s", buffer);
>
> Merged.
>
>> In ageTable.cpp why you changed type of 'age' from uint to uintx? The used format for it is "%3u".
>
> Reverted.
>
>> Did you changes some spacing in generateOopMap.cpp? I see only Copyright year update.
>
> Reverted.
>
>> In memnode.cpp use INTX_FORMAT instead of %" PRIdPTR.
>
> Fixed.
>
> I looked into the issue of changing PTR_FORMAT to be a platform-dependent variation on %p;
> however it appears to be not-possible to do this without changing with some formatted pointer
> outputs (I've tried hard not to change outputs at all, though there have been some changes at
> reviewer request), and because this may affect user-visible outputs (e.g., parsing GC logs) I'd
> rather not do it as part of this giant patch -- if there's a problem, this is a big revert.
>
> This bug fix also deals with https://bugs.openjdk.java.net/browse/JDK-8029996 -- that is,
> all printf-like methods are tagged as being printf-like, and all carn-inducing uses of those
> methods has been "fixed", either through actual correction of the format, or by selective
> muting of the gcc warnings when they were merely gcc being picky about different names
> for the same number of integer bytes (I think these should still be attended to incrementally).
>
> I think the follow-on bugs are:
>
> 1) investigate definition of PTR_FORMAT as some form of %p, together with a clean separation
> of PTR_FORMAT and INTPTR_FORMAT in formatting (we should be nearly there already).
> Do NOT do a wholesale replace of INTPTR_FORMAT.
>
> 2) incremental removal of annoying *and* unnecessary INTPTR_FORMAT/p2i where the output
> is not intended for external consumption.
>
> 3) Attention paid to the formatting macros in
>
> src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
> src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp
>
> which are so broken that format checking in those two files is completely disabled.
>
> 4) incremental removal of PRAGMA_FORMAT_MUTE_WARNINGS_FOR_GCC
> (This should follow resolution of follow-on bug (1), so that the PTR_FORMAT option is available).
>
> New webrevs (alleged backport included for completeness):
>
> jdk9:    http://cr.openjdk.java.net/~drchase/8037816/webrev-9.05-08a/
> jdk8u:  http://cr.openjdk.java.net/~drchase/8037816/webrev-8u.05-08a/
>
> (Note that copyrights will be inserted mechanically later; there is also a single instance
> of a .cpp file with an extra space on one line, that will also be fixed mechanically.)
>
> New testing:
>
> jdk9:
> Mavericks
> Mountain Lion
> Solaris 11 SPARC
> Ubuntu 32-bit, gcc-4.8
> Ubuntu 64-bit, gcc-4.8
> JPRT build, including closed
>
> jdk8u:
> Mountain Lion
> Ubuntu 32-bit, gcc-4.8
> Ubuntu 64-bit, gcc-4.8
> JPRT build, including closed
>
> Possible complications to backporting fixes from nine to 8u20 is an issue, thus
> I am also working on the 8u version of this concurrently.
>
> Is it soup yet?
>
> David
>
>