harmony-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ge...@apache.org
Subject svn commit: r446867 - in /incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet: cg.cpp cg.h cg_fld_arr.cpp cg_instr.cpp cg_meth.cpp jet.cpp val.h
Date Sat, 16 Sep 2006 12:35:40 GMT
Author: geirm
Date: Sat Sep 16 05:35:39 2006
New Revision: 446867

URL: http://svn.apache.org/viewvc?view=rev&rev=446867
Log:
HARMONY-1433

      Watchpoint implementation was fixed to restore all scratch registers correctly.
        Corresponding methods were also refactored.
    
    Short scenario to reproduce the bug:
            JVMTI agent should request watchpoint capabilities (can_generate_field_access_events,
            can_generate_field_modification_events) and DRLVM will throw NPE due to corrupted
            stack after notification code. 

Note that test_jthread_get_owned_monitors failed once, but after two retried didn't reappear.
 I 
suspect it's something else, so we'll move forward.  Wanted to note it though.

Ubuntu, c-unit and smoke.



Modified:
    incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg.cpp
    incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg.h
    incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_fld_arr.cpp
    incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_instr.cpp
    incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_meth.cpp
    incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/jet.cpp
    incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/val.h

Modified: incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg.cpp
URL: http://svn.apache.org/viewvc/incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg.cpp?view=diff&rev=446867&r1=446866&r2=446867
==============================================================================
--- incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg.cpp (original)
+++ incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg.cpp Sat Sep 16 05:35:39 2006
@@ -670,6 +670,7 @@
     }
     // 2. 
     vpark();
+    gen_gc_stack(-1, true);
     // 3.
     va_list valist;
     va_start(valist, idx);

Modified: incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg.h
URL: http://svn.apache.org/viewvc/incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg.h?view=diff&rev=446867&r1=446866&r2=446867
==============================================================================
--- incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg.h (original)
+++ incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg.h Sat Sep 16 05:35:39 2006
@@ -181,6 +181,35 @@
     void do_field_op(JavaByteCodes op, jtype jt, Field_Handle fld);
 
     /**
+    * @brief Generates modification watchpoints if VM need it.
+    *
+    * @param jt - field type.
+    * @param fld - field handle.
+    */
+    void gen_modification_watchpoint(JavaByteCodes opcode, jtype jt, Field_Handle fld);
+
+    /**
+    * @brief Generates access watchpoints if VM need it.
+    *
+    * @param jt - field type.
+    * @param fld - field handle.
+    */
+    void CodeGen::gen_access_watchpoint(JavaByteCodes opcode, jtype jt, Field_Handle fld);
+
+    /**
+    * @brief Restore all scratch registers and operand stack state
+    *
+    * @param saveBB - pointer to operand stack state.
+    */
+    void pop_all_state(BBState* saveBB);
+    /**
+    * @brief Save all scratch registers and operand stack state
+    *
+    * @param saveBB - pointer to operand stack state.
+    */
+    void push_all_state(BBState* saveBB);
+
+    /**
      * @brief Generates code for INVOKE instructions.
      */
     void gen_invoke(JavaByteCodes opcod, Method_Handle meth, 

Modified: incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_fld_arr.cpp
URL: http://svn.apache.org/viewvc/incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_fld_arr.cpp?view=diff&rev=446867&r1=446866&r2=446867
==============================================================================
--- incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_fld_arr.cpp (original)
+++ incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_fld_arr.cpp Sat Sep 16 05:35:39
2006
@@ -232,6 +232,10 @@
         gen_call_throw(ci_helper_linkerr, rt_helper_throw_linking_exc, 0,
                        m_klass, jinst.op0, jinst.opcode);
     }
+
+    if (!get && compilation_params.exe_notify_field_modification)  {
+        gen_modification_watchpoint(opcode, jt, fld);
+    }
     
     Opnd where;
     if (field_op) {
@@ -253,47 +257,12 @@
     // compilation, without access to g_refs_squeeze in runtime.
     assert(is_ia32() || g_refs_squeeze);
     
-    if (get) {
-        if (compilation_params.exe_notify_field_access) {
-            // Check whether VM need access notifications
-
-            char* fld_tr_add;
-            char fld_tr_mask;
-            field_get_track_access_flag(fld, &fld_tr_add, &fld_tr_mask);
-
-            Val fld_track_mask((int)fld_tr_mask);
-
-            AR ar = valloc(jobj);
-            movp(ar, (void*)fld_tr_add);
-            Opnd fld_track_opnd(i32, ar, 0);
-            rlock(fld_track_opnd);
-
-            alu(alu_test, fld_track_opnd, fld_track_mask.as_opnd());
-            runlock(fld_track_opnd);
-
-            unsigned br_off = br(z, 0, 0, taken);
-
-             //JVMTI helper takes field handle, method handle, byte code location, pointer
-             //to reference for fields or NULL for statics
-
-            const CallSig cs_ti_faccess(CCONV_HELPERS, jobj, jobj, i64, jobj);
-            rlock(cs_ti_faccess);
-            Val vlocation((jlong)m_pc);
 
-            Val vfield(jobj, fld);
-            Val vmeth(jobj, m_method);
-            Val vobject =  Val(jobj, NULL);
-
-            if (field_op) {
-                vobject = vstack(0);
-            }
-            
-            gen_args(cs_ti_faccess, 0, &vfield, &vmeth, &vlocation, &vobject);
-            gen_call_vm(cs_ti_faccess, rt_helper_ti_field_access, cs_ti_faccess.count());
-            runlock(cs_ti_faccess);
-
-            patch(br_off, ip());
-        }// end if (compilation_params.exe_notify_field_access) 
+    if (get && compilation_params.exe_notify_field_access) {
+        gen_access_watchpoint(opcode, jt, fld);
+    }
+    
+    if (get) {
 
         if (field_op) {
             // pop out ref
@@ -342,69 +311,6 @@
     } // if (get)
     
     vunref(jt);
-
-    if (compilation_params.exe_notify_field_modification)  {
-        // Check whether VM need access notifications
-
-        char* fld_tr_add;
-        char fld_tr_mask;
-        field_get_track_modification_flag(fld, &fld_tr_add, &fld_tr_mask);
-
-        Val fld_track_mask((int)fld_tr_mask);
-
-        AR ar = valloc(jobj);
-        movp(ar, (void*)fld_tr_add);
-        Opnd fld_track_opnd(i32, ar, 0);
-        rlock(fld_track_opnd);
-
-        alu(alu_test, fld_track_opnd, fld_track_mask.as_opnd());
-        runlock(fld_track_opnd);
-
-        unsigned br_off = br(z, 0, 0, taken);
-
-        //JVMTI helper takes field handle, method handle, byte code location, pointer
-        //to reference for fields or NULL for statics, pointer to field value
-
-        Val fieldVal;
-        Val fieldValPtr = Val(jobj, valloc(jobj));
-        rlock(fieldValPtr);
-        int st_depth = field_op ? 0 :-1;
-
-        if (jt != jvoid) {
-            // Make sure the top item is on the memory
-            vswap(st_depth + 1);
-            if (is_big(jt)) {
-                vswap(st_depth + 2);
-            }
-            const Val& s = vstack(st_depth + 1);
-            assert(s.is_mem());
-            lea(fieldValPtr.as_opnd(), s.as_opnd());
-        }
-        else {
-            Opnd stackTop(jobj, m_base, voff(m_stack.unused()));
-            lea(fieldValPtr.as_opnd(), stackTop);
-        }
-        runlock(fieldValPtr);
-
-        const CallSig cs_ti_fmodif(CCONV_HELPERS, jobj, jobj, i64, jobj, jobj);
-        rlock(cs_ti_fmodif);
-        Val vlocation((jlong)m_pc);
-
-        Val vfield(jobj, fld);
-        Val vmeth(jobj, m_method);
-        Val vobject =  Val(jobj, NULL);
-
-        if (field_op) {
-            vobject = vstack(st_depth);
-        }
-        
-        gen_args(cs_ti_fmodif, 0, &vfield, &vmeth, &vlocation, &vobject,
&fieldValPtr);
-        gen_call_vm(cs_ti_fmodif, rt_helper_ti_field_modification, cs_ti_fmodif.count());
-        runlock(cs_ti_fmodif);
-
-        patch(br_off, ip());
-    }// end if (compilation_params.exe_notify_field_modification) 
-
 
     if (!is_ia32() && g_refs_squeeze && jt == jobj && vis_imm(0))
{
         const Val& s = m_jframe->dip(0);

Modified: incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_instr.cpp
URL: http://svn.apache.org/viewvc/incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_instr.cpp?view=diff&rev=446867&r1=446866&r2=446867
==============================================================================
--- incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_instr.cpp (original)
+++ incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_instr.cpp Sat Sep 16 05:35:39
2006
@@ -56,7 +56,6 @@
         }
         return;
     }
-
     m_bbstate->seen_gcpt = true;
     // On Windows we could use a bit, but tricky way - we know about VM 
     // internals and we know how Windows manages TIB, and thus we can get a 
@@ -74,6 +73,227 @@
     patch(br_off, ip());
 }
 
+void CodeGen::gen_modification_watchpoint(JavaByteCodes opcode, jtype jt, Field_Handle fld)
{
+
+    unsigned ref_depth = is_wide(jt) ? 2 : 1;
+    bool field_op = (opcode == OPCODE_PUTFIELD) ? true : false;
+
+    // Check whether VM need modification notifications
+
+    char* fld_tr_add;
+    char fld_tr_mask;
+    field_get_track_modification_flag(fld, &fld_tr_add, &fld_tr_mask);
+
+    Val fld_track_mask((int)fld_tr_mask);
+
+    AR fld_trackAr = valloc(jobj);
+    movp(fld_trackAr, (void*)fld_tr_add);
+    Opnd fld_track_opnd(i32, fld_trackAr, 0);
+    //mov(fld_trackAr, Opnd(0xFFFFFFFF)); // Emulation to check access flag enabled
+    //Opnd fld_track_opnd(fld_trackAr);
+    
+    rlock(fld_track_opnd);
+
+    alu(alu_test, fld_track_opnd, fld_track_mask.as_opnd());
+    runlock(fld_track_opnd);
+
+    unsigned br_off = br(z, 0, 0, taken);
+    
+    // Store all scratch registers and operand stack state
+    BBState saveBB;
+    push_all_state(&saveBB);
+
+    //JVMTI helper takes field handle, method handle, byte code location, pointer
+    //to reference for fields or NULL for statics, pointer to field value
+
+    AR fieldValBaseAr = valloc(jobj);
+    Val fieldValPtr = Val(jobj, fieldValBaseAr);
+    rlock(fieldValPtr);
+    
+
+    if (jt != jvoid) {
+        // Make sure the value item is on the memory
+        vswap(0);
+        if (is_big(jt)) {
+            vswap(1);
+        }
+        const Val& s = vstack(0);
+        assert(s.is_mem());
+        lea(fieldValPtr.as_opnd(), s.as_opnd());
+    } else {
+        Opnd stackTop(jobj, m_base, voff(m_stack.unused()));
+        lea(fieldValPtr.as_opnd(), stackTop);
+    }
+    runlock(fieldValPtr);
+
+#ifndef _EM64T_
+    // Workaround since do_mov do not put jlong on stack in gen_args on ia32
+    const CallSig cs_ti_fmodif(CCONV_HELPERS, jobj, jobj, i32, i32, jobj, jobj);
+    Val vlocation((jlong)m_pc);
+    Val vlocationHi((jlong)0);
+#else
+    const CallSig cs_ti_fmodif(CCONV_HELPERS, jobj, jobj, i64, jobj, jobj);
+    Val vlocation((jlong)m_pc);
+#endif
+
+    Val vfield(jobj, fld);
+    Val vmeth(jobj, m_method);
+    Val vobject =  Val(jobj, NULL);
+
+    if (field_op) {
+        vobject = vstack(ref_depth);
+    }
+#ifndef _EM64T_
+    // Workaround since do_mov do not put jlong on stack in gen_args on ia32
+    gen_args(cs_ti_fmodif, 0, &vfield, &vmeth, &vlocationHi, &vlocation,
&vobject, &fieldValPtr);
+#else
+    gen_args(cs_ti_fmodif, 0, &vfield, &vmeth, &vlocation, &vobject, &fieldValPtr);
+#endif
+
+    // 2. Park all locals and operand stack
+    vpark();
+    // Store gc info
+    gen_gc_stack(-1, true);
+
+    // 3. Call VM
+    rlock(cs_ti_fmodif);
+    AR gr = valloc(jobj);
+    call( is_set(DBG_CHECK_STACK), gr, rt_helper_ti_field_modification, cs_ti_fmodif, cs_ti_fmodif.count());
+    runlock(cs_ti_fmodif);
+    
+    //Restore operand stack state and scratch registers
+    pop_all_state(&saveBB);
+
+    patch(br_off, ip());
+}
+
+
+void CodeGen::gen_access_watchpoint(JavaByteCodes opcode, jtype jt, Field_Handle fld) {
+
+    bool field_op = (opcode == OPCODE_GETFIELD) ? true : false;
+
+    // Check whether VM need access notifications
+    
+    char* fld_tr_add;
+    char fld_tr_mask;
+    field_get_track_access_flag(fld, &fld_tr_add, &fld_tr_mask);
 
+    Val fld_track_mask((int)fld_tr_mask);
+
+    AR fld_trackAr = valloc(jobj);
+    rlock(fld_trackAr);
+    
+    //mov(fld_trackAr, Opnd(0xFFFFFFFF)); // Emulation to check access flag enabled
+    //Opnd fld_track_opnd(fld_trackAr);
+    movp(fld_trackAr, (void*)fld_tr_add);
+    Opnd fld_track_opnd(i32, fld_trackAr, 0);
+    alu(alu_test, fld_track_opnd, fld_track_mask.as_opnd());
+
+    runlock(fld_trackAr);
+
+    unsigned br_off = br(z, 0, 0, taken);
+
+    // Store all scratch registers and operand stack state
+    BBState saveBB;
+    push_all_state(&saveBB);
+
+
+    //JVMTI helper takes field handle, method handle, byte code location, pointer
+    //to reference for fields or NULL for statics
+
+#ifndef _EM64T_
+    // Workaround since do_mov do not put jlong on stack in gen_args on ia32
+    const CallSig cs_ti_faccess(CCONV_HELPERS, jobj, jobj, i32, i32, jobj);
+    Val vlocation((jlong)m_pc);
+    Val vlocationHi((jlong)0);
+#else
+    const CallSig cs_ti_faccess(CCONV_HELPERS, jobj, jobj, i64, jobj);
+    Val vlocation((jlong)m_pc);
+#endif
+    rlock(cs_ti_faccess);
+
+
+    Val vfield(jobj, fld);
+    Val vmeth(jobj, m_method);
+    Val vobject =  Val(jobj, NULL);
+
+    if (field_op) {
+        vobject = vstack(0);
+    }
+    
+#ifndef _EM64T_
+    // Workaround since do_mov do not put jlong on stack in gen_args on ia32
+    gen_args(cs_ti_faccess, 0, &vfield, &vmeth, &vlocationHi, &vlocation,
&vobject);
+#else
+    gen_args(cs_ti_faccess, 0, &vfield, &vmeth, &vlocation, &vobject);
+#endif
+
+    // 2. Park all locals and operand stack
+    vpark();
+    // Store gc info
+    gen_gc_stack(-1, true);
+
+    // 3. Call VM
+    rlock(cs_ti_faccess);
+    AR gr = valloc(jobj);
+    call( is_set(DBG_CHECK_STACK), gr, rt_helper_ti_field_access, cs_ti_faccess, cs_ti_faccess.count());
+    runlock(cs_ti_faccess);
+
+    //Restore operand stack state and scratch registers
+    pop_all_state(&saveBB);
+
+
+    patch(br_off, ip());
+}
+
+
+void CodeGen::push_all_state(BBState *saveBB){
+    *saveBB = *m_bbstate;
+    // 1. store scratch registers in a secret place
+    // 2. park everything
+    // 3. call whatever
+    // 4. restore scratch regs from the secret place
+    // 5. restore the state for callee-save registers
+    //-----------------------------------------------
+    // 1. 
+    bool saveScratch = true;
+    for (unsigned i=0; i<ar_num; i++) {
+        AR ar = _ar(i);
+        if (is_callee_save(ar)) continue;
+        if (saveScratch && rrefs(ar) != 0) {
+            jtype jt = is_f(ar) ? dbl64 : jobj;
+            Opnd mem(jt, m_base, voff(m_stack.spill(ar)));
+            Opnd reg(jt, ar);
+            mov(mem, reg);
+        }
+        if (rlocks(ar) != 0) {
+            runlock(ar, true);
+        }
+    }
+}
+
+void CodeGen::pop_all_state(BBState* saveBB) {
+    bool saveScratch = true;
+    // 4.
+    *m_bbstate = *saveBB;
+    // restore the registers state
+    for (unsigned i=0; saveScratch && i<ar_num; i++) {
+        AR ar = _ar(i);
+        if (is_callee_save(ar)) continue;
+        if (rrefs(ar) != 0) {
+            jtype jt = is_f(ar) ? dbl64 : jobj;
+            Opnd mem(jt, m_base, voff(m_stack.spill(ar)));
+            Opnd reg(jt, ar);
+            mov(reg, mem);
+            }
+        }
+// 5. 
+// Actually nothing to do here.
+// If we had a local var on register before, then it's still on the reg
+// If we had the var with static assignment which was in memory, before,
+// then the memory was not corrupted.
+// So, just nothing to do with callee-save regs
+//
+}
 
 }}; // ~namespace Jitrino::Jet

Modified: incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_meth.cpp
URL: http://svn.apache.org/viewvc/incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_meth.cpp?view=diff&rev=446867&r1=446866&r2=446867
==============================================================================
--- incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_meth.cpp (original)
+++ incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/cg_meth.cpp Sat Sep 16 05:35:39
2006
@@ -46,8 +46,8 @@
 static CallSig cs_mon(CCONV_HELPERS, jobj);
 
 
-void Compiler::gen_prolog(void){
-    if (is_set(DBG_TRACE_CG)) {    
+void Compiler::gen_prolog(void) {
+    if (is_set(DBG_TRACE_CG)) {
         dbg(";; ========================================================\n");
         dbg(";; Prolog: max_stack=%d, num_locals=%d, in_slots=%d\n", 
                 m_stack.get_max_stack(), 

Modified: incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/jet.cpp
URL: http://svn.apache.org/viewvc/incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/jet.cpp?view=diff&rev=446867&r1=446866&r2=446867
==============================================================================
--- incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/jet.cpp (original)
+++ incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/jet.cpp Sat Sep 16 05:35:39
2006
@@ -485,8 +485,8 @@
         true,  // exe_notify_method_entry
         true,  // exe_notify_method_exit
         
-        false, // exe_notify_field_access
-        false, // exe_notify_field_modification 
+        true, // exe_notify_field_access
+        true, // exe_notify_field_modification 
         false, // exe_notify_exception_throw
         false, // exe_notify_exception_catch
         false, // exe_notify_monitor_enter

Modified: incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/val.h
URL: http://svn.apache.org/viewvc/incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/val.h?view=diff&rev=446867&r1=446866&r2=446867
==============================================================================
--- incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/val.h (original)
+++ incubator/harmony/enhanced/drlvm/trunk/vm/jitrino/src/jet/val.h Sat Sep 16 05:35:39 2006
@@ -272,8 +272,65 @@
      */
     bool    survive_calls(void) const { return m_surviveCalls; };
     /**
-     * Converts Val into memory reference.
-     */
+    * Converts Val into operand.
+    */
+    void    to_opnd(Opnd& op) {
+
+            m_kind = op.kind();
+            if (op.is_mem()) {
+                m_base = op.base();
+                m_disp = op.disp();
+                m_index = op.index();
+                m_scale = op.scale();
+            } else if (op.is_reg()){
+                m_reg = op.reg();
+            } else if (op.jt() <= i32) {
+                m_lval = op.ival();
+                m_surviveCalls = true;
+            } else if (op.jt() == jobj) {
+                m_pval = (const void*)op.lval();
+            } else if (op.jt() == flt32) {
+                m_fval = (float)op.ival();
+            } else if (op.jt() == dbl64) {
+                m_dval = (double)op.lval();
+            }  else {
+                assert(op.jt() == i64);
+                m_lval = op.lval();
+            }
+        }
+    /**
+    * Converts Val into operand.
+    */
+    void    to_val(Val& value) {
+            m_jt = value.jt();
+            m_kind = value.kind();
+            m_surviveCalls = value.get_survive_calls();
+            m_caddr = value.caddr();
+            m_attrs = value.attrs();
+
+            if (value.is_mem()) {
+                m_base = value.base();
+                m_disp = value.disp();
+                m_index = value.index();
+                m_scale = value.scale();
+            } else if (value.is_reg()){
+                m_reg = value.reg();
+            } else if (value.jt() <= i32) {
+                m_lval = value.ival();
+            } else if (value.jt() == jobj) {
+                m_pval = value.pval();
+            } else if (value.jt() == flt32) {
+                m_fval = value.fval();
+            } else if (value.jt() == dbl64) {
+                m_dval = value.dval();
+            }  else {
+                assert(value.jt() == i64);
+                m_lval = value.lval();
+            }
+        }
+    /**
+    * Converts Val into memory reference.
+    */
     void    to_mem(AR base, int disp, AR index = ar_x, unsigned scale=0)
     {
         m_kind = opnd_mem;



Mime
View raw message