RFR: 8151841: Build needs additional flags to compile with GCC 6
kim.barrett at oracle.com
Tue Mar 22 22:57:10 UTC 2016
> On Mar 16, 2016, at 6:12 AM, Andrew Haley <aph at redhat.com> wrote:
The change got pushed before I had a chance to look at the revised
version, but I wanted to get back to this part of the discussion.
> On 15/03/16 19:15, Kim Barrett wrote:
>>> On Mar 15, 2016, at 12:18 AM, Andrew Hughes <gnu.andrew at redhat.com> wrote:
>> I’ll probably have more to say later; just responding to one point here.
>>>>> 2. A number of optimisations in GCC 6 lead to a broken JVM. We need to
>>>>> add -fno-delete-null-pointer-checks and -fno-lifetime-dse to get a
>>>>> working JVM.
>>>> That's very disturbing!
>>> Andrew Haley (CCed) has more details on the need for these options,
>>> as he diagnosed the reason for the crash, with the help of the GCC
>>> developers. From what I understand of it, it is a case of more
>>> aggressive optimisations in the new GCC running into problems with
>>> code that doesn't strictly conform to the specification and exhibit
>>> undefined behaviour.
>> That is my suspicion too, though without more detail of the failures it’s
>> hard to completely discount the possibility of compiler bugs.
> They weren't compiler bugs: I analyzed the code and I am sure that the
> code in HotSpot isn't valid C++. The -fno-lifetime-dse is because we
> write to a field of an object in operator new before the constructor.
> This is in Node::operator new.
I agree that Node::operator new invokes UB. It's also rather strange
looking code. I was going to say that I'd be surprised if there were
any more like it, but Hotspot code keeps surprising me. (Just out of
curiosity I looked at a few operator new definitions (gosh, there are
a lot of them), and quickly found a couple that on a brief look appear
suspicious, though in different ways from Node.)
>> This comment is quite worrisome.
>> I very strongly suspect that -fno-strict-aliasing is broken in this
>> version of GCC.
>> Is that still thought to be a concern?
> No. I was wrong.
That's what I'd guessed from later discussion in that thread, but
thanks for confirming.
> As I mentioned above, we dereference null pointers a lot. For
> example, Register rax is defined as (RegisterImpl*)0. So, if we do
> something like
> guarantee(reg->is_valid(), "must be");
> if (reg == rax)
> GCC is quite within its rights to delete the call to "stuff". And it
I can easily imagine we dereference pointers and later assert they are
non-NULL. For example, I expect that can happen a lot where some
function includes a non-NULL "sanity" check. I would expect that
dereference followed by functional code for the NULL case would be
more rare, though again not always incorrect.
I was surprised by the assertion that we dereference NULL pointers a
lot; I would have expected doing so to result in crashes. But then I
looked at the register code you referred to. Yikes!
My concern with disabling optimizations is two-fold. Of course,
there's the possibility of slower code as a result. But there is also
the problem of being outside the well-travelled lane of compiler
configuration, with an associated chance of encountering previously
unknown bugs in the compiler. I think that's particularly important
for backporting, which is why I wasn't keen on applying these options
with earlier compiler versions; doing so would be changing from
configurations that have been in use for a while to something new.
> As I said, I would very much like to clean this stuff up, but I'd need
> support from the HotSpot team, and at the moment I feel that this is
I can't speak for the whole team, but I'm taking these kinds of issues
seriously and will offer what support I can. I know we (including,
perhaps especially, me) pushed back on the initial patch for 8145096,
but that was just part of the review process, and I think the final
changes were better as a result.
More information about the hotspot-dev