cisco-open / llvm-crash-analyzer

llvm crash analysis
Apache License 2.0
41 stars 17 forks source link

[TA] Treat XOR reg invalidation as zero load #19

Closed niktesic closed 2 years ago

niktesic commented 2 years ago

The XOR instruction is often used to invalidate a register value (example below), which has the same effect as loading zero into the same register. $edi = XOR32rr undef $edi(tied-def 0), undef $edi == $edi = MOV32ri 0

This instruction should terminate the Taint Analysis for the current branch, as it is treated as "Constant Found" case. However, the taint is propagated from $edi register to itself, and then again processed as the Constant is not found. This results in additional node in the Taint DFG, representing the same XOR instruction, and connected to the first one with a Deref Edge. (node with ID 5 in the graph image below)

dfg

Please, notice that nodes with ID 7 and 8, are incorrectly present, which should be fixed in the future. With this patch, graph looks like in the image below. There is no second XOR instruction node, and the right one is represented as loading constant zero, and not constant equal to the register $edi.

new

The second commit stops the TaintAnalysis if the TaintList is empty, after a processed function. In some cases that could be a valid situation and in some other cases that could be a premature termination. With the second commit, the DFG from above examples looks like in the following picture.

newdfg

niktesic commented 2 years ago

Thanks for catching and fixing this. Should we handle other similar cases (like and $reg, 0, for example) ?

Thanks for the review! I agree we should handle instructions with a similar effect, in the same manner, I need to catch some of those cases.