]> git.ipfire.org Git - thirdparty/valgrind.git/commitdiff
In vg_SP_update_pass (stack-pointer-change analysis code), correctly
authorJulian Seward <jseward@acm.org>
Sun, 11 Nov 2007 18:56:39 +0000 (18:56 +0000)
committerJulian Seward <jseward@acm.org>
Sun, 11 Nov 2007 18:56:39 +0000 (18:56 +0000)
handle partial updates of SP.  Tricky.  This fixes #152022.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@7147

coregrind/m_translate.c

index f3fadea762b95d2046795f81bc7b872904efacf2..f306b31de7d41d0c7e06a6de48e530f02a03bacc 100644 (file)
@@ -206,6 +206,9 @@ static void update_SP_aliases(Long delta)
    If all the changes to SP leading up to a PUT(SP) are by known, small
    constants, we can do a specific call to eg. new_mem_stack_4, otherwise
    we fall back to the case that handles an unknown SP change.
+
+   There is some extra complexity to deal correctly with updates to
+   only parts of SP.  Bizarre, but it has been known to happen.
 */
 static
 IRSB* vg_SP_update_pass ( void*             closureV,
@@ -216,6 +219,7 @@ IRSB* vg_SP_update_pass ( void*             closureV,
                           IRType            hWordTy )
 {
    Int         i, j, minoff_ST, maxoff_ST, sizeof_SP, offset_SP;
+   Int         first_SP, last_SP, first_Put, last_Put;
    IRDirty     *dcall, *d;
    IRStmt*     st;
    IRExpr*     e;
@@ -285,6 +289,7 @@ IRSB* vg_SP_update_pass ( void*             closureV,
       if (e->tag != Iex_Get)              goto case2;
       if (e->Iex.Get.offset != offset_SP) goto case2;
       if (e->Iex.Get.ty != typeof_SP)     goto case2;
+      vg_assert( typeOfIRTemp(bb->tyenv, st->Ist.WrTmp.tmp) == typeof_SP );
       add_SP_alias(st->Ist.WrTmp.tmp, 0);
       addStmtToIRSB( bb, st );
       continue;
@@ -299,6 +304,7 @@ IRSB* vg_SP_update_pass ( void*             closureV,
       if (e->Iex.Binop.arg2->tag != Iex_Const) goto case3;
       if (!IS_ADD_OR_SUB(e->Iex.Binop.op)) goto case3;
       con = GET_CONST(e->Iex.Binop.arg2->Iex.Const.con);
+      vg_assert( typeOfIRTemp(bb->tyenv, st->Ist.WrTmp.tmp) == typeof_SP );
       if (IS_ADD(e->Iex.Binop.op)) {
          add_SP_alias(st->Ist.WrTmp.tmp, delta + con);
       } else {
@@ -313,17 +319,44 @@ IRSB* vg_SP_update_pass ( void*             closureV,
       e = st->Ist.WrTmp.data;
       if (e->tag != Iex_RdTmp) goto case4;
       if (!get_SP_delta(e->Iex.RdTmp.tmp, &delta)) goto case4;
+      vg_assert( typeOfIRTemp(bb->tyenv, st->Ist.WrTmp.tmp) == typeof_SP );
       add_SP_alias(st->Ist.WrTmp.tmp, delta);
       addStmtToIRSB( bb, st );
       continue;
 
      case4:
       /* Put(sp) = curr */
+      /* More generally, we must correctly handle a Put which writes
+         any part of SP, not just the case where all of SP is
+         written. */
       if (st->tag != Ist_Put) goto case5;
-      if (st->Ist.Put.offset != offset_SP) goto case5;
-      if (st->Ist.Put.data->tag != Iex_RdTmp) goto case5;
-      if (get_SP_delta(st->Ist.Put.data->Iex.RdTmp.tmp, &delta)) {
+      first_SP  = offset_SP;
+      last_SP   = first_SP + sizeof_SP - 1;
+      first_Put = st->Ist.Put.offset;
+      last_Put  = first_Put
+                  + sizeofIRType( typeOfIRExpr( bb->tyenv, st->Ist.Put.data ))
+                  - 1;
+      vg_assert(first_SP <= last_SP);
+      vg_assert(first_Put <= last_Put);
+
+      if (last_Put < first_SP || last_SP < first_Put)
+         goto case5; /* no overlap */
+
+      if (st->Ist.Put.data->tag == Iex_RdTmp
+          && get_SP_delta(st->Ist.Put.data->Iex.RdTmp.tmp, &delta)) {
          IRTemp tttmp = st->Ist.Put.data->Iex.RdTmp.tmp;
+         /* Why should the following assertion hold?  Because any
+            alias added by put_SP_alias must be of a temporary which
+            has the same type as typeof_SP, and whose value is a Get
+            at exactly offset_SP of size typeof_SP.  Each call to
+            put_SP_alias is immediately preceded by an assertion that
+            we are putting in a binding for a correctly-typed
+            temporary. */
+         vg_assert( typeOfIRTemp(bb->tyenv, tttmp) == typeof_SP );
+         /* From the same type-and-offset-correctness argument, if 
+            we found a useable alias, it must for an "exact" write of SP. */
+         vg_assert(first_SP == first_Put);
+         vg_assert(last_SP == last_Put);
          switch (delta) {
             case    0:                      addStmtToIRSB(bb,st); continue;
             case    4: DO(die,  4,  tttmp); addStmtToIRSB(bb,st); continue;
@@ -350,6 +383,18 @@ IRSB* vg_SP_update_pass ( void*             closureV,
                goto generic;
          }
       } else {
+         /* Deal with an unknown update to SP.  We're here because
+            either:
+            (1) the Put does not exactly cover SP; it is a partial update.
+                Highly unlikely, but has been known to happen for 16-bit
+                Windows apps running on Wine, doing 16-bit adjustments to
+                %sp.
+            (2) the Put does exactly cover SP, but we are unable to
+                determine how the value relates to the old SP.  In any 
+                case, we cannot assume that the Put.data value is a tmp;
+                we must assume it can be anything allowed in flat IR (tmp
+                or const).
+         */
          IRTemp old_SP;
          n_SP_updates_generic_unknown++;
 
@@ -364,18 +409,68 @@ IRSB* vg_SP_update_pass ( void*             closureV,
             IRStmt_WrTmp( old_SP, IRExpr_Get(offset_SP, typeof_SP) ) 
          );
 
-         dcall = unsafeIRDirty_0_N( 
-                    2/*regparms*/, 
-                    "VG_(unknown_SP_update)", 
-                    VG_(fnptr_to_fnentry)( &VG_(unknown_SP_update) ),
-                    mkIRExprVec_2( IRExpr_RdTmp(old_SP), st->Ist.Put.data ) 
-                 );
-         addStmtToIRSB( bb, IRStmt_Dirty(dcall) );
-
-         addStmtToIRSB( bb, st );
+         /* Now we know what the old value of SP is.  But knowing the new
+            value is a bit tricky if there is a partial write. */
+         if (first_Put == first_SP && last_Put == last_SP) {
+           /* The common case, an exact write to SP.  So st->Ist.Put.data
+              does hold the new value; simple. */
+            dcall = unsafeIRDirty_0_N( 
+                       2/*regparms*/, 
+                       "VG_(unknown_SP_update)", 
+                       VG_(fnptr_to_fnentry)( &VG_(unknown_SP_update) ),
+                       mkIRExprVec_2( IRExpr_RdTmp(old_SP), st->Ist.Put.data ) 
+                    );
+            addStmtToIRSB( bb, IRStmt_Dirty(dcall) );
+            /* don't forget the original assignment */
+            addStmtToIRSB( bb, st );
+         } else {
+            /* We have a partial update to SP.  We need to know what
+               the new SP will be, and hand that to the helper call,
+               but when the helper call happens, SP must hold the
+               value it had before the update.  Tricky.
+               Therefore use the following kludge:
+               1. do the partial SP update (Put)
+               2. Get the new SP value into a tmp, new_SP
+               3. Put old_SP
+               4. Call the helper
+               5. Put new_SP
+            */
+            IRTemp new_SP;
+            /* 1 */
+            addStmtToIRSB( bb, st );
+            /* 2 */
+            new_SP = newIRTemp(bb->tyenv, typeof_SP);
+            addStmtToIRSB( 
+               bb,
+               IRStmt_WrTmp( new_SP, IRExpr_Get(offset_SP, typeof_SP) ) 
+            );
+            /* 3 */
+            addStmtToIRSB( bb, IRStmt_Put(offset_SP, IRExpr_RdTmp(old_SP) ));
+            /* 4 */
+            dcall = unsafeIRDirty_0_N( 
+                       2/*regparms*/, 
+                       "VG_(unknown_SP_update)", 
+                       VG_(fnptr_to_fnentry)( &VG_(unknown_SP_update) ),
+                       mkIRExprVec_2( IRExpr_RdTmp(old_SP),
+                                      IRExpr_RdTmp(new_SP))
+                    );
+            addStmtToIRSB( bb, IRStmt_Dirty(dcall) );
+            /* 5 */
+            addStmtToIRSB( bb, IRStmt_Put(offset_SP, IRExpr_RdTmp(new_SP) ));
+         }
 
+         /* Forget what we already know. */
          clear_SP_aliases();
-         add_SP_alias(st->Ist.Put.data->Iex.RdTmp.tmp, 0);
+
+         /* If this is a Put of a tmp that exactly updates SP,
+            start tracking aliases against this tmp. */
+
+         if (first_Put == first_SP && last_Put == last_SP
+             && st->Ist.Put.data->tag == Iex_RdTmp) {
+            vg_assert( typeOfIRTemp(bb->tyenv, st->Ist.Put.data->Iex.RdTmp.tmp)
+                       == typeof_SP );
+            add_SP_alias(st->Ist.Put.data->Iex.RdTmp.tmp, 0);
+         }
          continue;
       }
 
@@ -386,8 +481,10 @@ IRSB* vg_SP_update_pass ( void*             closureV,
       if (st->tag == Ist_PutI) {
          descr = st->Ist.PutI.descr;
          minoff_ST = descr->base;
-         maxoff_ST = descr->base + descr->nElems * sizeofIRType(descr->elemTy) - 1;
-         if (!(offset_SP > maxoff_ST || (offset_SP + sizeof_SP - 1) < minoff_ST))
+         maxoff_ST = descr->base 
+                     + descr->nElems * sizeofIRType(descr->elemTy) - 1;
+         if (!(offset_SP > maxoff_ST
+               || (offset_SP + sizeof_SP - 1) < minoff_ST))
             goto complain;
       }
       if (st->tag == Ist_Dirty) {
@@ -397,7 +494,8 @@ IRSB* vg_SP_update_pass ( void*             closureV,
             maxoff_ST = d->fxState[j].offset + d->fxState[j].size - 1;
             if (d->fxState[j].fx == Ifx_Read || d->fxState[j].fx == Ifx_None)
                continue;
-            if (!(offset_SP > maxoff_ST || (offset_SP + sizeof_SP - 1) < minoff_ST))
+            if (!(offset_SP > maxoff_ST
+                  || (offset_SP + sizeof_SP - 1) < minoff_ST))
                goto complain;
          }
       }