lifting-bits / remill

Library for lifting machine code to LLVM bitcode
Apache License 2.0
1.22k stars 143 forks source link

LiftCBranch does not set BRANCH_TAKEN #694

Closed m4xw closed 5 months ago

m4xw commented 5 months ago

Hello, I've been implementing a MIPS backend via Sleigh and noticed the conditional blocks are not evaluating branch_taken correctly because it's never stored. It looks to me that SPARC/AArch etc do have some own handling so this might've caused a oversight? I've seen the same happen with the PPC backend, which my MIPS backend is based on.

I have this as a fix right now in LiftCBranch, but given that LiftBranchTaken sets it as well I am not sure if this is the appropriate way, it might just need some minor refactoring (given LiftBranchTaken is currently a stub since its only called via LiftBtakenIfReached which is never invoked)?

image

      auto orig_pc_value = this->GetNextPc(bldr);
      //CHECK(pc_reg_param.has_value());
      auto next_pc_value =
          bldr.CreateSelect(trunc_should_branch, jump_addr, orig_pc_value);
      auto should_branch_value =
          bldr.CreateSelect(trunc_should_branch, 
          llvm::ConstantInt::get(llvm::Type::getInt8Ty(context), 1),
          llvm::ConstantInt::get(llvm::Type::getInt8Ty(context), 0));

      bldr.CreateStore(should_branch_value, this->GetBranchTakenRef());
      bldr.CreateStore(next_pc_value, this->GetNextPcRef());

Here how it looks with my MIPS backend with the change: image

Before the change (Pulling uninitialized memory from stack): image