RFR(M): 8073315: Enable gcc -Wtype-limits and fix upcoming issues.
Kim Barrett
kim.barrett at oracle.com
Thu Mar 12 22:27:05 UTC 2015
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.
More information about the hotspot-dev
mailing list