RFR 8050485: super() in a try block in a ctor causes VerifyError

[harold seigel]

Hi,

Please review this latest webrev for fixing 8050485.  The webrev is at: 
http://cr.openjdk.java.net/~hseigel/bug_8050485_3/

This new webrev has two major changes.  The first change is for 
branches.  For both conditional forward and unconditional forward 
branches, the implementation now parses both paths.  Because the parsing 
starts at the beginning of each catch clause, and parses both paths at 
branches, backward branches can be ignored.  In the following example, 
the target of the branch is 10.  The implementation first parses the 
bytecodes between 2 and 10 and then, if needed, parses the bytecodes 
from 10 onward.  In this case, it would detect the return at 2 and throw 
a VerifyError before parsing the bytecodes from 10 on.

    1   goto 10
    2   return
    3   ifeq 2
    ...
    10

In this example the branch target is 3, the code would parse the the 
bytecodes between 1 and 3.  It would see the athrow but would also parse 
the bytecodes from 3 onward to make sure that that path also ends in an 
athrow.

    1  ifeql 3
    2 athrow
    3  ...

One advantage of this approach is that it handles the following case.  
The TRY block at 5-6 does an unconditional goto around the catch clause 
to the throw at 10.  But, since both paths at the goto are parsed, the 
return at 8 is detected and VerifyError is thrown. This enables the 
implementation to avoid checking for exception handlers for every 
bytecode in a catch handler.

     1 public TryNestedTry2() {
     2    try {
     3        super();
     4    } catch(Exception e) {
     5        try {
     6            xxxx();
     7        } catch(Exception f) {
     8            return;
     9        }
     10       throw e;
     11   }
     12 }

The second major change adds support for catch clauses containing TRY 
blocks.  When the implementation finds an athrow it checks to see if the 
athrow is contained in TRY blocks.  If it is, then it pushes the 
starting bytecodes for those TRY blocks' catch handlers onto a separate 
stack.  The bytecodes in those catch handlers are then parsed after all 
normal paths have been parsed.  For example, it would return VerifyError 
for this case:

     static int n = 1;
     public TryNestedTryOK() {
         try {
             super();
         } catch(java.lang.VerifyError g) {
             try {
                 throw g;
             } catch(Exception h) {
                 n++;
             }
         }

Thanks, Harold


On 7/30/2014 4:53 PM, harold seigel wrote:
> Hi Dean,
>
> Thanks for finding this problem.  I will look into issues with 
> backward branches.  Perhaps, if I scan every forward path, backward 
> branches can be ignored.
>
> Harold
>
> On 7/29/2014 6:28 PM, Dean Long wrote:
>> Checking the algorithm, it looks like there is a change of an 
>> infinite loop:
>>
>> L0:
>> goto L2           // 2: set_interval(L2, code_length);
>> start_bc_offset:
>> L2:
>> ifle L0           // 1: set_interval(L0, L2)
>> L3:
>> code_length:
>>
>> dl
>>
>> On 7/29/2014 9:47 AM, harold seigel wrote:
>>> Hi,
>>>
>>> Please review this updated webrev for bug 8050485.  This update does 
>>> not use recursion.  Instead, it uses a GrowableArray to push and pop 
>>> bytecode intervals when parsing if*, goto*, tableswitch, 
>>> lookupswitch and athrow bytecodes. This updated webrev was tested 
>>> using the same tests as the previous weberv.
>>>
>>> The updated webrev is at: 
>>> http://cr.openjdk.java.net/~hseigel/bug_8050485_2/
>>>
>>> Thanks, Harold
>>>
>>> On 7/24/2014 1:55 PM, harold seigel wrote:
>>>> Hi,
>>>>
>>>> Please review this verifier fix for bug 8050485.
>>>>
>>>> The fix for JDK-8035119 
>>>> <https://bugs.openjdk.java.net/browse/JDK-8035119> broke existing 
>>>> tools, such as NetBeans Profiler, that generate bytecodes which 
>>>> place TRY blocks around constructor calls to super() and this().  
>>>> The purpose of that fix was to prevent exception handlers from 
>>>> handling exceptions thrown from super() and this(), and returning 
>>>> malformed objects to the callers of the constructors.
>>>>
>>>> The NB Profiler prevents malformed objects from being returned to 
>>>> the constructors' callers by having the exception handlers re-throw 
>>>> the exceptions.
>>>>
>>>> The purpose of this fix is to allow a TRY block around a 
>>>> constructor's call to super() and this(), provided that all code 
>>>> paths in the TRY block's exception handlers terminate with a throw. 
>>>> This prevents malformed objects from being returned and does not 
>>>> break tools like NB Profiler.
>>>>
>>>> The fix works by parsing the bytecodes inside of the exception 
>>>> handlers, making sure that all code paths end in an 'athrow' 
>>>> bytecode.  Otherwise, it throws a VerifyError exception.  This 
>>>> parsing is only done when the verifier detects a constructor's call 
>>>> to super() or this() from inside of a TRY block.
>>>>
>>>> Bug:  https://bugs.openjdk.java.net/browse/JDK-8050485
>>>> Open webrev: http://cr.openjdk.java.net/~hseigel/bug_8050485/
>>>>
>>>> The fix was tested with the JCK lang, vm, and api/java_lang tests, 
>>>> the UTE verifier and quick tests, the JTREG hotspot tests, and 
>>>> additional tests with constructor calls to super() and this() from 
>>>> inside of a TRY block, including one provided by NB Profiler.
>>>>
>>>> Thanks, Harold
>>>
>>
>