[11u] RFR 8255269: Unsigned overflow in g1Policy.cpp

[John Cuthbertson]

Hi William,

Thanks for confirming.

JohnC

> On Nov 4, 2020, at 9:04 AM, Kemper, William <kemperw at amazon.com> wrote:
> 
> G1GCPhaseTimes::OptObjCopy was added in 13 during a larger refactoring effort (https://bugs.openjdk.java.net/browse/JDK-8218668). This enum value doesn't exist in 11 so it's expected that it's not used in the patch for 11.
> 
> Thanks,
> William
> ________________________________________
> From: John Cuthbertson <johnc at azul.com>
> Sent: Wednesday, November 4, 2020 8:51 AM
> To: Hohensee, Paul
> Cc: Kemper, William; jdk-updates-dev at openjdk.java.net
> Subject: RE: [EXTERNAL] [11u] RFR 8255269: Unsigned overflow in g1Policy.cpp
> 
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> Hi William, Paul,
> 
> Does this patch have the same issue as before — the missed 'average_time_ms(G1GCPhaseTimes::OptObjCopy)’ term in the calculation of average_copy_time? Will that be resolved by back porting Paul’s fix?
> 
> JohnC
> 
>> On Nov 4, 2020, at 8:18 AM, Hohensee, Paul <hohensee at amazon.com> wrote:
>> 
>> Thanks, lgtm now. I'll tag the issue.
>> 
>> Paul
>> 
>> On 11/3/20, 5:23 PM, "jdk-updates-dev on behalf of Kemper, William" <jdk-updates-dev-retn at openjdk.java.net on behalf of kemperw at amazon.com> wrote:
>> 
>>   Here's a patch based on jdk13u using the Skara CLI tools:
>> 
>>   git hg-export 7e95c0a5
>>   # HG changeset patch
>>   # User phh
>>   # Date 1603811625 0
>>   #      Tue Oct 15:13:45 2020 +0000
>>   8255269: Unsigned overflow in g1Policy.cpp
>>   Reviewed-by: yan
>>   Contributed-by: William Kemper <kemperw at amazon.com<mailto:kemperw at amazon.com>>
>> 
>>   diff --git a/src/hotspot/share/gc/g1/g1Policy.cpp b/src/hotspot/share/gc/g1/g1Policy.cpp
>>   index 91eceb7375..dec7f57de6 100644
>>   --- a/src/hotspot/share/gc/g1/g1Policy.cpp
>>   +++ b/src/hotspot/share/gc/g1/g1Policy.cpp
>>   @@ -1,5 +1,5 @@
>>    /*
>>   - * Copyright (c) 2001, 2019, Oracle and/or its affiliates. All rights reserved.
>>   + * Copyright (c) 2001, 2020, Oracle and/or its affiliates. All rights reserved.
>>     * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>     *
>>     * This code is free software; you can redistribute it and/or modify it
>>   @@ -692,11 +692,11 @@ void G1Policy::record_collection_pause_end(double pause_time_ms, size_t cards_sc
>>        _analytics->report_rs_length_diff((double) rs_length_diff);
>> 
>>        size_t freed_bytes = heap_used_bytes_before_gc - cur_used_bytes;
>>   -    size_t copied_bytes = _collection_set->bytes_used_before() - freed_bytes;
>>   -    double cost_per_byte_ms = 0.0;
>> 
>>   -    if (copied_bytes > 0) {
>>   -      cost_per_byte_ms = (average_time_ms(G1GCPhaseTimes::ObjCopy) + average_time_ms(G1GCPhaseTimes::OptObjCopy)) / (double) copied_bytes;
>>   +    if (_collection_set->bytes_used_before() > freed_bytes) {
>>   +      size_t copied_bytes = _collection_set->bytes_used_before() - freed_bytes;
>>   +      double average_copy_time = average_time_ms(G1GCPhaseTimes::ObjCopy);
>>   +      double cost_per_byte_ms = average_copy_time / (double) copied_bytes;
>>          _analytics->report_cost_per_byte_ms(cost_per_byte_ms, collector_state()->mark_or_rebuild_in_progress());
>>        }
>> 
>>   I ran the jtreg tests for G1 against the release build on my workstation (x86, Ubuntu 18.04). The results were the same before and after the patch was applied:
>> 
>>   test/hotspot/jtreg/gc/g1
>>   Test results: passed: 56; failed: 4; error: 1
>> 
>>   It doesn't look like the failures and error have anything to do with this change.
>> 
>>   Thanks,
>>   William
>> 
>