From: Erich Keane Date: Thu, 10 Oct 2019 21:08:28 +0000 (+0000) Subject: Fix __builtin_assume_aligned with too large values. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=726918e196b413bcd80f08494c061fd6b3f26c94;p=clang Fix __builtin_assume_aligned with too large values. Code to handle __builtin_assume_aligned was allowing larger values, but would convert this to unsigned along the way. This patch removes the EmitAssumeAligned overloads that take unsigned to do away with this problem. Additionally, it adds a warning that values greater than 1 <<29 are ignored by LLVM. Differential Revision: https://reviews.llvm.org/D68824 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@374450 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index bcd059be91..68f2b37950 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -2853,6 +2853,10 @@ def err_alignment_dependent_typedef_name : Error< def err_attribute_aligned_too_great : Error< "requested alignment must be %0 bytes or smaller">; +def warn_assume_aligned_too_great + : Warning<"requested alignment must be %0 bytes or smaller; assumption " + "ignored">, + InGroup>; def warn_redeclaration_without_attribute_prev_attribute_ignored : Warning< "%q0 redeclared without %1 attribute: previous %1 ignored">, InGroup; diff --git a/lib/CodeGen/CGBuiltin.cpp b/lib/CodeGen/CGBuiltin.cpp index 0562a623ae..e6369d80cd 100644 --- a/lib/CodeGen/CGBuiltin.cpp +++ b/lib/CodeGen/CGBuiltin.cpp @@ -2048,11 +2048,10 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, Value *AlignmentValue = EmitScalarExpr(E->getArg(1)); ConstantInt *AlignmentCI = cast(AlignmentValue); - unsigned Alignment = (unsigned)AlignmentCI->getZExtValue(); EmitAlignmentAssumption(PtrValue, Ptr, /*The expr loc is sufficient.*/ SourceLocation(), - Alignment, OffsetValue); + AlignmentCI, OffsetValue); return RValue::get(PtrValue); } case Builtin::BI__assume: diff --git a/lib/CodeGen/CGCall.cpp b/lib/CodeGen/CGCall.cpp index 682a7ccb49..2ec9e91e31 100644 --- a/lib/CodeGen/CGCall.cpp +++ b/lib/CodeGen/CGCall.cpp @@ -4569,7 +4569,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, llvm::Value *Alignment = EmitScalarExpr(AA->getAlignment()); llvm::ConstantInt *AlignmentCI = cast(Alignment); EmitAlignmentAssumption(Ret.getScalarVal(), RetTy, Loc, AA->getLocation(), - AlignmentCI->getZExtValue(), OffsetValue); + AlignmentCI, OffsetValue); } else if (const auto *AA = TargetDecl->getAttr()) { llvm::Value *AlignmentVal = CallArgs[AA->getParamIndex().getLLVMIndex()] .getRValue(*this) diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp index 0b9271b1d3..c7f279d28e 100644 --- a/lib/CodeGen/CGExprScalar.cpp +++ b/lib/CodeGen/CGExprScalar.cpp @@ -294,8 +294,7 @@ public: Value *AlignmentValue = CGF.EmitScalarExpr(AVAttr->getAlignment()); llvm::ConstantInt *AlignmentCI = cast(AlignmentValue); - CGF.EmitAlignmentAssumption(V, E, AVAttr->getLocation(), - AlignmentCI->getZExtValue()); + CGF.EmitAlignmentAssumption(V, E, AVAttr->getLocation(), AlignmentCI); } /// EmitLoadOfLValue - Given an expression with complex type that represents a diff --git a/lib/CodeGen/CGStmtOpenMP.cpp b/lib/CodeGen/CGStmtOpenMP.cpp index e50cdb1b67..fa4304d7e5 100644 --- a/lib/CodeGen/CGStmtOpenMP.cpp +++ b/lib/CodeGen/CGStmtOpenMP.cpp @@ -1519,14 +1519,14 @@ static void emitAlignedClause(CodeGenFunction &CGF, if (!CGF.HaveInsertPoint()) return; for (const auto *Clause : D.getClausesOfKind()) { - unsigned ClauseAlignment = 0; + size_t ClauseAlignment = 0; if (const Expr *AlignmentExpr = Clause->getAlignment()) { auto *AlignmentCI = cast(CGF.EmitScalarExpr(AlignmentExpr)); - ClauseAlignment = static_cast(AlignmentCI->getZExtValue()); + ClauseAlignment = AlignmentCI->getZExtValue(); } for (const Expr *E : Clause->varlists()) { - unsigned Alignment = ClauseAlignment; + size_t Alignment = ClauseAlignment; if (Alignment == 0) { // OpenMP [2.8.1, Description] // If no optional parameter is specified, implementation-defined default @@ -1542,7 +1542,8 @@ static void emitAlignedClause(CodeGenFunction &CGF, if (Alignment != 0) { llvm::Value *PtrValue = CGF.EmitScalarExpr(E); CGF.EmitAlignmentAssumption( - PtrValue, E, /*No second loc needed*/ SourceLocation(), Alignment); + PtrValue, E, /*No second loc needed*/ SourceLocation(), + llvm::ConstantInt::get(CGF.SizeTy, Alignment)); } } } diff --git a/lib/CodeGen/CodeGenFunction.cpp b/lib/CodeGen/CodeGenFunction.cpp index 41b7f2f4b1..3f9a52ab76 100644 --- a/lib/CodeGen/CodeGenFunction.cpp +++ b/lib/CodeGen/CodeGenFunction.cpp @@ -2056,25 +2056,10 @@ void CodeGenFunction::EmitAlignmentAssumption(llvm::Value *PtrValue, } } -void CodeGenFunction::EmitAlignmentAssumption(llvm::Value *PtrValue, - QualType Ty, SourceLocation Loc, - SourceLocation AssumptionLoc, - unsigned Alignment, - llvm::Value *OffsetValue) { - llvm::Value *TheCheck; - llvm::Instruction *Assumption = Builder.CreateAlignmentAssumption( - CGM.getDataLayout(), PtrValue, Alignment, OffsetValue, &TheCheck); - if (SanOpts.has(SanitizerKind::Alignment)) { - llvm::Value *AlignmentVal = llvm::ConstantInt::get(IntPtrTy, Alignment); - EmitAlignmentAssumptionCheck(PtrValue, Ty, Loc, AssumptionLoc, AlignmentVal, - OffsetValue, TheCheck, Assumption); - } -} - void CodeGenFunction::EmitAlignmentAssumption(llvm::Value *PtrValue, const Expr *E, SourceLocation AssumptionLoc, - unsigned Alignment, + llvm::Value *Alignment, llvm::Value *OffsetValue) { if (auto *CE = dyn_cast(E)) E = CE->getSubExprAsWritten(); diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h index ea78309a06..bb4fed8083 100644 --- a/lib/CodeGen/CodeGenFunction.h +++ b/lib/CodeGen/CodeGenFunction.h @@ -2829,13 +2829,8 @@ public: llvm::Value *Alignment, llvm::Value *OffsetValue = nullptr); - void EmitAlignmentAssumption(llvm::Value *PtrValue, QualType Ty, - SourceLocation Loc, SourceLocation AssumptionLoc, - unsigned Alignment, - llvm::Value *OffsetValue = nullptr); - void EmitAlignmentAssumption(llvm::Value *PtrValue, const Expr *E, - SourceLocation AssumptionLoc, unsigned Alignment, + SourceLocation AssumptionLoc, llvm::Value *Alignment, llvm::Value *OffsetValue = nullptr); //===--------------------------------------------------------------------===// diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index 07d3648dc9..b35d591a7f 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -6063,6 +6063,16 @@ bool Sema::SemaBuiltinAssumeAligned(CallExpr *TheCall) { if (!Result.isPowerOf2()) return Diag(TheCall->getBeginLoc(), diag::err_alignment_not_power_of_two) << Arg->getSourceRange(); + + // FIXME: this should probably use llvm::Value::MaximumAlignment, however + // doing so results in a linking issue in GCC in a couple of assemblies. + // Alignment calculations can wrap around if it's greater than 2**28. + unsigned MaximumAlignment = + Context.getTargetInfo().getTriple().isOSBinFormatCOFF() ? 8192 + : 268435456; + if (Result > MaximumAlignment) + Diag(TheCall->getBeginLoc(), diag::warn_assume_aligned_too_great) + << Arg->getSourceRange() << MaximumAlignment; } if (NumArgs > 2) { diff --git a/test/Sema/builtin-assume-aligned.c b/test/Sema/builtin-assume-aligned.c index 057a500b32..a669ed172d 100644 --- a/test/Sema/builtin-assume-aligned.c +++ b/test/Sema/builtin-assume-aligned.c @@ -58,3 +58,7 @@ void *test_no_fn_proto() __attribute__((assume_aligned)); // expected-error {{'a void *test_no_fn_proto() __attribute__((assume_aligned())); // expected-error {{'assume_aligned' attribute takes at least 1 argument}} void *test_no_fn_proto() __attribute__((assume_aligned(32, 45, 37))); // expected-error {{'assume_aligned' attribute takes no more than 2 arguments}} +int pr43638(int *a) { + a = __builtin_assume_aligned(a, 4294967296); // expected-warning {{requested alignment must be 268435456 bytes or smaller; assumption ignored}} +return a[0]; +}