RFR: 8331201: UBSAN enabled build reports on Linux x86_64 runtime error: shift exponent 65 is too large for 64-bit type 'long unsigned int' [v2]

[Afshin Zafari]

On Thu, 9 Jan 2025 06:25:35 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

>> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   removed extra blank lines
>
> I think the best way to fix this specific issue is the way I proposed in the JBS entry. 
> 
> -   _accumulator |= ((fingerprint_t)type << _shift_count);
> -   _shift_count += fp_parameter_feature_size;
> +   if (_param_size < fp_max_size_of_parameters) {
> +     _accumulator |= ((fingerprint_t)type << _shift_count);
> +.    _shift_count += fp_parameter_feature_size;
> +   }
> 
> 
> My reasoning for using the `_param_size < fp_max_size_of_parameters` condition instead of some `_shift_count < 64` is that the later makes it more confusing IMO. The current implementation of fingerprinting uses the `_param_size < fp_max_size_of_parameters`  condition to determine if an overflow fingerprint should be used. (The only way we would currently install a accumulated fingerprint that when this condition is false is if there is a mismatch between `_method->size_of_parameters()` and `_param_size`. But I assume that `!_method || _method->size_of_parameters() == _param_size` is a post condition here).
> 
> Generally UB in the implementation is because we have not through through the design enough. I feel it is rarely the best solution to create a `if (!ub) { do_the_thing(); }` rather than look at the algorithm and find what are we actually trying to do and then adapt the implementation in terms of the algorithm. Which in this case is if  `_param_size < fp_max_size_of_parameters` we use an overflow fingerprint so stop accumulating a value which will never be read.
> 
> 
> I then think we should create a RFE for investigating if there are any `_param_size < fp_max_size_of_parameters` implies overflow fingerprint dependencies or assumptions anywhere else. And with that information in mind, change the algorithm such that the condition for using a overflow fingerprint is not tied to the size of the parameters, but the number of parameters. So we can fingerprint any method with 14 or less parameters, regardless of their types.

Dear @xmas92 and @kimbarrett and @dean-long , please check if new implementation is acceptable.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/22807#issuecomment-2678144650