Re: java/5794: gcj fails to verify .class file generated by Sun JDK 1.4 compiler

[jmr at ugcs dot caltech dot edu (Jesse Rosenstock)]

I originally sent this to gcc-patches by accident, so I'm resending it here.
Sorry if you get it twice.

http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=5794

This problem bit me yesterday, and I want to compile something I don't
have the source for, so I investigated it.  

As previously discussed, I commented out the check that the exception
handler target is outside of the range of the code the exception handler
is for.  This is not dangerous, the worst it could cause is an infinite loop.

Then I got the error:
Foo.java:6: error: verification error at PC=11
Foo.java:6: error: stack overflow

Here's the disassembled bytecode (javap -verbose -c Foo, with irrelevant
parts deleted):

Compiled from Foo.java
public class Foo extends java.lang.Object {
    public int invoke();
        /* Stack=1, Locals=4, Args_size=1 */
}

Method int invoke()
   0 iconst_1
   1 istore_1
   2 jsr 13
   5 iload_1
   6 ireturn
   7 astore_2
   8 jsr 13
  11 aload_2
  12 athrow
  13 astore_3
  14 iconst_2
  15 ireturn
Exception table:
   from   to  target type
     0     5     7   any
     7    11     7   any

Note that PC=8 is a jsr to PC=13, but that block ends at PC=15 with an
ireturn, which returns from the whole method, so PC=11 isn't reachable.
However, if you didn't know that, you'd expect there to be one thing
on the operand stack when the jsr did return, then the aload_2 at PC=11
would push another, exceeding the Stack=1 attribute.  Hence the error.

According to my reading of
http://java.sun.com/docs/books/vmspec/2nd-edition/html/ClassFile.doc.html#9801
the instructions after the jsr's should never be verified.

    3. Determine the instructions that can follow the current instruction.
       Successor instructions can be one of the following:

        * The next instruction, if the current instruction is not an
          unconditional control transfer instruction (for instance goto,
          return, or athrow). Verification fails if it is possible to
          "fall off" the last instruction of the method.

        * The target(s) of a conditional or unconditional branch or switch.

        * Any exception handlers for this instruction.

PC=8 is an unconditional jump, so PC=11 is not a successor according the the
first bullet.   It is also not the target of any conditional or
unconditional branch or switch (because ireturn was used instead of
ret), so it is not a successor according the the second bullet. It is
not the start of any exception handler.  PC=11 should never be analyzed,
but it is.

A patch follows, but I'm not sure of the patch.  (I think) I changed the
OPCODE_jsr to only push the successor instruction if a ret instruction
for the target has been found.  It is similar to the code after the comment
              /* FIXME: If we exit a subroutine via a throw, we might
                 have returned to an earlier caller.  Obviously a
                 "ret" can only return one level, but a throw may
                 return many levels.*/
However that comment troubles me.  I'm not sure what it's worrying about,
but for some interpretation, an ireturn can return many levels, too.

After patching, this example works fine, even correctly warning about the
unreachable bytecode:
; gcj Foo.class -c
Foo.java: In class `Foo':
Foo.java: In method `Foo.invoke()':
Foo.java:6: warning: unreachable bytecode from 5 to before 7
Foo.java:8: warning: unreachable bytecode from 11 to before 13

In case the patch is ok, a ChangeLog entry:

2002-09-03  Jesse Rosenstock  <jmr@ugcs.caltech.edu>

        For PR java/5794:
        * verify.c (verify_jvm_instructions): Don't require that the
        exception handler target doesn't overlap the range it's guarding.
        For OPCODE_jsr, only push the return label if a ret instruction
        for the jsr has been reached.

--- verify.c    4 Jun 2002 20:32:08 -0000       1.48
+++ verify.c    4 Sep 2002 19:19:23 -0000
@@ -470,9 +470,8 @@
 
       if (start_pc < 0 || start_pc >= length
          || end_pc < 0 || end_pc > length || start_pc >= end_pc
          || handler_pc < 0 || handler_pc >= length
-         || (handler_pc >= start_pc && handler_pc < end_pc)
          || ! (instruction_bits [start_pc] & BCODE_INSTRUCTION_START)
          || (end_pc < length &&
             ! (instruction_bits [end_pc] & BCODE_INSTRUCTION_START))
          || ! (instruction_bits [handler_pc] & BCODE_INSTRUCTION_START))
@@ -1323,9 +1322,10 @@
                    if (TREE_VEC_ELT (return_map, len) != TYPE_UNUSED)
                      type_map[len] = TREE_VEC_ELT (return_map, len);
                  }
                current_subr = LABEL_SUBR_CONTEXT (target);
-               PUSH_PENDING (return_label);
+               if (RETURN_MAP_ADJUSTED (return_map))
+                 PUSH_PENDING (return_label);
              }
 
            INVALIDATE_PC;
          }