[9] RFR (S): 8181872: C1: possible overflow when strength reducing integer multiply by constant

[Vladimir Kozlov]

On 6/15/17 9:53 AM, Vladimir Ivanov wrote:
> Thanks, Vladimir!
> 
> Do you agree the fix should go into 9?

Yes. Crush is bad.  Make request.

Vladimir

> 
> Best regards,
> Vladimir Ivanov
> 
> On 6/15/17 7:48 PM, Vladimir Kozlov wrote:
>> Good.
>>
>> Thanks,
>> Vladimir K
>>
>> On 6/15/17 9:47 AM, Vladimir Ivanov wrote:
>>>> Why c1_LIRGenerator.cpp does not have c < max_jint condition?
>>>
>>> That's because it uses c as is: is_power_of_2(c). So, it's enough to
>>> filter out negative values. c < max_jint is added into
>>> strength_reduce_multiply() which does is_power_of_2(c + 1).
>>>
>>>> Is c == 0 case handled in other place for C1?
>>>
>>> is_power_of_2(0) == false, so nothing changes for c == 0 case in
>>> LIRGenerator::arithmetic_op(). Also, I don't think c == 0 is observed
>>> there, because Canonicalizer::do_ArithmeticOp() does "x*0 => 0"
>>> transformation.
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 6/15/17 2:53 AM, Vladimir Ivanov wrote:
>>>>> http://cr.openjdk.java.net/~vlivanov/8181872/webrev.00
>>>>> https://bugs.openjdk.java.net/browse/JDK-8181872
>>>>>
>>>>> LIRGenerator tries to strength reduce integer multiply and replace it
>>>>> with a shift when the constant has 2^n - 1, 2^n, or 2^n + 1 shape.
>>>>>
>>>>> The problem is that there's only c > 0 check, but since signed integer
>>>>> overflow is undefined in C/C++, is_power_of_2(c+1) can become true for
>>>>> c == max_jint.
>>>>>
>>>>> The fix is to always check the constant to be in bounds (0 < c <
>>>>> max_jint) before detecting 2^n - 1, 2^n, 2^n + 1 shapes.
>>>>>
>>>>> The problem is C++ compiler-specific: it was only observed on MacOS
>>>>> with clang 8.1.0 and I wasn't able to reproduce it with 8.0.0 (or
>>>>> earlier). I don't see official jdk9 build platforms to be affected,
>>>>> but it seems safer to fix it in 9.
>>>>>
>>>>> Testing: regression test, JPRT, RBT (hs-tier0-comp).
>>>>>
>>>>> Thanks!
>>>>>
>>>>> Best regards,
>>>>> Vladimir Ivanov