r372422 - Ensure AtomicExpr goes through SEMA checking after TreeTransform
Erich Keane via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 20 12:17:31 PDT 2019
Author: erichkeane
Date: Fri Sep 20 12:17:31 2019
New Revision: 372422
URL: http://llvm.org/viewvc/llvm-project?rev=372422&view=rev
Log:
Ensure AtomicExpr goes through SEMA checking after TreeTransform
RebuildAtomicExpr was skipping doing semantic analysis which broke in
the cases where the expressions were not dependent. This resulted in the
ImplicitCastExpr from an array to a pointer being lost, causing a crash
in IR CodeGen.
Differential Revision: https://reviews.llvm.org/D67854
Added:
cfe/trunk/test/AST/atomic-expr.cpp (with props)
Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/lib/Sema/TreeTransform.h
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=372422&r1=372421&r2=372422&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Fri Sep 20 12:17:31 2019
@@ -4637,6 +4637,9 @@ public:
MultiExprArg ArgExprs, SourceLocation RParenLoc,
Expr *ExecConfig = nullptr,
bool IsExecConfig = false);
+ ExprResult BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
+ SourceLocation RParenLoc, MultiExprArg Args,
+ AtomicExpr::AtomicOp Op);
ExprResult
BuildResolvedCallExpr(Expr *Fn, NamedDecl *NDecl, SourceLocation LParenLoc,
ArrayRef<Expr *> Arg, SourceLocation RParenLoc,
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=372422&r1=372421&r2=372422&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Sep 20 12:17:31 2019
@@ -4475,7 +4475,15 @@ ExprResult Sema::SemaAtomicOpsOverloaded
AtomicExpr::AtomicOp Op) {
CallExpr *TheCall = cast<CallExpr>(TheCallResult.get());
DeclRefExpr *DRE =cast<DeclRefExpr>(TheCall->getCallee()->IgnoreParenCasts());
-
+ MultiExprArg Args{TheCall->getArgs(), TheCall->getNumArgs()};
+ return BuildAtomicExpr({TheCall->getBeginLoc(), TheCall->getEndLoc()},
+ DRE->getSourceRange(), TheCall->getRParenLoc(), Args,
+ Op);
+}
+
+ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
+ SourceLocation RParenLoc, MultiExprArg Args,
+ AtomicExpr::AtomicOp Op) {
// All the non-OpenCL operations take one of the following forms.
// The OpenCL operations take the __c11 forms with one extra argument for
// synchronization scope.
@@ -4622,21 +4630,21 @@ ExprResult Sema::SemaAtomicOpsOverloaded
if (IsOpenCL && Op != AtomicExpr::AO__opencl_atomic_init)
++AdjustedNumArgs;
// Check we have the right number of arguments.
- if (TheCall->getNumArgs() < AdjustedNumArgs) {
- Diag(TheCall->getEndLoc(), diag::err_typecheck_call_too_few_args)
- << 0 << AdjustedNumArgs << TheCall->getNumArgs()
- << TheCall->getCallee()->getSourceRange();
+ if (Args.size() < AdjustedNumArgs) {
+ Diag(CallRange.getEnd(), diag::err_typecheck_call_too_few_args)
+ << 0 << AdjustedNumArgs << static_cast<unsigned>(Args.size())
+ << ExprRange;
return ExprError();
- } else if (TheCall->getNumArgs() > AdjustedNumArgs) {
- Diag(TheCall->getArg(AdjustedNumArgs)->getBeginLoc(),
+ } else if (Args.size() > AdjustedNumArgs) {
+ Diag(Args[AdjustedNumArgs]->getBeginLoc(),
diag::err_typecheck_call_too_many_args)
- << 0 << AdjustedNumArgs << TheCall->getNumArgs()
- << TheCall->getCallee()->getSourceRange();
+ << 0 << AdjustedNumArgs << static_cast<unsigned>(Args.size())
+ << ExprRange;
return ExprError();
}
// Inspect the first argument of the atomic operation.
- Expr *Ptr = TheCall->getArg(0);
+ Expr *Ptr = Args[0];
ExprResult ConvertedPtr = DefaultFunctionArrayLvalueConversion(Ptr);
if (ConvertedPtr.isInvalid())
return ExprError();
@@ -4644,7 +4652,7 @@ ExprResult Sema::SemaAtomicOpsOverloaded
Ptr = ConvertedPtr.get();
const PointerType *pointerType = Ptr->getType()->getAs<PointerType>();
if (!pointerType) {
- Diag(DRE->getBeginLoc(), diag::err_atomic_builtin_must_be_pointer)
+ Diag(ExprRange.getBegin(), diag::err_atomic_builtin_must_be_pointer)
<< Ptr->getType() << Ptr->getSourceRange();
return ExprError();
}
@@ -4654,13 +4662,13 @@ ExprResult Sema::SemaAtomicOpsOverloaded
QualType ValType = AtomTy; // 'C'
if (IsC11) {
if (!AtomTy->isAtomicType()) {
- Diag(DRE->getBeginLoc(), diag::err_atomic_op_needs_atomic)
+ Diag(ExprRange.getBegin(), diag::err_atomic_op_needs_atomic)
<< Ptr->getType() << Ptr->getSourceRange();
return ExprError();
}
if ((Form != Load && Form != LoadCopy && AtomTy.isConstQualified()) ||
AtomTy.getAddressSpace() == LangAS::opencl_constant) {
- Diag(DRE->getBeginLoc(), diag::err_atomic_op_needs_non_const_atomic)
+ Diag(ExprRange.getBegin(), diag::err_atomic_op_needs_non_const_atomic)
<< (AtomTy.isConstQualified() ? 0 : 1) << Ptr->getType()
<< Ptr->getSourceRange();
return ExprError();
@@ -4668,7 +4676,7 @@ ExprResult Sema::SemaAtomicOpsOverloaded
ValType = AtomTy->getAs<AtomicType>()->getValueType();
} else if (Form != Load && Form != LoadCopy) {
if (ValType.isConstQualified()) {
- Diag(DRE->getBeginLoc(), diag::err_atomic_op_needs_non_const_pointer)
+ Diag(ExprRange.getBegin(), diag::err_atomic_op_needs_non_const_pointer)
<< Ptr->getType() << Ptr->getSourceRange();
return ExprError();
}
@@ -4679,7 +4687,7 @@ ExprResult Sema::SemaAtomicOpsOverloaded
// gcc does not enforce these rules for GNU atomics, but we do so for sanity.
if (IsAddSub && !ValType->isIntegerType()
&& !ValType->isPointerType()) {
- Diag(DRE->getBeginLoc(), diag::err_atomic_op_needs_atomic_int_or_ptr)
+ Diag(ExprRange.getBegin(), diag::err_atomic_op_needs_atomic_int_or_ptr)
<< IsC11 << Ptr->getType() << Ptr->getSourceRange();
return ExprError();
}
@@ -4687,12 +4695,12 @@ ExprResult Sema::SemaAtomicOpsOverloaded
const BuiltinType *BT = ValType->getAs<BuiltinType>();
if (!BT || (BT->getKind() != BuiltinType::Int &&
BT->getKind() != BuiltinType::UInt)) {
- Diag(DRE->getBeginLoc(), diag::err_atomic_op_needs_int32_or_ptr);
+ Diag(ExprRange.getBegin(), diag::err_atomic_op_needs_int32_or_ptr);
return ExprError();
}
}
if (!IsAddSub && !IsMinMax && !ValType->isIntegerType()) {
- Diag(DRE->getBeginLoc(), diag::err_atomic_op_bitwise_needs_atomic_int)
+ Diag(ExprRange.getBegin(), diag::err_atomic_op_bitwise_needs_atomic_int)
<< IsC11 << Ptr->getType() << Ptr->getSourceRange();
return ExprError();
}
@@ -4704,7 +4712,7 @@ ExprResult Sema::SemaAtomicOpsOverloaded
} else if (IsN && !ValType->isIntegerType() && !ValType->isPointerType()) {
// For __atomic_*_n operations, the value type must be a scalar integral or
// pointer type which is 1, 2, 4, 8 or 16 bytes in length.
- Diag(DRE->getBeginLoc(), diag::err_atomic_op_needs_atomic_int_or_ptr)
+ Diag(ExprRange.getBegin(), diag::err_atomic_op_needs_atomic_int_or_ptr)
<< IsC11 << Ptr->getType() << Ptr->getSourceRange();
return ExprError();
}
@@ -4713,7 +4721,7 @@ ExprResult Sema::SemaAtomicOpsOverloaded
!AtomTy->isScalarType()) {
// For GNU atomics, require a trivially-copyable type. This is not part of
// the GNU atomics specification, but we enforce it for sanity.
- Diag(DRE->getBeginLoc(), diag::err_atomic_op_needs_trivial_copy)
+ Diag(ExprRange.getBegin(), diag::err_atomic_op_needs_trivial_copy)
<< Ptr->getType() << Ptr->getSourceRange();
return ExprError();
}
@@ -4729,7 +4737,7 @@ ExprResult Sema::SemaAtomicOpsOverloaded
case Qualifiers::OCL_Autoreleasing:
// FIXME: Can this happen? By this point, ValType should be known
// to be trivially copyable.
- Diag(DRE->getBeginLoc(), diag::err_arc_atomic_ownership)
+ Diag(ExprRange.getBegin(), diag::err_arc_atomic_ownership)
<< ValType << Ptr->getSourceRange();
return ExprError();
}
@@ -4761,14 +4769,14 @@ ExprResult Sema::SemaAtomicOpsOverloaded
// - weak flag (always converted to bool)
// - memory order (always converted to int)
// - scope (always converted to int)
- for (unsigned i = 0; i != TheCall->getNumArgs(); ++i) {
+ for (unsigned i = 0; i != Args.size(); ++i) {
QualType Ty;
if (i < NumVals[Form] + 1) {
switch (i) {
case 0:
// The first argument is always a pointer. It has a fixed type.
// It is always dereferenced, a nullptr is undefined.
- CheckNonNullArgument(*this, TheCall->getArg(i), DRE->getBeginLoc());
+ CheckNonNullArgument(*this, Args[i], ExprRange.getBegin());
// Nothing else to do: we already know all we want about this pointer.
continue;
case 1:
@@ -4782,14 +4790,14 @@ ExprResult Sema::SemaAtomicOpsOverloaded
else if (Form == Copy || Form == Xchg) {
if (IsPassedByAddress)
// The value pointer is always dereferenced, a nullptr is undefined.
- CheckNonNullArgument(*this, TheCall->getArg(i), DRE->getBeginLoc());
+ CheckNonNullArgument(*this, Args[i], ExprRange.getBegin());
Ty = ByValType;
} else if (Form == Arithmetic)
Ty = Context.getPointerDiffType();
else {
- Expr *ValArg = TheCall->getArg(i);
+ Expr *ValArg = Args[i];
// The value pointer is always dereferenced, a nullptr is undefined.
- CheckNonNullArgument(*this, ValArg, DRE->getBeginLoc());
+ CheckNonNullArgument(*this, ValArg, ExprRange.getBegin());
LangAS AS = LangAS::Default;
// Keep address space of non-atomic pointer type.
if (const PointerType *PtrTy =
@@ -4804,7 +4812,7 @@ ExprResult Sema::SemaAtomicOpsOverloaded
// The third argument to compare_exchange / GNU exchange is the desired
// value, either by-value (for the C11 and *_n variant) or as a pointer.
if (IsPassedByAddress)
- CheckNonNullArgument(*this, TheCall->getArg(i), DRE->getBeginLoc());
+ CheckNonNullArgument(*this, Args[i], ExprRange.getBegin());
Ty = ByValType;
break;
case 3:
@@ -4819,11 +4827,11 @@ ExprResult Sema::SemaAtomicOpsOverloaded
InitializedEntity Entity =
InitializedEntity::InitializeParameter(Context, Ty, false);
- ExprResult Arg = TheCall->getArg(i);
+ ExprResult Arg = Args[i];
Arg = PerformCopyInitialization(Entity, SourceLocation(), Arg);
if (Arg.isInvalid())
return true;
- TheCall->setArg(i, Arg.get());
+ Args[i] = Arg.get();
}
// Permute the arguments into a 'consistent' order.
@@ -4832,36 +4840,36 @@ ExprResult Sema::SemaAtomicOpsOverloaded
switch (Form) {
case Init:
// Note, AtomicExpr::getVal1() has a special case for this atomic.
- SubExprs.push_back(TheCall->getArg(1)); // Val1
+ SubExprs.push_back(Args[1]); // Val1
break;
case Load:
- SubExprs.push_back(TheCall->getArg(1)); // Order
+ SubExprs.push_back(Args[1]); // Order
break;
case LoadCopy:
case Copy:
case Arithmetic:
case Xchg:
- SubExprs.push_back(TheCall->getArg(2)); // Order
- SubExprs.push_back(TheCall->getArg(1)); // Val1
+ SubExprs.push_back(Args[2]); // Order
+ SubExprs.push_back(Args[1]); // Val1
break;
case GNUXchg:
// Note, AtomicExpr::getVal2() has a special case for this atomic.
- SubExprs.push_back(TheCall->getArg(3)); // Order
- SubExprs.push_back(TheCall->getArg(1)); // Val1
- SubExprs.push_back(TheCall->getArg(2)); // Val2
+ SubExprs.push_back(Args[3]); // Order
+ SubExprs.push_back(Args[1]); // Val1
+ SubExprs.push_back(Args[2]); // Val2
break;
case C11CmpXchg:
- SubExprs.push_back(TheCall->getArg(3)); // Order
- SubExprs.push_back(TheCall->getArg(1)); // Val1
- SubExprs.push_back(TheCall->getArg(4)); // OrderFail
- SubExprs.push_back(TheCall->getArg(2)); // Val2
+ SubExprs.push_back(Args[3]); // Order
+ SubExprs.push_back(Args[1]); // Val1
+ SubExprs.push_back(Args[4]); // OrderFail
+ SubExprs.push_back(Args[2]); // Val2
break;
case GNUCmpXchg:
- SubExprs.push_back(TheCall->getArg(4)); // Order
- SubExprs.push_back(TheCall->getArg(1)); // Val1
- SubExprs.push_back(TheCall->getArg(5)); // OrderFail
- SubExprs.push_back(TheCall->getArg(2)); // Val2
- SubExprs.push_back(TheCall->getArg(3)); // Weak
+ SubExprs.push_back(Args[4]); // Order
+ SubExprs.push_back(Args[1]); // Val1
+ SubExprs.push_back(Args[5]); // OrderFail
+ SubExprs.push_back(Args[2]); // Val2
+ SubExprs.push_back(Args[3]); // Weak
break;
}
@@ -4875,7 +4883,7 @@ ExprResult Sema::SemaAtomicOpsOverloaded
}
if (auto ScopeModel = AtomicExpr::getScopeModel(Op)) {
- auto *Scope = TheCall->getArg(TheCall->getNumArgs() - 1);
+ auto *Scope = Args[Args.size() - 1];
llvm::APSInt Result(32);
if (Scope->isIntegerConstantExpr(Result, Context) &&
!ScopeModel->isValid(Result.getZExtValue())) {
@@ -4885,9 +4893,8 @@ ExprResult Sema::SemaAtomicOpsOverloaded
SubExprs.push_back(Scope);
}
- AtomicExpr *AE =
- new (Context) AtomicExpr(TheCall->getCallee()->getBeginLoc(), SubExprs,
- ResultType, Op, TheCall->getRParenLoc());
+ AtomicExpr *AE = new (Context)
+ AtomicExpr(ExprRange.getBegin(), SubExprs, ResultType, Op, RParenLoc);
if ((Op == AtomicExpr::AO__c11_atomic_load ||
Op == AtomicExpr::AO__c11_atomic_store ||
Modified: cfe/trunk/lib/Sema/TreeTransform.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=372422&r1=372421&r2=372422&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/TreeTransform.h (original)
+++ cfe/trunk/lib/Sema/TreeTransform.h Fri Sep 20 12:17:31 2019
@@ -3310,14 +3310,12 @@ public:
/// Subclasses may override this routine to provide different behavior.
ExprResult RebuildAtomicExpr(SourceLocation BuiltinLoc,
MultiExprArg SubExprs,
- QualType RetTy,
AtomicExpr::AtomicOp Op,
SourceLocation RParenLoc) {
- // Just create the expression; there is not any interesting semantic
- // analysis here because we can't actually build an AtomicExpr until
- // we are sure it is semantically sound.
- return new (SemaRef.Context) AtomicExpr(BuiltinLoc, SubExprs, RetTy, Op,
- RParenLoc);
+ // Use this for all of the locations, since we don't know the difference
+ // between the call and the expr at this point.
+ SourceRange Range{BuiltinLoc, RParenLoc};
+ return getSema().BuildAtomicExpr(Range, Range, RParenLoc, SubExprs, Op);
}
private:
@@ -12673,7 +12671,6 @@ TreeTransform<Derived>::TransformAsTypeE
template<typename Derived>
ExprResult
TreeTransform<Derived>::TransformAtomicExpr(AtomicExpr *E) {
- QualType RetTy = getDerived().TransformType(E->getType());
bool ArgumentChanged = false;
SmallVector<Expr*, 8> SubExprs;
SubExprs.reserve(E->getNumSubExprs());
@@ -12686,7 +12683,7 @@ TreeTransform<Derived>::TransformAtomicE
return E;
return getDerived().RebuildAtomicExpr(E->getBuiltinLoc(), SubExprs,
- RetTy, E->getOp(), E->getRParenLoc());
+ E->getOp(), E->getRParenLoc());
}
//===----------------------------------------------------------------------===//
Added: cfe/trunk/test/AST/atomic-expr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/atomic-expr.cpp?rev=372422&view=auto
==============================================================================
--- cfe/trunk/test/AST/atomic-expr.cpp (added)
+++ cfe/trunk/test/AST/atomic-expr.cpp Fri Sep 20 12:17:31 2019
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -ast-dump %s | FileCheck %s
+
+template<int N = 0>
+void pr43370() {
+ int arr[2];
+ __atomic_store_n(arr, 0, 0);
+}
+void useage(){
+ pr43370();
+}
+
+// CHECK:FunctionTemplateDecl 0x{{[0-9a-f]+}} <{{[^:]+}}:3:1, line:7:1> line:4:6 pr43370
+// CHECK: AtomicExpr
+// CHECK-NEXT: ImplicitCastExpr
+// CHECK-SAME: <ArrayToPointerDecay>
+// CHECK:FunctionDecl 0x{{[0-9a-f]+}} <line:4:1, line:7:1> line:4:6 used pr43370
+// CHECK: AtomicExpr
+// CHECK-NEXT: ImplicitCastExpr
+// CHECK-SAME: <ArrayToPointerDecay>
Propchange: cfe/trunk/test/AST/atomic-expr.cpp
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: cfe/trunk/test/AST/atomic-expr.cpp
------------------------------------------------------------------------------
svn:keywords = Author Date Id Rev URL
Propchange: cfe/trunk/test/AST/atomic-expr.cpp
------------------------------------------------------------------------------
svn:mime-type = text/plain
More information about the cfe-commits
mailing list