This revision was automatically updated to reflect the committed changes. Closed by commit rL372422: Ensure AtomicExpr goes through SEMA checking after TreeTransform (authored by erichkeane, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits.
Changed prior to commit: https://reviews.llvm.org/D67854?vs=221080&id=221088#toc Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67854/new/ https://reviews.llvm.org/D67854 Files: cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/Sema/SemaChecking.cpp cfe/trunk/lib/Sema/TreeTransform.h cfe/trunk/test/AST/atomic-expr.cpp
Index: cfe/trunk/include/clang/Sema/Sema.h =================================================================== --- cfe/trunk/include/clang/Sema/Sema.h +++ cfe/trunk/include/clang/Sema/Sema.h @@ -4637,6 +4637,9 @@ 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, Index: cfe/trunk/test/AST/atomic-expr.cpp =================================================================== --- cfe/trunk/test/AST/atomic-expr.cpp +++ cfe/trunk/test/AST/atomic-expr.cpp @@ -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> Index: cfe/trunk/lib/Sema/SemaChecking.cpp =================================================================== --- cfe/trunk/lib/Sema/SemaChecking.cpp +++ cfe/trunk/lib/Sema/SemaChecking.cpp @@ -4475,7 +4475,15 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ // 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 @@ 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 @@ } 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 @@ !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 @@ 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 @@ // - 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 @@ 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 @@ // 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 @@ 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 @@ 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 @@ } 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 @@ 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 || Index: cfe/trunk/lib/Sema/TreeTransform.h =================================================================== --- cfe/trunk/lib/Sema/TreeTransform.h +++ cfe/trunk/lib/Sema/TreeTransform.h @@ -3310,14 +3310,12 @@ /// 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 @@ 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 @@ return E; return getDerived().RebuildAtomicExpr(E->getBuiltinLoc(), SubExprs, - RetTy, E->getOp(), E->getRParenLoc()); + E->getOp(), E->getRParenLoc()); } //===----------------------------------------------------------------------===//
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits