[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