GP-3441 Protect INDIRECT path to switch variable

This commit is contained in:
caheckman 2023-05-16 18:37:19 -04:00
parent 7089e6de63
commit 0398e40b07
6 changed files with 116 additions and 2 deletions

View File

@ -56,6 +56,7 @@ src/decompile/datatests/retstruct.xml||GHIDRA||||END|
src/decompile/datatests/sbyte.xml||GHIDRA||||END|
src/decompile/datatests/skipnext2.xml||GHIDRA||||END|
src/decompile/datatests/statuscmp.xml||GHIDRA||||END|
src/decompile/datatests/switchind.xml||GHIDRA||||END|
src/decompile/datatests/threedim.xml||GHIDRA||||END|
src/decompile/datatests/twodim.xml||GHIDRA||||END|
src/decompile/datatests/union_datatype.xml||GHIDRA||||END|

View File

@ -2122,6 +2122,57 @@ int4 ActionLikelyTrash::apply(Funcdata &data)
return 0;
}
/// Test if the path to the given BRANCHIND originates from a constant but passes through INDIRECT operations.
/// This indicates that the switch value is produced indirectly, so we mark these INDIRECT
/// operations as \e not \e collapsible, to guarantee that the indirect value is not lost during analysis.
/// \param op is the given BRANCHIND op
void ActionRestructureVarnode::protectSwitchPathIndirects(PcodeOp *op)
{
vector<PcodeOp *> indirects;
Varnode *curVn = op->getIn(0);
while(curVn->isWritten()) {
PcodeOp *curOp = curVn->getDef();
uint4 evalType = curOp->getEvalType();
if ((evalType & (PcodeOp::binary | PcodeOp::ternary)) != 0) {
if (curOp->numInput() > 1) {
if (!curOp->getIn(1)->isConstant()) return; // Multiple paths
}
curVn = curOp->getIn(0);
}
else if ((evalType & PcodeOp::unary) != 0)
curVn = curOp->getIn(0);
else if (curOp->code() == CPUI_INDIRECT) {
indirects.push_back(curOp);
curVn = curOp->getIn(0);
}
else if (curOp->code() == CPUI_LOAD) {
curVn = curOp->getIn(1);
}
else
return;
}
if (!curVn->isConstant()) return;
// If we reach here, there is exactly one path, from a constant to a switch
for(int4 i=0;i<indirects.size();++i) {
indirects[i]->setNoIndirectCollapse();
}
}
/// Run through BRANCHIND ops, treat them as switches and protect the data-flow path to the destination variable
/// \param data is the function to examine
void ActionRestructureVarnode::protectSwitchPaths(Funcdata &data)
{
const BlockGraph &bblocks(data.getBasicBlocks());
for(int4 i=0;i<bblocks.getSize();++i) {
PcodeOp *op = bblocks.getBlock(i)->lastOp();
if (op == (PcodeOp *)0) continue;
if (op->code() != CPUI_BRANCHIND) continue;
protectSwitchPathIndirects(op);
}
}
int4 ActionRestructureVarnode::apply(Funcdata &data)
{
@ -2132,6 +2183,9 @@ int4 ActionRestructureVarnode::apply(Funcdata &data)
if (data.syncVarnodesWithSymbols(l1,false,aliasyes))
count += 1;
if (data.isJumptableRecoveryOn())
protectSwitchPaths(data);
numpass += 1;
#ifdef OPACTION_DEBUG
if ((flags&rule_debug)==0) return 0;

View File

@ -831,6 +831,8 @@ public:
/// This produces on intermediate view of symbols on the stack.
class ActionRestructureVarnode : public Action {
int4 numpass; ///< Number of passes performed for this function
static void protectSwitchPathIndirects(PcodeOp *op); ///< Protect path to the given switch from INDIRECT collapse
static void protectSwitchPaths(Funcdata &data); ///< Look for switches and protect path of switch variable
public:
ActionRestructureVarnode(const string &g) : Action(0,"restructure_varnode",g) {} ///< Constructor
virtual void reset(Funcdata &data) { numpass = 0; }

View File

@ -114,7 +114,8 @@ public:
is_cpool_transformed = 0x20, ///< Have we checked for cpool transforms
stop_type_propagation = 0x40, ///< Stop data-type propagation into output from descendants
hold_output = 0x80, ///< Output varnode (of call) should not be removed if it is unread
concat_root = 0x100 ///< Output of \b this is root of a CONCAT tree
concat_root = 0x100, ///< Output of \b this is root of a CONCAT tree
no_indirect_collapse = 0x200 ///< Do not collapse \b this INDIRECT (via RuleIndirectCollapse)
};
private:
TypeOp *opcode; ///< Pointer to class providing behavioral details of the operation
@ -219,6 +220,8 @@ public:
void setPartialRoot(void) { addlflags |= concat_root; } ///< Mark \b this as root of CONCAT tree
bool stopsCopyPropagation(void) const { return ((flags&no_copy_propagation)!=0); } ///< Does \b this allow COPY propagation
void setStopCopyPropagation(void) { flags |= no_copy_propagation; } ///< Stop COPY propagation through inputs
bool noIndirectCollapse(void) const { return ((addlflags & no_indirect_collapse)!=0); } ///< Check if INDIRECT collapse is possible
void setNoIndirectCollapse(void) { addlflags |= no_indirect_collapse; } ///< Prevent collapse of INDIRECT
/// \brief Return \b true if this LOADs or STOREs from a dynamic \e spacebase pointer
bool usesSpacebasePtr(void) const { return ((flags&PcodeOp::spacebase_ptr)!=0); }
uintm getCseHash(void) const; ///< Return hash indicating possibility of common subexpression elimination

View File

@ -2926,7 +2926,7 @@ int4 RuleIndirectCollapse::applyOp(PcodeOp *op,Funcdata &data)
}
}
else if (indop->isCall()) {
if (op->isIndirectCreation())
if (op->isIndirectCreation() || op->noIndirectCollapse())
return 0;
// If there are no aliases to a local variable, collapse
if (!op->getOut()->hasNoLocalAlias())

View File

@ -0,0 +1,54 @@
<decompilertest>
<binaryimage arch="x86:LE:64:default:gcc">
<bytechunk space="ram" offset="0x100010" readonly="true">
4883ec18488d7c240cc744240c000000
00e8da0f0000837c240c0a776b8b4424
0c488d1580000000486304824801d0ff
e000000000000000e8bb0f00004883c4
18c3000000000000e8b30f00004883c4
18c3000000000000e8ab0f00004883c4
18c3000000000000e8a30f00004883c4
18c3000000000000e89b0f00004883c4
18c3000000000000e8930f00004883c4
18c3
</bytechunk>
<bytechunk space="ram" offset="0x1000b8" readonly="true">
a0ffffffb0ffffff
c0ffffffd0ffffff90ffffff90ffffff
e0ffffffe0ffffffe0ffffffe0ffffff
90ffffff
</bytechunk>
<symbol space="ram" offset="0x100010" name="switchind"/>
</binaryimage>
<script>
<com>option readonly on</com>
<com>map fun r0x101000 get_value_byref</com>
<com>map fun r0x101010 casefunc0</com>
<com>map fun r0x101018 casefunc1</com>
<com>map fun r0x101020 casefunc2</com>
<com>map fun r0x101028 casefunc3</com>
<com>map fun r0x101008 casefuncmany</com>
<com>map fun r0x101030 casefuncdefault</com>
<com>lo fu switchind</com>
<com>map addr s0xfffffffffffffff4 int4 val</com>
<com>decompile</com>
<com>print C</com>
<com>quit</com>
</script>
<stringmatch name="Switch Indirect #1" min="1" max="1">case 0:</stringmatch>
<stringmatch name="Switch Indirect #2" min="1" max="1">case 1:</stringmatch>
<stringmatch name="Switch Indirect #3" min="1" max="1">case 2:</stringmatch>
<stringmatch name="Switch Indirect #4" min="1" max="1">case 3:</stringmatch>
<stringmatch name="Switch Indirect #5" min="1" max="1">case 4:</stringmatch>
<stringmatch name="Switch Indirect #6" min="1" max="1">case 5:</stringmatch>
<stringmatch name="Switch Indirect #7" min="1" max="1">case 10:</stringmatch>
<stringmatch name="Switch Indirect #8" min="1" max="1">default:</stringmatch>
<stringmatch name="Switch Indirect #9" min="1" max="1">casefunc0\(\);</stringmatch>
<stringmatch name="Switch Indirect #10" min="1" max="1">casefunc1\(\);</stringmatch>
<stringmatch name="Switch Indirect #11" min="1" max="1">casefunc2\(\);</stringmatch>
<stringmatch name="Switch Indirect #12" min="1" max="1">casefunc3\(\);</stringmatch>
<stringmatch name="Switch Indirect #13" min="1" max="1">casefuncmany\(\);</stringmatch>
<stringmatch name="Switch Indirect #14" min="1" max="1">casefuncdefault\(\);</stringmatch>
<stringmatch name="Switch Indirect #15" min="1" max="1">get_value_byref\(&amp;val\);</stringmatch>
<stringmatch name="Switch Indirect #16" min="1" max="1">switch\(val\)</stringmatch>
</decompilertest>