RFR: JDK-8236697 Stack overflow with cyclic hierarchy in class file

[Vicente Romero]

looks sensible to me,

Vicente

On 6/5/20 4:22 AM, Adam Sotona wrote:
> Hi,
> c.s.t.j.code.Types::asSuper causes stack overflow when there is involved a class with cyclic hierarchy.
> Proposed fix (option #2 below) performs minimal necessary cycle detection to avoid stack overflow by breaking the loop, using LOCKED flag as a part of c.s.t.j.code.Types::asSuper execution.
> Proposed test simulates the situation using JCOD.
> Tier 1, 2 and 3 tests passed and performance benchmarks found no significant regression.
>
> webrev: http://cr.openjdk.java.net/~asotona/8236697/webrev.01/
> JBS: https://bugs.openjdk.java.net/browse/JDK-8236697
>
> Thanks,
> Adam
>
>
>> On 28 May 2020, at 16:28, Adam Sotona <adam.sotona at oracle.com> wrote:
>>
>> Hi,
>> I would like to ask for help with decision and review fix of JDK-8236697 Stack overflow with cyclic hierarchy in class file.
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8236697
>>
>> First important information is that the Stack overflow is caused by artificially corrupted library class file.
>> The reproducible case described in the issue can be significantly simplified and I've transformed it into a test case (a part of the patches below).
>>
>> Now I'm considering three possible options:
>>
>> Option #1 -  complete detection and reporting of dependency cycles as a part of c.s.t.j.code.Types::asSuper by call of c.s.t.j.comp.Check::checkNonCyclic method, with positive impact on cycle detection and reporting, with minimal negative impact on performance and with negative impact on EagerInterfaceCompletionTest, which does not expect that by calling asSuper will be updated error status of all ancestors.
>> Here is related patch in webrev: http://cr.openjdk.java.net/~asotona/8236697/webrev.00/
>>
>> Option #2 - minimal necessary cycle detection just to avoid stack overflow, using LOCKED flag as a part of c.s.t.j.code.Types::asSuper execution, without any error reporting, just to avoid erroneous state, with almost no impact on performance and no detected impact on other parts of javac and tests.
>> webrev: http://cr.openjdk.java.net/~asotona/8236697/webrev.01/
>>
>> Option #3 - do not fix it, as artificially corrupted cyclic class file is an extreme case, where Stack overflow from javac might be considered as relevant response
>>
>> I've run Tier 1, 2 and 3 tests for options #1 and #2 and also javac compilation benchmarks comparing javac performance against JDK 15 build 25.
>>
>> I propose to use option #2, however please let me know your opinion and/or review.
>>
>> Thanks,
>> Adam
>>
>>