From: Julian Seward Date: Sun, 11 Nov 2007 18:56:39 +0000 (+0000) Subject: In vg_SP_update_pass (stack-pointer-change analysis code), correctly X-Git-Tag: svn/VALGRIND_3_3_0~143 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dc77d6e27af2f189348557a8404831accf6c06aa;p=thirdparty%2Fvalgrind.git In vg_SP_update_pass (stack-pointer-change analysis code), correctly handle partial updates of SP. Tricky. This fixes #152022. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@7147 --- diff --git a/coregrind/m_translate.c b/coregrind/m_translate.c index f3fadea762..f306b31de7 100644 --- a/coregrind/m_translate.c +++ b/coregrind/m_translate.c @@ -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; } }