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

David Chase david.r.chase at oracle.com
Thu Apr 24 20:49:16 UTC 2014


Revised webrev.

Webrev: http://cr.openjdk.java.net/~drchase/8037816/webrev.02/

Bug: https://bugs.openjdk.java.net/browse/JDK-8037816

Testing: built everywhere that JPRT builds, personally checked on Mountain Lion (gcc),
Mavericks (clang), Linux-32 (gcc), Linux-64 (gcc), Solaris-11-sparc (solstudio).

Before you look at the webrev, four things:

(0) I'll handle copyrights in an automated post-pass, if that's okay.  In defense of its absurd size,
this patch is wide but not very deep; the most common changes were:
     sprinkle in calls to p2i (277 lines)
     MUTE_WARNINGS_FOR_GCC (128 lines)
     white space (120 lines)
     print_raw[_cr] (108)
     print_cr() (68)
     PRAGMA_ defines or push/pop/diagnostic macros. (90)
     matching format/arg for SIZE_FORMAT (47)
     ATTRIBUTE_PRINTF (26)
     Other macro definitions (#endif, #else, remaining format macros) (24)
     Added "%s" format strings to resolve-nonliteral-string warnings. (20)
     Other instances of _FORMAT (may be format change or cast arg change) (19)
     Comments (11)
     Stray copyright updates (6)
     "fmt_" macros (6)
     "PRI.PTR" (5)
I've appended the remaining uncategorized added/changed lines at the bottom.

(1) the revised change set leaves all the format checking enabled as much as possible for gcc
and clang compilation, and all gross format errors have been repaired.  This includes our own
functions and methods with printf-like behavior.  Not only with this build with clang, it should generally
tend to continue to build with clang.

No new big format mistakes (*) will be allowed anywhere, and new little mistakes will be warned
except in those files currently marked with PRAGMA_FORMAT_MUTE_WARNINGS_FOR_GCC.
The intent is that those files will get cleaned up incrementally (one by one, maybe dozen by dozen).
(*) Defined to be: mismatched format/arg lengths, empty format string, non-literal format string.

(2) there will be additional RFEs covering code I was unsure of; it's as broken/unportable
as it ever was, but now we know it.

(a) For two files,
  src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
  src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp
the macro ATTRIBUTE_PRINTF(x,y)
is defined to be empty above the header inclusions; the format strings are wrong in peculiar ways, and they are buried in macros.  This is conditional on not-clang, because clang (1) doesn’t warn about the formats in the same way and (2) has a problem with conflicting definitions before includes and within precompiled header files.

(b) The xsl file used to generate some of the jvmti sources now generates a mute-warnings macro after the header inclusions, because gcc doesn't like some of the format/arg pairs in that generated code, and fixing it was beyond me.


(3) these are the parts/questions I think need opinions/resolution:

(a) print_raw(foo) vs print(“%s”, foo)?

(b) print_cr() vs print_raw_cr(“”) vs print_cr(“%s”, “”) vs cr().
Note that some of our format-related constructors demand a format, hence they go
with print("%s", "") when they start empty.

(c) Parameter was passed with no format (two cases).  I removed it.  What do we really want here?

--- old/src/share/vm/opto/gcm.cpp	2014-04-23 14:08:48.000000000 -0400
+++ new/src/share/vm/opto/gcm.cpp	2014-04-23 14:08:47.000000000 -0400
@@ -2014,7 +2014,7 @@
   tty->print("%s: %d  trip_count: %6.0f freq: %6.0f\n",
              _depth == 0 ? "Method" : "Loop", _id, trip_count(), _freq);
   for (int i = 0; i < _depth; i++) tty->print("   ");
-  tty->print("         members:", _id);
+  tty->print("         members:");
   int k = 0;
   for (int i = 0; i < _members.length(); i++) {
     if (k++ >= 6) {


--- old/src/share/vm/oops/methodData.cpp	2014-04-24 11:18:22.000000000 -0400
+++ new/src/share/vm/oops/methodData.cpp	2014-04-24 11:18:22.000000000 -0400
@@ -634,7 +636,7 @@
 }
 
 void ParametersTypeData::print_data_on(outputStream* st, const char* extra) const {
-  st->print("parameter types", extra);
+  st->print("parameter types"); // FIXME extra ignored?
   _parameters.print_data_on(st);
 }
 
---------------------------------------------------------------------

Among the changes/observations:

int64_t and jlong are NOT NOT NOT the same type on all platforms.

I tested the various pragma/macros idioms for clang as far back as 3.1, gcc as far back as 4.2 .

Added some formatting macros:
#define UINT64_FORMAT_X        "%" PRIx64
#define PTR_FORMAT_W(width)   "%" #width PRIxPTR
#define SIZE_FORMAT_XW(width) "%" #width PRIxPTR
#define SIZE_FORMAT_X         "%" PRIxPTR

The garbage collector seems to be a little bit nutty about the uintx type.
Look at some of the command-line options parameters that are uintx,
in many cases a uint8_t would be completely adequate.

The mute-gcc macro:
#define PRAGMA_FORMAT_MUTE_WARNINGS_FOR_GCC _Pragma("GCC diagnostic ignored \"-Wformat\"") _Pragma("GCC diagnostic error \"-Wformat-nonliteral\"") _Pragma("GCC diagnostic error \"-Wformat-security\"")

---------------------------------------------------------------------

+       warning("Failed to reserve shared memory (errno = %d).", errno);
+       warning("Failed to attach shared memory (errno = %d).", err);
+                      _level, (int) MaxBCEAEstimateLevel);
+        tty->print_cr("code size (%d) exceeds MaxBCEAEstimateSize (%d).",
+                      method()->code_size(), (int) MaxBCEAEstimateSize);
+              out->print(" %d %s", (int)(dp_to_di(vdata->dp() + in_bytes(vdata->receiver_offset(i))) / sizeof(intptr_t)), k->name()->as_quoted_ascii());
+              out->print(" %d %s", (int)(dp_to_di(vdata->dp() + in_bytes(vdata->receiver_offset(i))) / sizeof(intptr_t)), k->name()->as_quoted_ascii());
+inline void assert_property(bool b, const char* msg, TRAPS) {
+    tty->print_cr(" #%d %s = %dK (hdr %d%%,  loc %d%%, code %d%%, stub %d%%, [oops %d%%, metadata %d%%, data %d%%, pcs %d%%])",
+                  (int)(total() / K),
+              mark_stack_size, (uintx) 1, MarkStackSizeMax);
+                  MarkStackSize, (uintx) 1, MarkStackSizeMax);
+      (reserved().byte_size()/K), reserved().byte_size());
+      (working_promoted/K), working_promoted);
+      (used_in_bytes()/K), used_in_bytes());
+      (min_gen_size()/K), min_gen_size());
+      (max_contraction/K), max_contraction);
+      (policy->promo_increment(max_contraction)/K),
+        err_msg("MaxTenuringThreshold should be 0 or markOopDesc::max_age + 1, but is %d", (int) MaxTenuringThreshold));
+       (int) active_workers, (int) new_active_workers, (int) prev_active_workers,
+       (int) active_workers_by_JT, (int) active_workers_by_heap_size);
+        "of %d%%", (int) GCTimeLimit);
+        (int) GCTimeLimit, gc_overhead_limit_count());
+    uintx age = 1;
+        desired_survivor_size*oopSize, (uintx) result, MaxTenuringThreshold);
+	    age, sizes[age]*oopSize, total*oopSize);
+        gclog_or_tty->print_cr(
+                           demand,     old_rate,             rate,     new_rate,                 old_desired,                   _desired);
+    st->print_cr("%d not in CP[*]?", i);
+    st->print_cr("%d not in OBJ[*]?", i);
+    st->print(" <MethodHandle of kind %d index at %d>", kind, i2);
+  st->print("parameter types"); // FIXME extra ignored?
+  tty->print("         members:");
+                  (int) (hi-lo+1), nsing, _max_switch_depth, _est_switch_depth);
+                    (int) _verify_full_passes);
+                  (int) _verify_counter, (int) _verify_full_passes);
+     const char * trace_mesg, TRAPS)  {
+      (unsigned int) (MarkStackSize / K), (unsigned int) (MarkStackSizeMax / K));
+    tty->print_cr("ConcGCThreads: %u", (unsigned int) ConcGCThreads);
+       " based on pause goal of %d (ms)", (int) MaxGCPauseMillis);
+      (unsigned int) (MarkStackSize / K), (unsigned int) (MarkStackSizeMax / K));
+    tty->print_cr("ConcGCThreads: %u", (unsigned int) ConcGCThreads);
+inline void Events::log(Thread* thread, const char* format, ...)  {
+  return (intptr_t) p;
+}




More information about the hotspot-dev mailing list