RFR(M): 8073315: Enable gcc -Wtype-limits and fix upcoming issues.

[Lindenmaier, Goetz]

Hi Kim, Jesper, 

thanks for dealing with this.
I made a new webrev, rebased and including your patch, Kim:
http://cr.openjdk.java.net/~goetz/webrevs/8073315-wUnsign/webrev.07/

Jesper, I don't care whether you push this webrev or one you assembled
yourself - whatever is less trouble for you.

Best regards,
  Goetz.

-----Original Message-----
From: Kim Barrett [mailto:kim.barrett at oracle.com] 
Sent: Thursday, March 12, 2015 11:27 PM
To: Lindenmaier, Goetz
Cc: Jesper Wilhelmsson; hotspot-dev Source Developers
Subject: Re: RFR(M): 8073315: Enable gcc -Wtype-limits and fix upcoming issues.

On Mar 6, 2015, at 4:36 AM, Lindenmaier, Goetz <goetz.lindenmaier at sap.com> wrote:
> 
> Hi Jesper,
> 
> You offered to sponsor this change.  I think it's ok to be pushed.
> Kim and Volker are ok with it, too, and I ran it through our nightly tests.
> These cover quite a range of compilers and platforms.
> 
> This is the final webrev:
> http://cr.openjdk.java.net/~goetz/webrevs/8073315-wUnsign/webrev.06/
> 
> I would appreciate a lot if you could sponsor this!

During JPRT testing we discovered a change like the following is
needed in order to build on 32bit x86 platforms:

diff -r 47e882f3a6bc src/share/vm/runtime/virtualspace.cpp
--- a/src/share/vm/runtime/virtualspace.cpp	Fri Mar 06 13:16:32 2015 -0500
+++ b/src/share/vm/runtime/virtualspace.cpp	Thu Mar 12 18:13:52 2015 -0400
@@ -519,12 +519,13 @@
 
       // Calc address range within we try to attach (range of possible start addresses).
       char *const highest_start = (char *)align_ptr_down(zerobased_max - size, attach_point_alignment);
-      // SS10 and SS12u1 cannot compile "(char *)UnscaledOopHeapMax - size" on solaris sparc 32-bit:
-      // "Cannot use int to initialize char*." Introduce aux variable.
-      char *unscaled_end = (char *)UnscaledOopHeapMax;
-      unscaled_end -= size;
-      char *lowest_start = (size < UnscaledOopHeapMax) ?
-        MAX2(unscaled_end, aligned_heap_base_min_address) : aligned_heap_base_min_address;
+      // Need to be careful about size being guaranteed to be less
+      // than UnscaledOopHeapMax due to type constraints.
+      char *lowest_start = aligned_heap_base_min_address;
+      uint64_t unscaled_end = UnscaledOopHeapMax - size;
+      if (unscaled_end < UnscaledOopHeapMax) { // unscaled_end wrapped if size is large
+        lowest_start = MAX2(lowest_start, (char*)unscaled_end);
+      }
       lowest_start  = (char *)align_ptr_up(lowest_start, attach_point_alignment);
       try_reserve_range(highest_start, lowest_start, attach_point_alignment,
                         aligned_heap_base_min_address, zerobased_max, size, alignment, large);

The problem is that UnscaledOopHeapMax is a uint64_t with a value of
2^32.  On a 32bit platform, with size_t being of the same size as
uint32_t, all possible values for "size" are < UnscaledOopHeapMax.
And that triggers the newly enabled warning.

There are many possible ways to deal with this. I could pretty easily
be convinced to use something else.

Can we get the requisite reviewers for this additional change? And
have it added to the patch / webrev / whatever that will be handed off
to Jesper for pushing?

Also, the webrev.06 doesn't apply entirely cleanly to the current tip
of hs-comp.