[NFC] Avoid nullptr dereference

[FreddyYe]

Summary

These warning are reported by static code analysis tool: Klocwork

[skan]

llvm/lib/CodeGen/TypePromotion.cpp

259–260

You mean

Return->getReturnValue() may return a NULL pointer but LessOrEqualTypeSize assumes its argument is never be a NULL?

If the answer is yes, you need to provide a test case tiggering the compfail.

[pengfei]

llvm/lib/CodeGen/TypePromotion.cpp

259–260

Agreed with @skan . You changed the functionality here.
Besides, I didn't find difference between StoreInst and ReturnInst, why only warn here?

[FreddyYe]

Besides, I didn't find difference between StoreInst and ReturnInst, why only warn here?

The difference is Store->getValueOperand() doesn't return a nullptr but Return->getValueOperand() does.

Hi both, thx for review. This is reported by static code analysis tool, so I don't have a reproduce. I can also add assert() to these callees to satisfy the tool, but there will be more than only one of them. Does that sound OK to you?

[FreddyYe]

changed into assert()

[skan]

In D113164#3108071, @FreddyYe wrote:

Hi both, thx for review. This is reported by static code analysis tool, so I don't have a reproduce. I can also add assert() to these callees to satisfy the tool, but there will be more than only one of them. Does that sound OK to you?

Why not add the assert in this if clause if you make sure Return->getReturnValue() can never be NULL here.

[skan]

llvm/lib/CodeGen/TypePromotion.cpp

207

I don't know if this assert is necessary...
https://llvm.org/docs/CodingStandards.html#assert-liberally
The code standard suggests using the “assert” macro to its fullest. However I think the NULL ptr check can be omitted b/c we can get error message like "null pointer deference" at run time even w/o the assert.

[FreddyYe]

After reading the whole logic, I'll judge this as not a defect. Since TypePromotion::isSupportedValue() has checked before codes here. ReturnInst's operand won't be null. So getReturnValue() here won't return Null. I'll close this PR, sorry for the noise.

llvm/lib/CodeGen/TypePromotion.cpp

return Opc == Instruction::AShr || Opc == Instruction::SDiv ||
          Opc == Instruction::SRem || Opc == Instruction::SExt;
 }
 
 bool TypePromotion::EqualTypeSize(Value *V) {
   return V->getType()->getScalarSizeInBits() == TypeSize;
 }
 
 bool TypePromotion::LessOrEqualTypeSize(Value *V) {
+  assert(!V);
   return V->getType()->getScalarSizeInBits() <= TypeSize;
 }
 
 bool TypePromotion::GreaterThanTypeSize(Value *V) {
   return V->getType()->getScalarSizeInBits() > TypeSize;
 }
 
 bool TypePromotion::LessThanTypeSize(Value *V) {
bool TypePromotion::isSink(Value *V) {
   // Sinks are:
   // - points where the value in the register is being observed, such as an
   //   icmp, switch or store.
   // - points where value types have to match, such as calls and returns.
   // - zext are included to ease the transformation and are generally removed
   //   later on.
   if (auto *Store = dyn_cast<StoreInst>(V))
     return LessOrEqualTypeSize(Store->getValueOperand());
   if (auto *Return = dyn_cast<ReturnInst>(V))
     return LessOrEqualTypeSize(Return->getReturnValue());
   if (auto *ZExt = dyn_cast<ZExtInst>(V))
     return GreaterThanTypeSize(ZExt);
   if (auto *Switch = dyn_cast<SwitchInst>(V))
     return LessThanTypeSize(Switch->getCondition());
   if (auto *ICmp = dyn_cast<ICmpInst>(V))
     return ICmp->isSigned() || LessThanTypeSize(ICmp->getOperand(0));
 
   return isa<CallInst>(V);