Skip to content

Commit 31e454c

Browse files
author
Erich Keane
committed
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 llvm-svn: 374450
1 parent 5e866e4 commit 31e454c

File tree

9 files changed

+28
-31
lines changed

9 files changed

+28
-31
lines changed

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2853,6 +2853,10 @@ def err_alignment_dependent_typedef_name : Error<
28532853

28542854
def err_attribute_aligned_too_great : Error<
28552855
"requested alignment must be %0 bytes or smaller">;
2856+
def warn_assume_aligned_too_great
2857+
: Warning<"requested alignment must be %0 bytes or smaller; assumption "
2858+
"ignored">,
2859+
InGroup<DiagGroup<"builtin-assume-aligned-alignment">>;
28562860
def warn_redeclaration_without_attribute_prev_attribute_ignored : Warning<
28572861
"%q0 redeclared without %1 attribute: previous %1 ignored">,
28582862
InGroup<MicrosoftInconsistentDllImport>;

clang/lib/CodeGen/CGBuiltin.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2048,11 +2048,10 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
20482048

20492049
Value *AlignmentValue = EmitScalarExpr(E->getArg(1));
20502050
ConstantInt *AlignmentCI = cast<ConstantInt>(AlignmentValue);
2051-
unsigned Alignment = (unsigned)AlignmentCI->getZExtValue();
20522051

20532052
EmitAlignmentAssumption(PtrValue, Ptr,
20542053
/*The expr loc is sufficient.*/ SourceLocation(),
2055-
Alignment, OffsetValue);
2054+
AlignmentCI, OffsetValue);
20562055
return RValue::get(PtrValue);
20572056
}
20582057
case Builtin::BI__assume:

clang/lib/CodeGen/CGCall.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4569,7 +4569,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
45694569
llvm::Value *Alignment = EmitScalarExpr(AA->getAlignment());
45704570
llvm::ConstantInt *AlignmentCI = cast<llvm::ConstantInt>(Alignment);
45714571
EmitAlignmentAssumption(Ret.getScalarVal(), RetTy, Loc, AA->getLocation(),
4572-
AlignmentCI->getZExtValue(), OffsetValue);
4572+
AlignmentCI, OffsetValue);
45734573
} else if (const auto *AA = TargetDecl->getAttr<AllocAlignAttr>()) {
45744574
llvm::Value *AlignmentVal = CallArgs[AA->getParamIndex().getLLVMIndex()]
45754575
.getRValue(*this)

clang/lib/CodeGen/CGExprScalar.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -294,8 +294,7 @@ class ScalarExprEmitter
294294

295295
Value *AlignmentValue = CGF.EmitScalarExpr(AVAttr->getAlignment());
296296
llvm::ConstantInt *AlignmentCI = cast<llvm::ConstantInt>(AlignmentValue);
297-
CGF.EmitAlignmentAssumption(V, E, AVAttr->getLocation(),
298-
AlignmentCI->getZExtValue());
297+
CGF.EmitAlignmentAssumption(V, E, AVAttr->getLocation(), AlignmentCI);
299298
}
300299

301300
/// EmitLoadOfLValue - Given an expression with complex type that represents a

