bug fix: implicit exception handling

Triple Yang triple.yang at linaro.org
Tue Dec 22 16:48:07 UTC 2015


Hi, all,

our team fix a bug in handling exception, and here is the test case for
the bug:

//////// ExTest.java ////////
public class ExTest
{
   int a;
   public static void main(String[] args)
   {
      try {
        ExTest test = null;
        test.a = 11;
      }
      catch (Exception ex) {
        System.out.println(ex);
      }
   }
}
///////////// end ///////////////

and the bytecodes of method main:

///////////// start //////////////
  public static void main(java.lang.String[]);
    descriptor: ([Ljava/lang/String;)V
    flags: ACC_PUBLIC, ACC_STATIC
    Code:
      stack=2, locals=2, args_size=1
         0: aconst_null
         1: astore_1
         2: aload_1
         3: bipush        11
         5: putfield      #2 // Field a:I
         8: goto          19
        11: astore_1
        12: getstatic     #4 // Field
java/lang/System.out:Ljava/io/PrintStream;
        15: aload_1
        16: invokevirtual #5 // Method
java/io/PrintStream.println:(Ljava/lang/Object;)V
        19: return
      Exception table:
         from    to  target type
            0     8    11   Class java/lang/Exception
//////////// end  ////////////////

1. Bug description:
when interpreting "putfield", a null-pointer exception occurs since "ExTest
test"
is null. After exception handler is figured out in template assembly
generated
by TemplateInterpreterGenerator::generate_throw_exception(), control flow
starts to interpret bytecode 11 astore_1 but pc points to a wrong place.
This
is because rdispatch register is modified when to interpret "putfield" and
could
not be able to be restored in time.

2. Bug analisys:
We state "const Register off   = rdispatch;" in
TemplateTable::putfield_or_static(), and so rdispatch is polluted, and we
restore it in the end of this template. The problem is, in this case,
control
flow is interrupted since a null-pointer load/store instruction, and
rdispatch
is not restored as expected.

3. Bug solution:
option 1:
we restore rdispatch before we begin to interpret the exception handler, and
this works when we restore rdispatch at Interpreter::_throw_exception_entry.

option 2:
we do not pollute rdispatch in putfield_or_static/getfield_or_static
template,
as the comment says: " //FIXME Find a better way than this! ". And that what
we do in the patch below:

/////////// start patch /////////////
diff -r 0e020032c581 src/cpu/aarch32/vm/templateTable_aarch32.cpp
--- a/src/cpu/aarch32/vm/templateTable_aarch32.cpp   Mon Dec 07 21:48:58
2015 +0000
+++ b/src/cpu/aarch32/vm/templateTable_aarch32.cpp   Sun Dec 20 20:12:15
2015 +0800
@@ -2504,7 +2504,7 @@
   const Register cache = r2;
   const Register index = r3;
   const Register obj   = r14;
-  const Register off   = rdispatch; //pop_and_check_object
+  const Register off   = rscratch2; //pop_and_check_object
   const Register flags = r0;
   const Register bc    = r14; // uses same reg as obj, so don't mix them

@@ -2643,8 +2643,6 @@
   // It's really not worth bothering to check whether this field
   // really is volatile in the slow case.
   __ membar(MacroAssembler::LoadLoad | MacroAssembler::LoadStore);
-
-  __ get_dispatch();
 }


@@ -2725,7 +2723,7 @@
   const Register cache = r2;
   const Register index = r0;
   const Register obj   = r2;
-  const Register off   = rdispatch;
+  const Register off   = rscratch2;
   const Register flags = r14;
   const Register bc    = r3;

@@ -2899,8 +2897,6 @@
     __ membar(MacroAssembler::StoreLoad);
     __ bind(notVolatile);
   }
-  //FIXME find a more elegant way!
-  __ get_dispatch();
 }

 void TemplateTable::putfield(int byte_no)
///////// end patch  ///////////////

Attachments are the test case and the patch. We have tested this patch with
jtreg and mauve and see no regression.

Best regards.
-------------- next part --------------
diff -r 0e020032c581 src/cpu/aarch32/vm/templateTable_aarch32.cpp
--- a/src/cpu/aarch32/vm/templateTable_aarch32.cpp	Mon Dec 07 21:48:58 2015 +0000
+++ b/src/cpu/aarch32/vm/templateTable_aarch32.cpp	Sun Dec 20 20:12:15 2015 +0800
@@ -2504,7 +2504,7 @@
   const Register cache = r2;
   const Register index = r3;
   const Register obj   = r14;
-  const Register off   = rdispatch; //pop_and_check_object
+  const Register off   = rscratch2; //pop_and_check_object
   const Register flags = r0;
   const Register bc    = r14; // uses same reg as obj, so don't mix them
 
@@ -2643,8 +2643,6 @@
   // It's really not worth bothering to check whether this field
   // really is volatile in the slow case.
   __ membar(MacroAssembler::LoadLoad | MacroAssembler::LoadStore);
-
-  __ get_dispatch();
 }
 
 
@@ -2725,7 +2723,7 @@
   const Register cache = r2;
   const Register index = r0;
   const Register obj   = r2;
-  const Register off   = rdispatch;
+  const Register off   = rscratch2;
   const Register flags = r14;
   const Register bc    = r3;
 
@@ -2899,8 +2897,6 @@
     __ membar(MacroAssembler::StoreLoad);
     __ bind(notVolatile);
   }
-  //FIXME find a more elegant way!
-  __ get_dispatch();
 }
 
 void TemplateTable::putfield(int byte_no)


More information about the aarch32-port-dev mailing list