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

[David Holmes]

On 9/05/2014 7:08 AM, David Chase wrote:
> On 2014-05-07, at 6:38 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>> 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.

But surely the p2i + INTPTR_FORMAT change is an even bigger revert! 
Unless you propose to keep using PTR_FORMAT even with p2i ? (In case it 
is half as big a revert but still big).

Put another way, once you've done the p2i+INTPTR_FORMAT change, what 
incentive is there to fix the %p issue and revert everything you just 
changed? This seems like a now-or-never fix to me.

David H.
-------


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