From ca46ae5e88965d284a5ebdda29c9e40604812d09 Mon Sep 17 00:00:00 2001 From: Gilles Grospellier Date: Wed, 31 Jan 2024 01:34:55 +0100 Subject: [PATCH] Fix potential 'Floating Point Exception' with speculative execution (#93517) * Fix potential 'Floating Point Exception' with speculative execution * Rewrite the calculation of 'generation_allocator_efficiency' and 'generation_unusable_fragmentation' to remove floating point usage. - Rename 'generation_allocator_efficiency' to 'generation_allocator_efficiency_percent' because now it returns a number between 0 and 100. - Do not use 'generation_allocator_efficiency' in 'generation_unusable_fragmentation' and do direct calculation. * Use 'uint64_t' instead of 'size_t' for computing generation fragmentation. * Use 'size_t' instead of 'uint64_t' for return type. Also add comment to explain why we use integer division instead of floating point division. * Improve readability of the test * Remove cast from 'size_t' to 'int' when using 'generation_allocator_efficiency_percent'. --- src/coreclr/gc/gc.cpp | 10 +++++----- src/coreclr/gc/gcpriv.h | 31 ++++++++++++++++++++----------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 891b6bdbaec1c..0471326c0af5f 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -3327,9 +3327,9 @@ gc_heap::dt_high_frag_p (gc_tuning_point tp, fragmentation_burden = (gen_size ? ((float)fr / (float)gen_size) : 0.0f); ret = (fragmentation_burden > dd_v_fragmentation_burden_limit (dd)); } - dprintf (GTC_LOG, ("h%d: gen%d, frag is %zd, alloc effi: %d%%, unusable frag is %zd, ratio is %d", + dprintf (GTC_LOG, ("h%d: gen%d, frag is %zd, alloc effi: %zu%%, unusable frag is %zd, ratio is %d", heap_number, gen_number, dd_fragmentation (dd), - (int)(100*generation_allocator_efficiency (generation_of (gen_number))), + generation_allocator_efficiency_percent (generation_of (gen_number)), fr, (int)(fragmentation_burden*100))); } break; @@ -22276,7 +22276,7 @@ void gc_heap::gc1() } } - get_gc_data_per_heap()->maxgen_size_info.running_free_list_efficiency = (uint32_t)(generation_allocator_efficiency (generation_of (max_generation)) * 100); + get_gc_data_per_heap()->maxgen_size_info.running_free_list_efficiency = (uint32_t)(generation_allocator_efficiency_percent (generation_of (max_generation))); free_list_info (max_generation, "after computing new dynamic data"); } @@ -33005,9 +33005,9 @@ void gc_heap::plan_phase (int condemned_gen_number) if ((free_list_allocated + rejected_free_space) != 0) free_list_efficiency = (int)(((float) (free_list_allocated) / (float)(free_list_allocated + rejected_free_space)) * (float)100); - int running_free_list_efficiency = (int)(generation_allocator_efficiency(older_gen)*100); + size_t running_free_list_efficiency = generation_allocator_efficiency_percent(older_gen); - dprintf (1, ("gen%d free list alloc effi: %d%%, current effi: %d%%", + dprintf (1, ("gen%d free list alloc effi: %d%%, current effi: %zu%%", older_gen->gen_num, free_list_efficiency, running_free_list_efficiency)); diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index bdd78ec6c4afc..71dc19b9f0c67 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -975,7 +975,7 @@ class generation allocator free_list_allocator; // The following fields are maintained in the older generation we allocate into, and they are only for diagnostics - // except free_list_allocated which is currently used in generation_allocator_efficiency. + // except free_list_allocated which is currently used in generation_allocator_efficiency_percent. // // If we rearrange regions between heaps, we will no longer have valid values for these fields unless we just merge // regions from multiple heaps into one, in which case we can simply combine the values from all heaps. @@ -5055,21 +5055,30 @@ size_t& generation_allocated_since_last_pin (generation* inst) } #endif //FREE_USAGE_STATS +// Return the percentage of efficiency (between 0 and 100) of the allocator. inline -float generation_allocator_efficiency (generation* inst) -{ - if ((generation_free_list_allocated (inst) + generation_free_obj_space (inst)) != 0) - { - return ((float) (generation_free_list_allocated (inst)) / (float)(generation_free_list_allocated (inst) + generation_free_obj_space (inst))); - } - else - return 0; +size_t generation_allocator_efficiency_percent (generation* inst) +{ + // Use integer division to prevent potential floating point exception. + // FPE may occur if we use floating point division because of speculative execution. + uint64_t free_obj_space = generation_free_obj_space (inst); + uint64_t free_list_allocated = generation_free_list_allocated (inst); + if ((free_list_allocated + free_obj_space) == 0) + return 0; + return (size_t)((100 * free_list_allocated) / (free_list_allocated + free_obj_space)); } + inline size_t generation_unusable_fragmentation (generation* inst) { - return (size_t)(generation_free_obj_space (inst) + - (1.0f-generation_allocator_efficiency(inst))*generation_free_list_space (inst)); + // Use integer division to prevent potential floating point exception. + // FPE may occur if we use floating point division because of speculative execution. + uint64_t free_obj_space = generation_free_obj_space (inst); + uint64_t free_list_allocated = generation_free_list_allocated (inst); + uint64_t free_list_space = generation_free_list_space (inst); + if ((free_list_allocated + free_obj_space) == 0) + return 0; + return (size_t)(free_obj_space + (free_obj_space * free_list_space) / (free_list_allocated + free_obj_space)); } #define plug_skew sizeof(ObjHeader)