Skip to content

Commit 8d069d9

Browse files
xusheng6claude
andcommitted
[emulator] Fix signed IL ops for operands wider than 8 bytes and INT_MIN/-1
DIVS/MODS/MULS_DP/CMP_S*/MINS/MAXS funneled operands through int64_t via SignExtend(v, sz, 8), which silently truncated any operand wider than 8 bytes to its low 64 bits, and native signed division of INT_MIN by -1 is undefined behavior (SIGFPE on x86). Add wide-signed helpers (IsNegative/SignedLess/SignedDivideOrModulo) that operate directly on the 512-bit value: comparisons use sign-aware unsigned compares, and division works on magnitudes then reapplies the sign, so INT_MIN / -1 wraps to INT_MIN (and % to 0) instead of trapping. Addresses bdash review comment on PR #8314. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9805adc commit 8d069d9

2 files changed

Lines changed: 75 additions & 36 deletions

File tree

plugins/emulator/core/llilemulator.cpp

Lines changed: 68 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,46 @@ intx::uint512 LLILEmulator::SignExtend(const intx::uint512& value, size_t fromSi
132132
}
133133

134134

135+
bool LLILEmulator::IsNegative(const intx::uint512& value, size_t size)
136+
{
137+
if (size == 0 || size > 64)
138+
return false;
139+
intx::uint512 signBit = intx::uint512(1) << (size * 8 - 1);
140+
return (MaskToSize(value, size) & signBit) != 0;
141+
}
142+
143+
144+
bool LLILEmulator::SignedLess(const intx::uint512& a, const intx::uint512& b, size_t size)
145+
{
146+
bool an = IsNegative(a, size);
147+
bool bn = IsNegative(b, size);
148+
if (an != bn)
149+
return an; // negative < non-negative
150+
// Same sign: the unsigned comparison of the masked values gives the correct order.
151+
return MaskToSize(a, size) < MaskToSize(b, size);
152+
}
153+
154+
155+
intx::uint512 LLILEmulator::SignedDivideOrModulo(
156+
const intx::uint512& a, const intx::uint512& b, size_t size, bool modulo)
157+
{
158+
// Work with magnitudes as unsigned wide integers, then reapply the sign. This is correct
159+
// for operands of any width up to 64 bytes and, unlike native signed division, does not
160+
// trap on INT_MIN / -1 (which wraps to INT_MIN for divide and 0 for modulo).
161+
intx::uint512 am = MaskToSize(a, size);
162+
intx::uint512 bm = MaskToSize(b, size);
163+
bool an = IsNegative(am, size);
164+
bool bn = IsNegative(bm, size);
165+
intx::uint512 au = an ? MaskToSize(~am + 1, size) : am;
166+
intx::uint512 bu = bn ? MaskToSize(~bm + 1, size) : bm;
167+
intx::uint512 result = modulo ? (au % bu) : (au / bu);
168+
bool resultNeg = modulo ? an : (an != bn);
169+
if (resultNeg)
170+
result = MaskToSize(~result + 1, size);
171+
return MaskToSize(result, size);
172+
}
173+
174+
135175
BNEndianness LLILEmulator::GetEndianness() const
136176
{
137177
if (m_arch)
@@ -1105,15 +1145,14 @@ intx::uint512 LLILEmulator::EvalExpr(const LowLevelILInstruction& expr)
11051145

11061146
case LLIL_DIVS:
11071147
{
1108-
// For signed division, work with 64-bit values since intx doesn't provide signed types
1109-
int64_t left = static_cast<int64_t>(static_cast<uint64_t>(SignExtend(EvalExpr(expr.GetRawOperandAsExpr(0)), sz, 8)));
1110-
int64_t right = static_cast<int64_t>(static_cast<uint64_t>(SignExtend(EvalExpr(expr.GetRawOperandAsExpr(1)), sz, 8)));
1111-
if (right == 0)
1148+
intx::uint512 left = EvalExpr(expr.GetRawOperandAsExpr(0));
1149+
intx::uint512 right = EvalExpr(expr.GetRawOperandAsExpr(1));
1150+
if (MaskToSize(right, sz) == 0)
11121151
{
11131152
SetStopReason(ILEmulatorStopReason::Error, "division by zero");
11141153
return 0;
11151154
}
1116-
return MaskToSize(intx::uint512(static_cast<uint64_t>(left / right)), sz);
1155+
return SignedDivideOrModulo(left, right, sz, false);
11171156
}
11181157

11191158
case LLIL_MODU:
@@ -1130,14 +1169,14 @@ intx::uint512 LLILEmulator::EvalExpr(const LowLevelILInstruction& expr)
11301169

11311170
case LLIL_MODS:
11321171
{
1133-
int64_t left = static_cast<int64_t>(static_cast<uint64_t>(SignExtend(EvalExpr(expr.GetRawOperandAsExpr(0)), sz, 8)));
1134-
int64_t right = static_cast<int64_t>(static_cast<uint64_t>(SignExtend(EvalExpr(expr.GetRawOperandAsExpr(1)), sz, 8)));
1135-
if (right == 0)
1172+
intx::uint512 left = EvalExpr(expr.GetRawOperandAsExpr(0));
1173+
intx::uint512 right = EvalExpr(expr.GetRawOperandAsExpr(1));
1174+
if (MaskToSize(right, sz) == 0)
11361175
{
11371176
SetStopReason(ILEmulatorStopReason::Error, "modulo by zero");
11381177
return 0;
11391178
}
1140-
return MaskToSize(intx::uint512(static_cast<uint64_t>(left % right)), sz);
1179+
return SignedDivideOrModulo(left, right, sz, true);
11411180
}
11421181

11431182
// --- Double-precision ---
@@ -1150,18 +1189,11 @@ intx::uint512 LLILEmulator::EvalExpr(const LowLevelILInstruction& expr)
11501189

11511190
case LLIL_MULS_DP:
11521191
{
1153-
// Signed multiply double-precision: sign extend operands, multiply, mask
1154-
int64_t left = static_cast<int64_t>(static_cast<uint64_t>(SignExtend(EvalExpr(expr.GetRawOperandAsExpr(0)), sz / 2, 8)));
1155-
int64_t right = static_cast<int64_t>(static_cast<uint64_t>(SignExtend(EvalExpr(expr.GetRawOperandAsExpr(1)), sz / 2, 8)));
1156-
// Sign-extend both operands to full width as two's-complement intx values and
1157-
// multiply; the low bits of the product are correct regardless of sign. This
1158-
// avoids __int128, which MSVC does not support.
1159-
intx::uint512 l(static_cast<uint64_t>(left));
1160-
intx::uint512 r(static_cast<uint64_t>(right));
1161-
if (left < 0)
1162-
l |= ~intx::uint512(0) << 64;
1163-
if (right < 0)
1164-
r |= ~intx::uint512(0) << 64;
1192+
// Signed multiply double-precision: sign-extend each sz/2-byte operand to the full
1193+
// wide value, then multiply as two's-complement. The low sz bytes of the product are
1194+
// correct regardless of sign, and this handles operands wider than 8 bytes.
1195+
intx::uint512 l = SignExtend(EvalExpr(expr.GetRawOperandAsExpr(0)), sz / 2, 64);
1196+
intx::uint512 r = SignExtend(EvalExpr(expr.GetRawOperandAsExpr(1)), sz / 2, 64);
11651197
return MaskToSize(l * r, sz);
11661198
}
11671199

@@ -1342,9 +1374,9 @@ intx::uint512 LLILEmulator::EvalExpr(const LowLevelILInstruction& expr)
13421374
{
13431375
intx::uint512 left = MaskToSize(EvalExpr(expr.GetRawOperandAsExpr(0)), sz);
13441376
intx::uint512 right = MaskToSize(EvalExpr(expr.GetRawOperandAsExpr(1)), sz);
1345-
int64_t sl = static_cast<int64_t>(static_cast<uint64_t>(SignExtend(left, sz, 8)));
1346-
int64_t sr = static_cast<int64_t>(static_cast<uint64_t>(SignExtend(right, sz, 8)));
1347-
bool leftWins = (expr.operation == LLIL_MINS) ? (sl <= sr) : (sl >= sr);
1377+
// leftWins for MINS when left <= right, for MAXS when left >= right (i.e. !(left < right))
1378+
bool leftWins = (expr.operation == LLIL_MINS) ? !SignedLess(right, left, sz)
1379+
: !SignedLess(left, right, sz);
13481380
return leftWins ? left : right;
13491381
}
13501382

@@ -1374,9 +1406,9 @@ intx::uint512 LLILEmulator::EvalExpr(const LowLevelILInstruction& expr)
13741406

13751407
case LLIL_CMP_SLT:
13761408
{
1377-
int64_t left = static_cast<int64_t>(static_cast<uint64_t>(SignExtend(EvalExpr(expr.GetRawOperandAsExpr(0)), sz, 8)));
1378-
int64_t right = static_cast<int64_t>(static_cast<uint64_t>(SignExtend(EvalExpr(expr.GetRawOperandAsExpr(1)), sz, 8)));
1379-
return left < right ? intx::uint512(1) : intx::uint512(0);
1409+
intx::uint512 left = EvalExpr(expr.GetRawOperandAsExpr(0));
1410+
intx::uint512 right = EvalExpr(expr.GetRawOperandAsExpr(1));
1411+
return SignedLess(left, right, sz) ? intx::uint512(1) : intx::uint512(0);
13801412
}
13811413

13821414
case LLIL_CMP_ULT:
@@ -1388,9 +1420,9 @@ intx::uint512 LLILEmulator::EvalExpr(const LowLevelILInstruction& expr)
13881420

13891421
case LLIL_CMP_SLE:
13901422
{
1391-
int64_t left = static_cast<int64_t>(static_cast<uint64_t>(SignExtend(EvalExpr(expr.GetRawOperandAsExpr(0)), sz, 8)));
1392-
int64_t right = static_cast<int64_t>(static_cast<uint64_t>(SignExtend(EvalExpr(expr.GetRawOperandAsExpr(1)), sz, 8)));
1393-
return left <= right ? intx::uint512(1) : intx::uint512(0);
1423+
intx::uint512 left = EvalExpr(expr.GetRawOperandAsExpr(0));
1424+
intx::uint512 right = EvalExpr(expr.GetRawOperandAsExpr(1));
1425+
return !SignedLess(right, left, sz) ? intx::uint512(1) : intx::uint512(0);
13941426
}
13951427

13961428
case LLIL_CMP_ULE:
@@ -1402,9 +1434,9 @@ intx::uint512 LLILEmulator::EvalExpr(const LowLevelILInstruction& expr)
14021434

14031435
case LLIL_CMP_SGE:
14041436
{
1405-
int64_t left = static_cast<int64_t>(static_cast<uint64_t>(SignExtend(EvalExpr(expr.GetRawOperandAsExpr(0)), sz, 8)));
1406-
int64_t right = static_cast<int64_t>(static_cast<uint64_t>(SignExtend(EvalExpr(expr.GetRawOperandAsExpr(1)), sz, 8)));
1407-
return left >= right ? intx::uint512(1) : intx::uint512(0);
1437+
intx::uint512 left = EvalExpr(expr.GetRawOperandAsExpr(0));
1438+
intx::uint512 right = EvalExpr(expr.GetRawOperandAsExpr(1));
1439+
return !SignedLess(left, right, sz) ? intx::uint512(1) : intx::uint512(0);
14081440
}
14091441

14101442
case LLIL_CMP_UGE:
@@ -1416,9 +1448,9 @@ intx::uint512 LLILEmulator::EvalExpr(const LowLevelILInstruction& expr)
14161448

14171449
case LLIL_CMP_SGT:
14181450
{
1419-
int64_t left = static_cast<int64_t>(static_cast<uint64_t>(SignExtend(EvalExpr(expr.GetRawOperandAsExpr(0)), sz, 8)));
1420-
int64_t right = static_cast<int64_t>(static_cast<uint64_t>(SignExtend(EvalExpr(expr.GetRawOperandAsExpr(1)), sz, 8)));
1421-
return left > right ? intx::uint512(1) : intx::uint512(0);
1451+
intx::uint512 left = EvalExpr(expr.GetRawOperandAsExpr(0));
1452+
intx::uint512 right = EvalExpr(expr.GetRawOperandAsExpr(1));
1453+
return SignedLess(right, left, sz) ? intx::uint512(1) : intx::uint512(0);
14221454
}
14231455

14241456
case LLIL_CMP_UGT:

plugins/emulator/core/llilemulator.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,13 @@ namespace BinaryNinjaEmulator
7070
// Helpers
7171
static intx::uint512 MaskToSize(const intx::uint512& value, size_t size);
7272
static intx::uint512 SignExtend(const intx::uint512& value, size_t fromSize, size_t toSize);
73+
// Signed-integer helpers that operate directly on the wide value, so operands wider
74+
// than 8 bytes are handled correctly (and INT_MIN / -1 does not trap as it would on
75+
// native signed division).
76+
static bool IsNegative(const intx::uint512& value, size_t size);
77+
static bool SignedLess(const intx::uint512& a, const intx::uint512& b, size_t size);
78+
static intx::uint512 SignedDivideOrModulo(
79+
const intx::uint512& a, const intx::uint512& b, size_t size, bool modulo);
7380
void Push(const intx::uint512& value, size_t size);
7481
intx::uint512 Pop(size_t size);
7582
intx::uint512 EvalFlagCondition(BNLowLevelILFlagCondition cond, uint32_t semClass);

0 commit comments

Comments
 (0)