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

David Chase david.r.chase at oracle.com
Fri May 9 02:54:40 UTC 2014


FYI, both jdk9 and jdk8u versions of the patch passed hotspot testing on JPRT
(in addition to passing for all builds).

David

On 2014-05-08, at 8:32 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

> 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
>> 
>> 



More information about the hotspot-dev mailing list