Skip to content

Commit 43360b1

Browse files
committed
Cover a return value that comes back in the SSE registers
A post hook that returns a value of the same type leaves it in xmm0, which is where a caller expecting an aggregate of floats looks. Without the change that goes with this the caller gets the hook's value rather than the function's, so the case is worth pinning down. The hooks come off before the values are judged. A CHECK that returns early would leave them installed, and SourceHook shutting down after the generator is gone takes the process with it, which turns a readable failure into a crash after the test has already found the fault.
1 parent 802f8eb commit 43360b1

1 file changed

Lines changed: 114 additions & 0 deletions

File tree

core/sourcehook/test/testhookmangen.cpp

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,64 @@ namespace
598598
}
599599
};
600600

601+
602+
// A return value made only of floats comes back in the SSE registers. A
603+
// PassInfo carries sizes but not field types, so the generator cannot tell
604+
// that apart from an aggregate of integers, and reads SourceHook's copy of
605+
// the value as INTEGER. What it can do, and has to, is carry the registers
606+
// the original returned in through untouched. Replacing such a value from a
607+
// hook is not covered below because it does not work: the copy a hook writes
608+
// is read back as INTEGER as well.
609+
struct FloatPair
610+
{
611+
float a;
612+
float b;
613+
};
614+
615+
MAKE_STATE_1(State_FloatPair_Called, int);
616+
MAKE_STATE_1(State_FloatPair_PreHook, int);
617+
618+
class FloatPairTest
619+
{
620+
public:
621+
virtual FloatPair Get(int seed)
622+
{
623+
ADD_STATE(State_FloatPair_Called(seed));
624+
FloatPair ret;
625+
ret.a = seed + 0.5f;
626+
ret.b = seed + 1.5f;
627+
return ret;
628+
}
629+
};
630+
631+
MAKE_STATE_1(State_FloatPair_PostHook, int);
632+
633+
// A post hook that returns a value of the same type leaves it in the SSE
634+
// registers, which is what the original's return value has to survive.
635+
class FloatPair_PostDeleg : public MyDelegate
636+
{
637+
virtual FloatPair Call(int seed)
638+
{
639+
ADD_STATE(State_FloatPair_PostHook(seed));
640+
FloatPair scratch;
641+
scratch.a = seed * 3.25f;
642+
scratch.b = seed * 7.75f;
643+
RETURN_META_VALUE(MRES_IGNORED, scratch);
644+
}
645+
};
646+
647+
class FloatPair_Deleg : public MyDelegate
648+
{
649+
virtual FloatPair Call(int seed)
650+
{
651+
ADD_STATE(State_FloatPair_PreHook(seed));
652+
FloatPair unused;
653+
unused.a = 0.0f;
654+
unused.b = 0.0f;
655+
RETURN_META_VALUE(MRES_IGNORED, unused);
656+
}
657+
};
658+
601659
bool Tests1(std::string &error)
602660
{
603661
THGM_DO_TEST_void(0, ());
@@ -1092,6 +1150,60 @@ namespace
10921150

10931151
return true;
10941152
}
1153+
1154+
bool Tests6(std::string &error)
1155+
{
1156+
FloatPairTest *pFloatPair = new FloatPairTest;
1157+
CAutoPtrDestruction<FloatPairTest> apdFloatPair(pFloatPair);
1158+
1159+
SourceHook::CProtoInfoBuilder floatPairPi(SourceHook::ProtoInfo::CallConv_ThisCall);
1160+
floatPairPi.AddParam(sizeof(int), SourceHook::PassInfo::PassType_Basic,
1161+
SourceHook::PassInfo::PassFlag_ByVal, NULL, NULL, NULL, NULL);
1162+
floatPairPi.SetReturnType(sizeof(FloatPair), SourceHook::PassInfo::PassType_Object,
1163+
SourceHook::PassInfo::PassFlag_ByVal, NULL, NULL, NULL, NULL);
1164+
1165+
// FloatPairTest::Get is the only virtual it has.
1166+
SourceHook::HookManagerPubFunc floatPairHM = g_HMAGPtr->MakeHookMan(floatPairPi, 0, 0);
1167+
CHECK_COND(floatPairHM != NULL, "TestFloatPairRet /makehookman");
1168+
CAutoReleaseHookMan arhm_floatPair(floatPairHM);
1169+
1170+
FloatPair unhooked = pFloatPair->Get(10);
1171+
CHECK_STATES((&g_States,
1172+
new State_FloatPair_Called(10),
1173+
NULL), "TestFloatPairRet Part1");
1174+
1175+
int floatPairHook = g_SHPtr->AddHook(g_PLID, SourceHook::ISourceHook::Hook_Normal,
1176+
reinterpret_cast<void*>(pFloatPair), 0, floatPairHM, new FloatPair_Deleg, false);
1177+
1178+
FloatPair preHooked = pFloatPair->Get(10);
1179+
CHECK_STATES((&g_States,
1180+
new State_FloatPair_PreHook(10),
1181+
new State_FloatPair_Called(10),
1182+
NULL), "TestFloatPairRet Part2");
1183+
1184+
int floatPairPost = g_SHPtr->AddHook(g_PLID, SourceHook::ISourceHook::Hook_Normal,
1185+
reinterpret_cast<void*>(pFloatPair), 0, floatPairHM, new FloatPair_PostDeleg, true);
1186+
1187+
FloatPair bothHooked = pFloatPair->Get(10);
1188+
CHECK_STATES((&g_States,
1189+
new State_FloatPair_PreHook(10),
1190+
new State_FloatPair_Called(10),
1191+
new State_FloatPair_PostHook(10),
1192+
NULL), "TestFloatPairRet Part3");
1193+
1194+
// The hooks come off before the values are judged: a CHECK that returns
1195+
// early would leave them installed, and SourceHook shutting down after the
1196+
// generator has gone takes the process with it.
1197+
g_SHPtr->RemoveHookByID(floatPairPost);
1198+
g_SHPtr->RemoveHookByID(floatPairHook);
1199+
1200+
CHECK_COND(unhooked.a == 10.5f && unhooked.b == 11.5f, "TestFloatPairRet Part1 /value");
1201+
CHECK_COND(preHooked.a == 10.5f && preHooked.b == 11.5f, "TestFloatPairRet Part2 /value");
1202+
CHECK_COND(bothHooked.a == 10.5f && bothHooked.b == 11.5f, "TestFloatPairRet Part3 /value");
1203+
1204+
return true;
1205+
}
1206+
10951207
}
10961208

10971209
#if !defined( _M_AMD64 )
@@ -1120,6 +1232,8 @@ bool TestHookManGen(std::string &error)
11201232
return false;
11211233
if (!Tests5(error))
11221234
return false;
1235+
if (!Tests6(error))
1236+
return false;
11231237

11241238
// Shutdown now!
11251239
// If we don't SH will auto-shutdown _after_ genc's destructor is called

0 commit comments

Comments
 (0)