RFR: JDK-8230776: Javac throws AssertionError in jvm.Gen.visitExec

[Vicente Romero]

Hi Jan,

I agree with your comments, I have uploaded another iteration at [1]. 
The only changes are in PoolReader,

Thanks,
Vicente

[1] http://cr.openjdk.java.net/~vromero/8230776/webrev.02/

On 8/28/20 9:57 AM, Jan Lahoda wrote:
> Hi Vicente,
>
> I think it looks great overall.
>
> Basically my only comment is for PoolReader. Given the performance of 
> this code is potentially important, would it make sense to make the 
> validity checks as fast as possible? E.g. having something like:
> readIfNeeded(int index, BitSet expectedTags) {
>
> ...
> if (!expectedTags.get(currentTag)) {
>     throw reader.badClassFile("unexpected.const.pool.tag.at", 
> tag(index), offset(index));
> }
>
> And having static final pre-created BitSets used to pass into this 
> method?
>
> Thanks,
>     Jan
>
> On 28. 08. 20 0:01, Vicente Romero wrote:
>> Hi Jan,
>>
>> Please review another iteration of the patch at [1], this one is a 
>> bit more complex as it addresses several very related bugs together. 
>> These bugs are [2-6]. The common factor is that javac interrupts 
>> compilation with an exception or assertion error while compiling a 
>> source file that extends a fuzzed class file.
>>
>> Thanks,
>> Vicente
>>
>> [1] http://cr.openjdk.java.net/~vromero/8230776/webrev.01/
>> [2] https://bugs.openjdk.java.net/browse/JDK-8231311
>> [3] https://bugs.openjdk.java.net/browse/JDK-8230964
>> [4] https://bugs.openjdk.java.net/browse/JDK-8230963
>> [5] https://bugs.openjdk.java.net/browse/JDK-8230919
>> [6] https://bugs.openjdk.java.net/browse/JDK-8230776
>>
>>
>> On 8/26/20 4:26 AM, Jan Lahoda wrote:
>>> Hi Vicente,
>>>
>>> I am sorry, but I don't think this is a fix on a good place.
>>>
>>> The root cause here is that the classfile for Test has an invalid 
>>> method descriptor for the Test's constructor (returns float instead 
>>> of void). See JVMS 4.6., descriptor_index. I think it would be much 
>>> better to validate method descriptors, and reject classfiles with 
>>> descriptors not adhering to JVMS. The user would then get a much 
>>> more helpful error, saying something like:
>>> ---
>>> Sub.java:1: error: cannot access Test
>>> class Sub extends Test {}
>>>                   ^
>>>   bad class file: 
>>> /home/jlahoda/src/jdk/jdk/build/linux-x86_64-server-release/nb-jtreg/work/classes/tools/javac/T8230776/AssertionErrorTest.d/Test.class 
>>>
>>>     method descriptor invalid for <init>
>>>     Please remove or make sure it appears in the correct 
>>> subdirectory of the classpath.
>>> 1 error
>>> ---
>>>
>>> What do you think?
>>>
>>> Jan
>>>
>>> On 25. 08. 20 19:23, Vicente Romero wrote:
>>>> Please review fix for [1] at [2]. The fix is ending the compilation 
>>>> at the code generation phase if a statement cant be generated and 
>>>> there is no way for the compilation to continue.
>>>>
>>>> Thanks,
>>>> Vicente
>>>>
>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8230776
>>>> [2] http://cr.openjdk.java.net/~vromero/8230776/webrev.00/
>>