cisco-open / llvm-crash-analyzer

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

Upper frame crash-start sunk into next BB #34

Open niktesic opened 1 year ago

niktesic commented 1 year ago

Test source code:

void crash(int* ptr) {
  *ptr = 3;
}

int main(){
  int* p;
  int cond = 1;
  p = 0x0;
  if (cond)
    crash(p);
  cond += 2;
  return cond - *p;
}

Compiled with: $ clang -g -O0 m.c -o m

In the decompiled MIR, for the presented test case, in the main() frame, call instruction (corresponding to crash(p)) is the last in it's Basic block. Since Decompiler marks with crash-start flag, the instruction after call, it means that crash-start instruction for main() frame is first in it's Basic block. For this reason, Taint Analysis is skipped for function main() and any upper frames in similar cases.

ananthakri commented 1 year ago

It does not make sense why main() is skipped. Is there an issue in starting analysis at the start of a basic-block? The function main() should be analyzed from call-site( crash()) back to entry point of main.

niktesic commented 1 year ago

It does not make sense why main() is skipped. Is there an issue in starting analysis at the start of a basic-block? The function main() should be analyzed from call-site( crash()) back to entry point of main.

The current Taint Analysis implementation expects the following sequence of instructions for any upper frame: 1) First analyzed instruction for that frame 2) Call instruction to the lower frame 3) Crash-start instruction after the call Whole sequence needs to be in the same Basic Block.

The code which prevents analysis if the sequence is not as expected is at crash_analyzer::TaintAnalysis::runOnBlameMF.

There are two possible solutions that come to my mind: 1) Add artificial NOP after Call instruction to ensure there is expected sequence 2) Make TA resilient to such cases, and look for the first analyzed instruction in the previous BB.

For this reason, Taint Analysis is skipped for function main() and any upper frames in similar cases. My mistake, the TA would be performed for upper frames, but the corrupted one (without expected instruction sequence) would be skipped.

Thanks, Nikola