diff options
author | goldsteinn <35538541+goldsteinn@users.noreply.github.com> | 2024-10-17 11:32:55 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-10-17 10:32:55 -0500 |
commit | c85611e8583e6392d56075ebdfa60893b6284813 (patch) | |
tree | 0e9755aaff4332c655e985d957f821faca52661f /llvm | |
parent | ab208de34efbde4fea03732eaa353a701e72f626 (diff) | |
download | llvm-c85611e8583e6392d56075ebdfa60893b6284813.zip llvm-c85611e8583e6392d56075ebdfa60893b6284813.tar.gz llvm-c85611e8583e6392d56075ebdfa60893b6284813.tar.bz2 |
[SimplifyLibCall][Attribute] Fix bug where we may keep `range` attr with incompatible type (#112649)
In a variety of places we change the bitwidth of a parameter but don't
update the attributes.
The issue in this case is from the `range` attribute when inlining
`__memset_chk`. `optimizeMemSetChk` will replace an `i32` with an
`i8`, and if the `i32` had a `range` attr assosiated it will cause an
error.
Fixes #112633
Diffstat (limited to 'llvm')
-rw-r--r-- | llvm/include/llvm/IR/Argument.h | 2 | ||||
-rw-r--r-- | llvm/include/llvm/IR/Attributes.h | 16 | ||||
-rw-r--r-- | llvm/include/llvm/IR/InstrTypes.h | 16 | ||||
-rw-r--r-- | llvm/lib/Bitcode/Reader/BitcodeReader.cpp | 5 | ||||
-rw-r--r-- | llvm/lib/IR/Attributes.cpp | 7 | ||||
-rw-r--r-- | llvm/lib/IR/AutoUpgrade.cpp | 6 | ||||
-rw-r--r-- | llvm/lib/IR/Function.cpp | 4 | ||||
-rw-r--r-- | llvm/lib/IR/Verifier.cpp | 2 | ||||
-rw-r--r-- | llvm/lib/Target/AMDGPU/AMDGPULibCalls.cpp | 3 | ||||
-rw-r--r-- | llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp | 8 | ||||
-rw-r--r-- | llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp | 11 | ||||
-rw-r--r-- | llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp | 4 | ||||
-rw-r--r-- | llvm/lib/Transforms/Utils/CallPromotionUtils.cpp | 6 | ||||
-rw-r--r-- | llvm/lib/Transforms/Utils/CloneFunction.cpp | 4 | ||||
-rw-r--r-- | llvm/lib/Transforms/Utils/InlineFunction.cpp | 4 | ||||
-rw-r--r-- | llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp | 8 | ||||
-rw-r--r-- | llvm/test/Transforms/InstCombine/simplify-libcalls.ll | 12 | ||||
-rw-r--r-- | llvm/test/Verifier/range-attr.ll | 4 |
18 files changed, 88 insertions, 34 deletions
diff --git a/llvm/include/llvm/IR/Argument.h b/llvm/include/llvm/IR/Argument.h index 0ffcb05..5be58d7 100644 --- a/llvm/include/llvm/IR/Argument.h +++ b/llvm/include/llvm/IR/Argument.h @@ -182,6 +182,8 @@ public: Attribute getAttribute(Attribute::AttrKind Kind) const; + AttributeSet getAttributes() const; + /// Method for support type inquiry through isa, cast, and dyn_cast. static bool classof(const Value *V) { return V->getValueID() == ArgumentVal; diff --git a/llvm/include/llvm/IR/Attributes.h b/llvm/include/llvm/IR/Attributes.h index 57db52e..feeb3a9 100644 --- a/llvm/include/llvm/IR/Attributes.h +++ b/llvm/include/llvm/IR/Attributes.h @@ -1288,11 +1288,17 @@ enum AttributeSafetyKind : uint8_t { /// follows the same type rules as FPMathOperator. bool isNoFPClassCompatibleType(Type *Ty); -/// Which attributes cannot be applied to a type. The argument \p ASK indicates, -/// if only attributes that are known to be safely droppable are contained in -/// the mask; only attributes that might be unsafe to drop (e.g., ABI-related -/// attributes) are in the mask; or both. -AttributeMask typeIncompatible(Type *Ty, AttributeSafetyKind ASK = ASK_ALL); +/// Which attributes cannot be applied to a type. The argument \p AS +/// is used as a hint for the attributes whose compatibility is being +/// checked against \p Ty. This does not mean the return will be a +/// subset of \p AS, just that attributes that have specific dynamic +/// type compatibilities (i.e `range`) will be checked against what is +/// contained in \p AS. The argument \p ASK indicates, if only +/// attributes that are known to be safely droppable are contained in +/// the mask; only attributes that might be unsafe to drop (e.g., +/// ABI-related attributes) are in the mask; or both. +AttributeMask typeIncompatible(Type *Ty, AttributeSet AS, + AttributeSafetyKind ASK = ASK_ALL); /// Get param/return attributes which imply immediate undefined behavior if an /// invalid value is passed. For example, this includes noundef (where undef diff --git a/llvm/include/llvm/IR/InstrTypes.h b/llvm/include/llvm/IR/InstrTypes.h index 86d88da..99f7279 100644 --- a/llvm/include/llvm/IR/InstrTypes.h +++ b/llvm/include/llvm/IR/InstrTypes.h @@ -1453,14 +1453,22 @@ public: /// looking through to the attributes on the called function when necessary). ///@{ - /// Return the parameter attributes for this call. - /// + /// Return the attributes for this call. AttributeList getAttributes() const { return Attrs; } - /// Set the parameter attributes for this call. - /// + /// Set the attributes for this call. void setAttributes(AttributeList A) { Attrs = A; } + /// Return the return attributes for this call. + AttributeSet getRetAttributes() const { + return getAttributes().getRetAttrs(); + } + + /// Return the param attributes for this call. + AttributeSet getParamAttributes(unsigned ArgNo) const { + return getAttributes().getParamAttrs(ArgNo); + } + /// Try to intersect the attributes from 'this' CallBase and the /// 'Other' CallBase. Sets the intersected attributes to 'this' and /// return true if successful. Doesn't modify 'this' and returns diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp index 182c580..5a6fb50 100644 --- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp @@ -7040,11 +7040,12 @@ Error BitcodeReader::materialize(GlobalValue *GV) { // Remove incompatible attributes on function calls. if (auto *CI = dyn_cast<CallBase>(&I)) { CI->removeRetAttrs(AttributeFuncs::typeIncompatible( - CI->getFunctionType()->getReturnType())); + CI->getFunctionType()->getReturnType(), CI->getRetAttributes())); for (unsigned ArgNo = 0; ArgNo < CI->arg_size(); ++ArgNo) CI->removeParamAttrs(ArgNo, AttributeFuncs::typeIncompatible( - CI->getArgOperand(ArgNo)->getType())); + CI->getArgOperand(ArgNo)->getType(), + CI->getParamAttributes(ArgNo))); } } diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp index c2fba49..223c917 100644 --- a/llvm/lib/IR/Attributes.cpp +++ b/llvm/lib/IR/Attributes.cpp @@ -2300,7 +2300,7 @@ bool AttributeFuncs::isNoFPClassCompatibleType(Type *Ty) { } /// Which attributes cannot be applied to a type. -AttributeMask AttributeFuncs::typeIncompatible(Type *Ty, +AttributeMask AttributeFuncs::typeIncompatible(Type *Ty, AttributeSet AS, AttributeSafetyKind ASK) { AttributeMask Incompatible; @@ -2316,6 +2316,11 @@ AttributeMask AttributeFuncs::typeIncompatible(Type *Ty, // Attributes that only apply to integers or vector of integers. if (ASK & ASK_SAFE_TO_DROP) Incompatible.addAttribute(Attribute::Range); + } else { + Attribute RangeAttr = AS.getAttribute(Attribute::Range); + if (RangeAttr.isValid() && + RangeAttr.getRange().getBitWidth() != Ty->getScalarSizeInBits()) + Incompatible.addAttribute(Attribute::Range); } if (!Ty->isPointerTy()) { diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp index 3aceb52..bb03c92 100644 --- a/llvm/lib/IR/AutoUpgrade.cpp +++ b/llvm/lib/IR/AutoUpgrade.cpp @@ -5378,9 +5378,11 @@ void llvm::UpgradeFunctionAttributes(Function &F) { } // Remove all incompatibile attributes from function. - F.removeRetAttrs(AttributeFuncs::typeIncompatible(F.getReturnType())); + F.removeRetAttrs(AttributeFuncs::typeIncompatible( + F.getReturnType(), F.getAttributes().getRetAttrs())); for (auto &Arg : F.args()) - Arg.removeAttrs(AttributeFuncs::typeIncompatible(Arg.getType())); + Arg.removeAttrs( + AttributeFuncs::typeIncompatible(Arg.getType(), Arg.getAttributes())); // Older versions of LLVM treated an "implicit-section-name" attribute // similarly to directly setting the section on a Function. diff --git a/llvm/lib/IR/Function.cpp b/llvm/lib/IR/Function.cpp index 09b9071..8892959 100644 --- a/llvm/lib/IR/Function.cpp +++ b/llvm/lib/IR/Function.cpp @@ -359,6 +359,10 @@ Attribute Argument::getAttribute(Attribute::AttrKind Kind) const { return getParent()->getParamAttribute(getArgNo(), Kind); } +AttributeSet Argument::getAttributes() const { + return getParent()->getAttributes().getParamAttrs(getArgNo()); +} + //===----------------------------------------------------------------------===// // Helper Methods in Function //===----------------------------------------------------------------------===// diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp index 0412b93..f34fe75 100644 --- a/llvm/lib/IR/Verifier.cpp +++ b/llvm/lib/IR/Verifier.cpp @@ -2012,7 +2012,7 @@ void Verifier::verifyParameterAttrs(AttributeSet Attrs, Type *Ty, Attrs.hasAttribute(Attribute::ReadOnly)), "Attributes writable and readonly are incompatible!", V); - AttributeMask IncompatibleAttrs = AttributeFuncs::typeIncompatible(Ty); + AttributeMask IncompatibleAttrs = AttributeFuncs::typeIncompatible(Ty, Attrs); for (Attribute Attr : Attrs) { if (!Attr.isStringAttribute() && IncompatibleAttrs.contains(Attr.getKindAsEnum())) { diff --git a/llvm/lib/Target/AMDGPU/AMDGPULibCalls.cpp b/llvm/lib/Target/AMDGPU/AMDGPULibCalls.cpp index eb553ae..26d8ce7 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPULibCalls.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPULibCalls.cpp @@ -788,7 +788,8 @@ bool AMDGPULibCalls::fold(CallInst *CI) { B.CreateFPToSI(FPOp->getOperand(1), PownType->getParamType(1)); // Have to drop any nofpclass attributes on the original call site. Call->removeParamAttrs( - 1, AttributeFuncs::typeIncompatible(CastedArg->getType())); + 1, AttributeFuncs::typeIncompatible(CastedArg->getType(), + Call->getParamAttributes(1))); Call->setCalledFunction(PownFunc); Call->setArgOperand(1, CastedArg); return fold_pow(FPOp, B, PownInfo) || true; diff --git a/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp b/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp index d154859..ed93b44 100644 --- a/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp +++ b/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp @@ -857,9 +857,10 @@ bool DeadArgumentEliminationPass::removeDeadStuffFromFunction(Function *F) { // here. Currently, this should not be possible, but special handling might be // required when new return value attributes are added. if (NRetTy->isVoidTy()) - RAttrs.remove(AttributeFuncs::typeIncompatible(NRetTy)); + RAttrs.remove(AttributeFuncs::typeIncompatible(NRetTy, PAL.getRetAttrs())); else - assert(!RAttrs.overlaps(AttributeFuncs::typeIncompatible(NRetTy)) && + assert(!RAttrs.overlaps( + AttributeFuncs::typeIncompatible(NRetTy, PAL.getRetAttrs())) && "Return attributes no longer compatible?"); AttributeSet RetAttrs = AttributeSet::get(F->getContext(), RAttrs); @@ -903,7 +904,8 @@ bool DeadArgumentEliminationPass::removeDeadStuffFromFunction(Function *F) { // Adjust the call return attributes in case the function was changed to // return void. AttrBuilder RAttrs(F->getContext(), CallPAL.getRetAttrs()); - RAttrs.remove(AttributeFuncs::typeIncompatible(NRetTy)); + RAttrs.remove( + AttributeFuncs::typeIncompatible(NRetTy, CallPAL.getRetAttrs())); AttributeSet RetAttrs = AttributeSet::get(F->getContext(), RAttrs); // Declare these outside of the loops, so we can reuse them for the second diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp index 37841e9..07b9405 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp @@ -4151,7 +4151,8 @@ bool InstCombinerImpl::transformConstExprCastCall(CallBase &Call) { if (!CallerPAL.isEmpty() && !Caller->use_empty()) { AttrBuilder RAttrs(FT->getContext(), CallerPAL.getRetAttrs()); - if (RAttrs.overlaps(AttributeFuncs::typeIncompatible(NewRetTy))) + if (RAttrs.overlaps(AttributeFuncs::typeIncompatible( + NewRetTy, CallerPAL.getRetAttrs()))) return false; // Attribute not compatible with transformed value. } @@ -4197,7 +4198,8 @@ bool InstCombinerImpl::transformConstExprCastCall(CallBase &Call) { // Check if there are any incompatible attributes we cannot drop safely. if (AttrBuilder(FT->getContext(), CallerPAL.getParamAttrs(i)) .overlaps(AttributeFuncs::typeIncompatible( - ParamTy, AttributeFuncs::ASK_UNSAFE_TO_DROP))) + ParamTy, CallerPAL.getParamAttrs(i), + AttributeFuncs::ASK_UNSAFE_TO_DROP))) return false; // Attribute not compatible with transformed value. if (Call.isInAllocaArgument(i) || @@ -4235,7 +4237,8 @@ bool InstCombinerImpl::transformConstExprCastCall(CallBase &Call) { // If the return value is not being used, the type may not be compatible // with the existing attributes. Wipe out any problematic attributes. - RAttrs.remove(AttributeFuncs::typeIncompatible(NewRetTy)); + RAttrs.remove( + AttributeFuncs::typeIncompatible(NewRetTy, CallerPAL.getRetAttrs())); LLVMContext &Ctx = Call.getContext(); AI = Call.arg_begin(); @@ -4250,7 +4253,7 @@ bool InstCombinerImpl::transformConstExprCastCall(CallBase &Call) { // Add any parameter attributes except the ones incompatible with the new // type. Note that we made sure all incompatible ones are safe to drop. AttributeMask IncompatibleAttrs = AttributeFuncs::typeIncompatible( - ParamTy, AttributeFuncs::ASK_SAFE_TO_DROP); + ParamTy, CallerPAL.getParamAttrs(i), AttributeFuncs::ASK_SAFE_TO_DROP); ArgAttrs.push_back( CallerPAL.getParamAttrs(i).removeAttributes(Ctx, IncompatibleAttrs)); } diff --git a/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp index 577647c..e226727 100644 --- a/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp @@ -1305,8 +1305,8 @@ DataFlowSanitizer::buildWrapperFunction(Function *F, StringRef NewFName, Function *NewF = Function::Create(NewFT, NewFLink, F->getAddressSpace(), NewFName, F->getParent()); NewF->copyAttributesFrom(F); - NewF->removeRetAttrs( - AttributeFuncs::typeIncompatible(NewFT->getReturnType())); + NewF->removeRetAttrs(AttributeFuncs::typeIncompatible( + NewFT->getReturnType(), NewF->getAttributes().getRetAttrs())); BasicBlock *BB = BasicBlock::Create(*Ctx, "entry", NewF); if (F->isVarArg()) { diff --git a/llvm/lib/Transforms/Utils/CallPromotionUtils.cpp b/llvm/lib/Transforms/Utils/CallPromotionUtils.cpp index 4ead5cd..17cba2e6 100644 --- a/llvm/lib/Transforms/Utils/CallPromotionUtils.cpp +++ b/llvm/lib/Transforms/Utils/CallPromotionUtils.cpp @@ -529,7 +529,8 @@ CallBase &llvm::promoteCall(CallBase &CB, Function *Callee, // Remove any incompatible attributes for the argument. AttrBuilder ArgAttrs(Ctx, CallerPAL.getParamAttrs(ArgNo)); - ArgAttrs.remove(AttributeFuncs::typeIncompatible(FormalTy)); + ArgAttrs.remove(AttributeFuncs::typeIncompatible( + FormalTy, CallerPAL.getParamAttrs(ArgNo))); // We may have a different byval/inalloca type. if (ArgAttrs.getByValType()) @@ -549,7 +550,8 @@ CallBase &llvm::promoteCall(CallBase &CB, Function *Callee, AttrBuilder RAttrs(Ctx, CallerPAL.getRetAttrs()); if (!CallSiteRetTy->isVoidTy() && CallSiteRetTy != CalleeRetTy) { createRetBitCast(CB, CallSiteRetTy, RetBitCast); - RAttrs.remove(AttributeFuncs::typeIncompatible(CalleeRetTy)); + RAttrs.remove( + AttributeFuncs::typeIncompatible(CalleeRetTy, CallerPAL.getRetAttrs())); AttributeChanged = true; } diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp index c6ba85b..5dc82a8 100644 --- a/llvm/lib/Transforms/Utils/CloneFunction.cpp +++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp @@ -819,9 +819,9 @@ void llvm::CloneAndPruneIntoFromInst(Function *NewFunc, const Function *OldFunc, // Drop all incompatible return attributes that cannot be applied to NewFunc // during cloning, so as to allow instruction simplification to reason on the // old state of the function. The original attributes are restored later. - AttributeMask IncompatibleAttrs = - AttributeFuncs::typeIncompatible(OldFunc->getReturnType()); AttributeList Attrs = NewFunc->getAttributes(); + AttributeMask IncompatibleAttrs = AttributeFuncs::typeIncompatible( + OldFunc->getReturnType(), Attrs.getRetAttrs()); NewFunc->removeRetAttrs(IncompatibleAttrs); // As phi-nodes have been now remapped, allow incremental simplification of diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp index 13eb588..a0a93dc 100644 --- a/llvm/lib/Transforms/Utils/InlineFunction.cpp +++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp @@ -3057,8 +3057,8 @@ llvm::InlineResult llvm::InlineFunction(CallBase &CB, InlineFunctionInfo &IFI, else Builder.CreateRet(NewDeoptCall); // Since the ret type is changed, remove the incompatible attributes. - NewDeoptCall->removeRetAttrs( - AttributeFuncs::typeIncompatible(NewDeoptCall->getType())); + NewDeoptCall->removeRetAttrs(AttributeFuncs::typeIncompatible( + NewDeoptCall->getType(), NewDeoptCall->getRetAttributes())); } // Leave behind the normal returns so we can merge control flow. diff --git a/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp b/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp index cb4ef87..79e91ad 100644 --- a/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp +++ b/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp @@ -342,7 +342,13 @@ static Value *copyFlags(const CallInst &Old, Value *New) { static Value *mergeAttributesAndFlags(CallInst *NewCI, const CallInst &Old) { NewCI->setAttributes(AttributeList::get( NewCI->getContext(), {NewCI->getAttributes(), Old.getAttributes()})); - NewCI->removeRetAttrs(AttributeFuncs::typeIncompatible(NewCI->getType())); + NewCI->removeRetAttrs(AttributeFuncs::typeIncompatible( + NewCI->getType(), NewCI->getRetAttributes())); + for (unsigned I = 0; I < NewCI->arg_size(); ++I) + NewCI->removeParamAttrs( + I, AttributeFuncs::typeIncompatible(NewCI->getArgOperand(I)->getType(), + NewCI->getParamAttributes(I))); + return copyFlags(Old, NewCI); } diff --git a/llvm/test/Transforms/InstCombine/simplify-libcalls.ll b/llvm/test/Transforms/InstCombine/simplify-libcalls.ll index bb2728a..c4ae7e7e 100644 --- a/llvm/test/Transforms/InstCombine/simplify-libcalls.ll +++ b/llvm/test/Transforms/InstCombine/simplify-libcalls.ll @@ -342,5 +342,17 @@ define signext i32 @emit_stpncpy() { ret i32 0 } +define void @simplify_memset_chk_pr112633(ptr %p, i32 %conv) { +; CHECK-LABEL: @simplify_memset_chk_pr112633( +; CHECK-NEXT: [[CALL_I:%.*]] = tail call ptr @__memset_chk(ptr [[P:%.*]], i32 range(i32 0, 123) [[CONV:%.*]], i64 1, i64 1) +; CHECK-NEXT: ret void +; + %call.i = tail call ptr @__memset_chk(ptr %p, i32 range(i32 0, 123) %conv, i64 1, i64 1) + ret void +} + +declare ptr @__memset_chk(ptr, i32, i64, i64) + + attributes #0 = { nobuiltin } attributes #1 = { builtin } diff --git a/llvm/test/Verifier/range-attr.ll b/llvm/test/Verifier/range-attr.ll index f985ab696..9141236 100644 --- a/llvm/test/Verifier/range-attr.ll +++ b/llvm/test/Verifier/range-attr.ll @@ -1,12 +1,12 @@ ; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s -; CHECK: Range bit width must match type bit width! +; CHECK: Attribute 'range(i8 1, 0)' applied to incompatible type! ; CHECK-NEXT: ptr @bit_widths_do_not_match define void @bit_widths_do_not_match(i32 range(i8 1, 0) %a) { ret void } -; CHECK: Range bit width must match type bit width! +; CHECK: Attribute 'range(i8 1, 0)' applied to incompatible type! ; CHECK-NEXT: ptr @bit_widths_do_not_match_vector define void @bit_widths_do_not_match_vector(<4 x i32> range(i8 1, 0) %a) { ret void |