clang/lib/CodeGen/CGStmtOpenMP.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1519,14 +1519,14 @@ static void emitAlignedClause(CodeGenFunction &CGF,
15191519
if (!CGF.HaveInsertPoint())
15201520
return;
15211521
for (const auto *Clause : D.getClausesOfKind<OMPAlignedClause>()) {
1522-
unsigned ClauseAlignment = 0;
1522+
size_t ClauseAlignment = 0;
15231523
if (const Expr *AlignmentExpr = Clause->getAlignment()) {
15241524
auto *AlignmentCI =
15251525
cast<llvm::ConstantInt>(CGF.EmitScalarExpr(AlignmentExpr));
1526-
ClauseAlignment = static_cast<unsigned>(AlignmentCI->getZExtValue());
1526+
ClauseAlignment = AlignmentCI->getZExtValue();
15271527
}
15281528
for (const Expr *E : Clause->varlists()) {
1529-
unsigned Alignment = ClauseAlignment;
1529+
size_t Alignment = ClauseAlignment;
15301530
if (Alignment == 0) {
15311531
// OpenMP [2.8.1, Description]
15321532
// If no optional parameter is specified, implementation-defined default
@@ -1542,7 +1542,8 @@ static void emitAlignedClause(CodeGenFunction &CGF,
15421542
if (Alignment != 0) {
15431543
llvm::Value *PtrValue = CGF.EmitScalarExpr(E);
15441544
CGF.EmitAlignmentAssumption(
1545-
PtrValue, E, /*No second loc needed*/ SourceLocation(), Alignment);
1545+
PtrValue, E, /*No second loc needed*/ SourceLocation(),
1546+
llvm::ConstantInt::get(CGF.SizeTy, Alignment));
15461547
}
15471548
}
15481549
}

clang/lib/CodeGen/CodeGenFunction.cpp

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2056,25 +2056,10 @@ void CodeGenFunction::EmitAlignmentAssumption(llvm::Value *PtrValue,
20562056
}
20572057
}
20582058

2059-
void CodeGenFunction::EmitAlignmentAssumption(llvm::Value *PtrValue,
2060-
QualType Ty, SourceLocation Loc,
2061-
SourceLocation AssumptionLoc,
2062-
unsigned Alignment,
2063-
llvm::Value *OffsetValue) {
2064-
llvm::Value *TheCheck;
2065-
llvm::Instruction *Assumption = Builder.CreateAlignmentAssumption(
2066-
CGM.getDataLayout(), PtrValue, Alignment, OffsetValue, &TheCheck);
2067-
if (SanOpts.has(SanitizerKind::Alignment)) {
2068-
llvm::Value *AlignmentVal = llvm::ConstantInt::get(IntPtrTy, Alignment);
2069-
EmitAlignmentAssumptionCheck(PtrValue, Ty, Loc, AssumptionLoc, AlignmentVal,
2070-
OffsetValue, TheCheck, Assumption);
2071-
}
2072-
}
2073-
20742059
void CodeGenFunction::EmitAlignmentAssumption(llvm::Value *PtrValue,
20752060
const Expr *E,
20762061
SourceLocation AssumptionLoc,
2077-
unsigned Alignment,
2062+
llvm::Value *Alignment,
20782063
llvm::Value *OffsetValue) {
20792064
if (auto *CE = dyn_cast<CastExpr>(E))
20802065
E = CE->getSubExprAsWritten();

clang/lib/CodeGen/CodeGenFunction.h

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2829,13 +2829,8 @@ class CodeGenFunction : public CodeGenTypeCache {
28292829
llvm::Value *Alignment,
28302830
llvm::Value *OffsetValue = nullptr);
28312831

2832-
void EmitAlignmentAssumption(llvm::Value *PtrValue, QualType Ty,
2833-
SourceLocation Loc, SourceLocation AssumptionLoc,
2834-
unsigned Alignment,
2835-
llvm::Value *OffsetValue = nullptr);
2836-
28372832
void EmitAlignmentAssumption(llvm::Value *PtrValue, const Expr *E,
2838-
SourceLocation AssumptionLoc, unsigned Alignment,
2833+
SourceLocation AssumptionLoc, llvm::Value *Alignment,
28392834
llvm::Value *OffsetValue = nullptr);
28402835

28412836
//===--------------------------------------------------------------------===//

clang/lib/Sema/SemaChecking.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6063,6 +6063,16 @@ bool Sema::SemaBuiltinAssumeAligned(CallExpr *TheCall) {
60636063
if (!Result.isPowerOf2())
60646064
return Diag(TheCall->getBeginLoc(), diag::err_alignment_not_power_of_two)
60656065
<< Arg->getSourceRange();
6066+
6067+
// FIXME: this should probably use llvm::Value::MaximumAlignment, however
6068+
// doing so results in a linking issue in GCC in a couple of assemblies.
6069+
// Alignment calculations can wrap around if it's greater than 2**28.
6070+
unsigned MaximumAlignment =
6071+
Context.getTargetInfo().getTriple().isOSBinFormatCOFF() ? 8192
6072+
: 268435456;
6073+
if (Result > MaximumAlignment)
6074+
Diag(TheCall->getBeginLoc(), diag::warn_assume_aligned_too_great)
6075+
<< Arg->getSourceRange() << MaximumAlignment;
60666076
}
60676077

60686078
if (NumArgs > 2) {

clang/test/Sema/builtin-assume-aligned.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,3 +58,7 @@ void *test_no_fn_proto() __attribute__((assume_aligned)); // expected-error {{'a
5858
void *test_no_fn_proto() __attribute__((assume_aligned())); // expected-error {{'assume_aligned' attribute takes at least 1 argument}}
5959
void *test_no_fn_proto() __attribute__((assume_aligned(32, 45, 37))); // expected-error {{'assume_aligned' attribute takes no more than 2 arguments}}
6060

61+
int pr43638(int *a) {
62+
a = __builtin_assume_aligned(a, 4294967296); // expected-warning {{requested alignment must be 268435456 bytes or smaller; assumption ignored}}
63+
return a[0];
64+
}

0 commit comments

Comments
 (0)