[IR] add bounds checking to paramHasAttr

[sanjoy]

Summary

This is intended to make a later change simpler.

Note: adding this bounds checking required fixing X86FastISel. As
far I can tell I've preserved original behavior but a careful review
will be appreciated.

[reames]

LGTM

Do we intend to allow ReturnIndex here?

[sanjoy]

In D14304#280567, @reames wrote:

LGTM

Do we intend to allow ReturnIndex here?

At least for now I think we should allow ReturnIndex here since there isn't an existing hasReturnAttr.

[sanjoy]

• make the bounds checking stricter, and fix an out of bounds access

[sanjoy]

@ributzka, @eli.friedman -- can one of you please take a look at the X86 FastISel fix?

[eli.friedman]

The FastIsel changes appear to be correct... but probably not the most clear way to write it. It would be better to combine the if statements so it only checks arg_size != 0 once.

Granted, I'm just basing that assessment off of r160725. If I did have any special knowledge here, I've forgotten it.

[sanjoy]

• address review: clean up the edit to X86FastISel.cpp

[reames]

LGTM w/minor comment addressed given the FastISel correctness is now covered.

lib/Target/X86/X86FastISel.cpp

2822 ↗

(On Diff #39160)

This would be clearer as a single if within the CS one.
if (CS->arg_empty() ||

  no struct ret ||
  inref) 
return 0

llvm/trunk/lib/IR/Instructions.cpp

 
 void CallInst::addDereferenceableOrNullAttr(unsigned i, uint64_t Bytes) {
   AttributeSet PAL = getAttributes();
   PAL = PAL.addDereferenceableOrNullAttr(getContext(), i, Bytes);
   setAttributes(PAL);
 }
 
 bool CallInst::paramHasAttr(unsigned i, Attribute::AttrKind A) const {
+  assert(i < (getNumArgOperands() + 1) && "Param index out of bounds!");
+
   if (AttributeList.hasAttribute(i, A))
     return true;
   if (const Function *F = getCalledFunction())
     return F->getAttributes().hasAttribute(i, A);
   return false;
 }
 
 /// IsConstantOne - Return true only if val is constant int 1
if (isFnAttrDisallowedByOpBundle(A))
     return false;
 
   if (const Function *F = getCalledFunction())
     return F->getAttributes().hasAttribute(AttributeSet::FunctionIndex, A);
   return false;
 }
 
 bool InvokeInst::paramHasAttr(unsigned i, Attribute::AttrKind A) const {
+  assert(i < (getNumArgOperands() + 1) && "Param index out of bounds!");
+
   if (AttributeList.hasAttribute(i, A))
     return true;
   if (const Function *F = getCalledFunction())
     return F->getAttributes().hasAttribute(i, A);
   return false;
 }
 
 void InvokeInst::addAttribute(unsigned i, Attribute::AttrKind attr) {

llvm/trunk/lib/Target/X86/X86FastISel.cpp

static unsigned computeBytesPoppedByCallee(const X86Subtarget *Subtarget,
                                            ImmutableCallSite *CS) {
   if (Subtarget->is64Bit())
     return 0;
   if (Subtarget->getTargetTriple().isOSMSVCRT())
     return 0;
   if (CC == CallingConv::Fast || CC == CallingConv::GHC ||
       CC == CallingConv::HiPE)
     return 0;
-  if (CS && !CS->paramHasAttr(1, Attribute::StructRet))
-    return 0;
-  if (CS && CS->paramHasAttr(1, Attribute::InReg))
+
+  if (CS)
+    if (CS->arg_empty() || !CS->paramHasAttr(1, Attribute::StructRet) ||
+        CS->paramHasAttr(1, Attribute::InReg))
       return 0;
+
   return 4;
 }
 
 bool X86FastISel::fastLowerCall(CallLoweringInfo &CLI) {
   auto &OutVals       = CLI.OutVals;
   auto &OutFlags      = CLI.OutFlags;
   auto &OutRegs       = CLI.OutRegs;
   auto &Ins           = CLI.Ins;