Skip to content

Commit 3513991

Browse files
CouleeAppsCouleeApps
committed
Better block checking
1 parent 0301994 commit 3513991

File tree

2 files changed

+108
-56
lines changed

2 files changed

+108
-56
lines changed

‎plugins/lift_check/lifted_il_lift_check.cpp‎

Lines changed: 106 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,11 @@ void LiftedILVerifier::Verify()
195195
{
196196
/*
197197
Invariants:
198+
-[-] All blocks either branch to existing blocks or terminate
199+
(n/i for now)
200+
-[-] No jumping to entry block
201+
(currently allowed)
202+
-[x] All blocks have source blocks
198203
-[x] Sizes of expressions are consistent
199204
-[x] Base level instructions are of a limited subset of operations (setreg, call, etc) or set flags
200205
-[x] Child expressions are of a limited subset of operations (eg NOT goto)
@@ -216,99 +221,144 @@ void LiftedILVerifier::Verify()
216221
return;
217222
}
218223

224+
// Check block layout
225+
auto entryBlock = m_il->GetBasicBlockForInstruction(0);
226+
if (!entryBlock)
227+
{
228+
m_diagnostics.push_back(Diagnostic::Diag(WarningSeverity, this, "no entry block for function"));
229+
}
230+
for (auto& bb: m_il->GetBasicBlocks())
231+
{
232+
for (auto& outgoing: bb->GetOutgoingEdges())
233+
{
234+
auto source = bb->GetSourceBlock();
235+
if (!source)
236+
{
237+
m_diagnostics.push_back(Diagnostic::Error(this, fmt::format("block {}->{} has no source block? (probably need to call Finalize again or something)", bb->GetStart(), bb->GetEnd())));
238+
source = bb;
239+
}
240+
// TODO: This is currently valid but we want this to eventually be lifted as a tailcall
241+
if (outgoing.target == entryBlock)
242+
{
243+
m_diagnostics.push_back(Diagnostic::Diag(WarningSeverity, this, fmt::format("block {:#x}->{:#x} jumps to entry block (probably a bug in core's Finalize)", source->GetStart(), source->GetEnd())));
244+
}
245+
}
246+
}
247+
219248
// Check expr sizes
220-
for (size_t instrIndex = 0; instrIndex < m_il->GetInstructionCount(); instrIndex++)
249+
for (auto& bb: m_il->GetBasicBlocks())
221250
{
222-
LowLevelILInstruction instr = m_il->GetInstruction(instrIndex);
223-
CheckInstrSize(instr);
251+
for (size_t instrIndex = bb->GetStart(); instrIndex != bb->GetEnd(); instrIndex++)
252+
{
253+
LowLevelILInstruction instr = m_il->GetInstruction(instrIndex);
254+
CheckInstrSize(instr);
255+
}
224256
}
225257

226258
// Check exprs are where we expect them to be
227-
for (size_t instrIndex = 0; instrIndex < m_il->GetInstructionCount(); instrIndex ++)
259+
for (auto& bb: m_il->GetBasicBlocks())
228260
{
229-
LowLevelILInstruction instr = m_il->GetInstruction(instrIndex);
230-
instr.VisitExprs([&](const LowLevelILInstruction& expr) {
231-
if (auto found = g_instructionValidity.find(expr.operation); found != g_instructionValidity.end())
232-
{
233-
// We always check Non-SSA form (no Lifted IL SSA)
234-
if ((found->second & ValidInNonSSA) == 0)
235-
{
236-
m_diagnostics.push_back(Diagnostic::Error(this, expr, "Expression is not valid in non-ssa form"));
237-
}
238-
if (expr.exprIndex == instr.exprIndex)
261+
for (size_t instrIndex = bb->GetStart(); instrIndex != bb->GetEnd(); instrIndex++)
262+
{
263+
LowLevelILInstruction instr = m_il->GetInstruction(instrIndex);
264+
instr.VisitExprs([&](const LowLevelILInstruction& expr) {
265+
if (auto found = g_instructionValidity.find(expr.operation); found != g_instructionValidity.end())
239266
{
240-
if ((found->second & ValidAsParent) == 0 && expr.flags == 0)
267+
// We always check Non-SSA form (no Lifted IL SSA)
268+
if ((found->second & ValidInNonSSA) == 0)
241269
{
242-
m_diagnostics.push_back(Diagnostic::Diag(WarningSeverity, this, expr, "Expression is not expected to be parent instruction without setting flags"));
270+
m_diagnostics.push_back(Diagnostic::Error(this, expr, "Expression is not valid in non-ssa form"));
271+
}
272+
if (expr.exprIndex == instr.exprIndex)
273+
{
274+
if ((found->second & ValidAsParent) == 0 && expr.flags == 0)
275+
{
276+
m_diagnostics.push_back(Diagnostic::Diag(WarningSeverity, this, expr, "Expression is not expected to be parent instruction without setting flags"));
277+
}
278+
}
279+
else
280+
{
281+
if ((found->second & ValidAsChild) == 0)
282+
{
283+
m_diagnostics.push_back(Diagnostic::Error(this, expr, "Instruction is not valid as child expression"));
284+
}
243285
}
244286
}
245287
else
246288
{
247-
if ((found->second & ValidAsChild) == 0)
248-
{
249-
m_diagnostics.push_back(Diagnostic::Error(this, expr, "Instruction is not valid as child expression"));
250-
}
289+
m_diagnostics.push_back(Diagnostic::Error(this, expr, "Unknown expression operation"));
251290
}
252-
}
253-
else
254-
{
255-
m_diagnostics.push_back(Diagnostic::Error(this, expr, "Unknown expression operation"));
256-
}
257-
return true;
258-
});
291+
return true;
292+
});
293+
}
259294
}
260295

261296
// Check expr operands
262-
for (size_t instrIndex = 0; instrIndex < m_il->GetInstructionCount(); instrIndex++)
297+
for (auto& bb: m_il->GetBasicBlocks())
263298
{
264-
LowLevelILInstruction instr = m_il->GetInstruction(instrIndex);
265-
CheckExprOperands(instr);
299+
for (size_t instrIndex = bb->GetStart(); instrIndex != bb->GetEnd(); instrIndex++)
300+
{
301+
LowLevelILInstruction instr = m_il->GetInstruction(instrIndex);
302+
CheckExprOperands(instr);
303+
}
266304
}
267305

268306
// Check exprs are used at most once
269307
std::unordered_set<size_t> seenExprs;
270-
for (size_t instrIndex = 0; instrIndex < m_il->GetInstructionCount(); instrIndex++)
308+
for (auto& bb: m_il->GetBasicBlocks())
271309
{
272-
LowLevelILInstruction instr = m_il->GetInstruction(instrIndex);
273-
instr.VisitExprs([&](const LowLevelILInstruction& expr) {
274-
if (seenExprs.insert(expr.exprIndex).second == false)
275-
{
276-
m_diagnostics.push_back(Diagnostic::Error(this, expr, "Expression used more than once"));
277-
}
278-
return true;
279-
});
310+
for (size_t instrIndex = bb->GetStart(); instrIndex != bb->GetEnd(); instrIndex++)
311+
{
312+
LowLevelILInstruction instr = m_il->GetInstruction(instrIndex);
313+
instr.VisitExprs([&](const LowLevelILInstruction& expr) {
314+
if (seenExprs.insert(expr.exprIndex).second == false)
315+
{
316+
m_diagnostics.push_back(Diagnostic::Error(this, expr, "Expression used more than once"));
317+
}
318+
return true;
319+
});
320+
}
280321
}
281322

282323
// Check exprs have addresses
283-
for (size_t instrIndex = 0; instrIndex < m_il->GetInstructionCount(); instrIndex++)
324+
for (auto& bb: m_il->GetBasicBlocks())
284325
{
285-
LowLevelILInstruction instr = m_il->GetInstruction(instrIndex);
286-
instr.VisitExprs([&](const LowLevelILInstruction& expr) {
287-
if (expr.address == 0)
288-
{
289-
m_diagnostics.push_back(Diagnostic::Error(this, expr, "Found expression with no address"));
290-
}
291-
return true;
292-
});
326+
for (size_t instrIndex = bb->GetStart(); instrIndex != bb->GetEnd(); instrIndex++)
327+
{
328+
LowLevelILInstruction instr = m_il->GetInstruction(instrIndex);
329+
instr.VisitExprs([&](const LowLevelILInstruction& expr) {
330+
if (expr.address == 0)
331+
{
332+
m_diagnostics.push_back(Diagnostic::Error(this, expr, "Found expression with no address"));
333+
}
334+
return true;
335+
});
336+
}
293337
}
294338

295339
// Check not more than 1 pop per tree
296-
for (size_t instrIndex = 0; instrIndex < m_il->GetInstructionCount(); instrIndex++)
340+
for (auto& bb: m_il->GetBasicBlocks())
297341
{
298-
LowLevelILInstruction instr = m_il->GetInstruction(instrIndex);
299-
if (GetTreePopCount(instr) > 1)
342+
for (size_t instrIndex = bb->GetStart(); instrIndex != bb->GetEnd(); instrIndex++)
300343
{
301-
m_diagnostics.push_back(Diagnostic::Error(this, instr, "Found more than 1 pop in instruction"));
344+
LowLevelILInstruction instr = m_il->GetInstruction(instrIndex);
345+
if (GetTreePopCount(instr) > 1)
346+
{
347+
m_diagnostics.push_back(Diagnostic::Error(this, instr, "Found more than 1 pop in instruction"));
348+
}
302349
}
303350
}
304351

305352
// Check not more than 1 flag write per tree
306-
for (size_t instrIndex = 0; instrIndex < m_il->GetInstructionCount(); instrIndex++)
353+
for (auto& bb: m_il->GetBasicBlocks())
307354
{
308-
LowLevelILInstruction instr = m_il->GetInstruction(instrIndex);
309-
if (GetTreeFlagWriteCount(instr) > 1)
355+
for (size_t instrIndex = bb->GetStart(); instrIndex != bb->GetEnd(); instrIndex++)
310356
{
311-
m_diagnostics.push_back(Diagnostic::Error(this, instr, "Found more than 1 flag write in instruction"));
357+
LowLevelILInstruction instr = m_il->GetInstruction(instrIndex);
358+
if (GetTreeFlagWriteCount(instr) > 1)
359+
{
360+
m_diagnostics.push_back(Diagnostic::Error(this, instr, "Found more than 1 flag write in instruction"));
361+
}
312362
}
313363
}
314364
}

‎plugins/lift_check/llil_lift_check.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1117,6 +1117,8 @@ void LowLevelILVerifier::CheckExprOperands(const BinaryNinja::LowLevelILInstruct
11171117
size_t instrCount = m_il->GetInstructionCount();
11181118
size_t target = expr.GetTarget<LLIL_GOTO>();
11191119
CHECK(target < instrCount, "target {} out of range of function with {} instructions", target, instrCount);
1120+
auto targetBlock = m_il->GetBasicBlockForInstruction(target);
1121+
CHECK(targetBlock != nullptr, "target {} has no basic block? (probably need to call Finalize again)", target);
11201122
break;
11211123
}
11221124
case LLIL_SYSCALL:

0 commit comments

Comments
 (0)