intel / intel-graphics-compiler

Other
594 stars 155 forks source link

Null pointer dereference is not possible due to preconditions. #324

Closed lsatanov closed 4 months ago

lsatanov commented 5 months ago

Hi, @vmustya.

https://github.com/intel/intel-graphics-compiler/blob/8266ce17d23dedb98e5fb35b0e0352544bec7d09/IGC/VectorCompiler/lib/GenXCodeGen/GenXGVClobberChecker.cpp#L314

Hi. The change introduced in this commit: https://github.com/intel/intel-graphics-compiler/commit/7088ebda63bb5a653e79dc439048e94b5515f03a is based on assumption that nullptr dereference is possible here, however it is not due to the following preconditions:

1) genx.vload-retrieved value users (which are being iterated here) can not be anything but Instruction. 2) genx::peelBitCastsWhileSingleUserChain(U) will not return anything but llvm::Instruction because of (1).

Also, genx::peelBitCastsWhileSingleUserChain(...) is excessive here.

Therefore the more appropriate change would be replacing dyn_cast\ found in the initial code with assertion (which is excessive)+ cast\ like this:

  for (const auto &U : LI->users()) {
    // excessive IGC_ASSERT(isa<Instruction>(U));
    Detected |= CheckUserInst(
        CheckGVClobbOpt_ChkWithBales
            ? Baling->getBaleHead(cast<Instruction>(U))
            : cast<Instruction>(U));
  }
pszymich commented 4 months ago

Hi @lsatanov, we've committed your suggestion: https://github.com/intel/intel-graphics-compiler/commit/4a178bfdb5903dc6d51d2802fca2a2efe560c1f5

Thanks!