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

David Chase david.r.chase at oracle.com
Thu May 8 21:08:53 UTC 2014


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




More information about the hotspot-dev mailing list