r290181 - [OPENMP] Fix for PR31416: Clang crashes on OMPCapturedExpr during source

Vedant Kumar via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 20 10:48:37 PST 2016


Thank you for looking into this :)!

I noticed that your test case doesn't hit the code coverage path, however.

Perhaps it would be better to remove the PROF-INSTR-PATH check and instead
introduce something like `test/CoverageMapping/openmp.cpp`:

  // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -std=c++11 -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -main-file-name openmp.cpp %s | FileCheck %s -check-prefix=COV
  // Construct an OMPCapturedExpr in some function 'f'...
  // COV: f

Initially the test can just check that we emit a coverage mapping for the
function 'f'. Later, I can flesh out the test with precise checks.

best,
vedant

> On Dec 20, 2016, at 8:51 AM, Alexey Bataev via cfe-commits <cfe-commits at lists.llvm.org> wrote:
> 
> Author: abataev
> Date: Tue Dec 20 10:51:02 2016
> New Revision: 290181
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=290181&view=rev
> Log:
> [OPENMP] Fix for PR31416: Clang crashes on OMPCapturedExpr during source
> based coverage compilation
> 
> Added source location info to captured expression declaration + fixed
> source location info for loop based directives.
> 
> Modified:
>    cfe/trunk/include/clang/AST/DeclOpenMP.h
>    cfe/trunk/lib/AST/DeclOpenMP.cpp
>    cfe/trunk/lib/Sema/SemaOpenMP.cpp
>    cfe/trunk/test/OpenMP/for_codegen.cpp
> 
> Modified: cfe/trunk/include/clang/AST/DeclOpenMP.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclOpenMP.h?rev=290181&r1=290180&r2=290181&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/DeclOpenMP.h (original)
> +++ cfe/trunk/include/clang/AST/DeclOpenMP.h Tue Dec 20 10:51:02 2016
> @@ -173,18 +173,21 @@ class OMPCapturedExprDecl final : public
>   void anchor() override;
> 
>   OMPCapturedExprDecl(ASTContext &C, DeclContext *DC, IdentifierInfo *Id,
> -                      QualType Type)
> -      : VarDecl(OMPCapturedExpr, C, DC, SourceLocation(), SourceLocation(), Id,
> -                Type, nullptr, SC_None) {
> +                      QualType Type, SourceLocation StartLoc)
> +      : VarDecl(OMPCapturedExpr, C, DC, StartLoc, SourceLocation(), Id, Type,
> +                nullptr, SC_None) {
>     setImplicit();
>   }
> 
> public:
>   static OMPCapturedExprDecl *Create(ASTContext &C, DeclContext *DC,
> -                                     IdentifierInfo *Id, QualType T);
> +                                     IdentifierInfo *Id, QualType T,
> +                                     SourceLocation StartLoc);
> 
>   static OMPCapturedExprDecl *CreateDeserialized(ASTContext &C, unsigned ID);
> 
> +  SourceRange getSourceRange() const override LLVM_READONLY;
> +
>   // Implement isa/cast/dyncast/etc.
>   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
>   static bool classofKind(Kind K) { return K == OMPCapturedExpr; }
> 
> Modified: cfe/trunk/lib/AST/DeclOpenMP.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclOpenMP.cpp?rev=290181&r1=290180&r2=290181&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/DeclOpenMP.cpp (original)
> +++ cfe/trunk/lib/AST/DeclOpenMP.cpp Tue Dec 20 10:51:02 2016
> @@ -90,13 +90,18 @@ OMPDeclareReductionDecl::getPrevDeclInSc
> void OMPCapturedExprDecl::anchor() {}
> 
> OMPCapturedExprDecl *OMPCapturedExprDecl::Create(ASTContext &C, DeclContext *DC,
> -                                                 IdentifierInfo *Id,
> -                                                 QualType T) {
> -  return new (C, DC) OMPCapturedExprDecl(C, DC, Id, T);
> +                                                 IdentifierInfo *Id, QualType T,
> +                                                 SourceLocation StartLoc) {
> +  return new (C, DC) OMPCapturedExprDecl(C, DC, Id, T, StartLoc);
> }
> 
> OMPCapturedExprDecl *OMPCapturedExprDecl::CreateDeserialized(ASTContext &C,
>                                                              unsigned ID) {
> -  return new (C, ID) OMPCapturedExprDecl(C, nullptr, nullptr, QualType());
> +  return new (C, ID)
> +      OMPCapturedExprDecl(C, nullptr, nullptr, QualType(), SourceLocation());
> }
> 
> +SourceRange OMPCapturedExprDecl::getSourceRange() const {
> +  assert(hasInit());
> +  return SourceRange(getInit()->getLocStart(), getInit()->getLocEnd());
> +}
> 
> Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=290181&r1=290180&r2=290181&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Tue Dec 20 10:51:02 2016
> @@ -1753,7 +1753,8 @@ static OMPCapturedExprDecl *buildCapture
>     }
>     WithInit = true;
>   }
> -  auto *CED = OMPCapturedExprDecl::Create(C, S.CurContext, Id, Ty);
> +  auto *CED = OMPCapturedExprDecl::Create(C, S.CurContext, Id, Ty,
> +                                          CaptureExpr->getLocStart());
>   if (!WithInit)
>     CED->addAttr(OMPCaptureNoInitAttr::CreateImplicit(C, SourceRange()));
>   S.CurContext->addHiddenDecl(CED);
> @@ -3862,14 +3863,16 @@ CheckOpenMPLoop(OpenMPDirectiveKind DKin
>   Scope *CurScope = DSA.getCurScope();
>   for (unsigned Cnt = 1; Cnt < NestedLoopCount; ++Cnt) {
>     if (PreCond.isUsable()) {
> -      PreCond = SemaRef.BuildBinOp(CurScope, SourceLocation(), BO_LAnd,
> -                                   PreCond.get(), IterSpaces[Cnt].PreCond);
> +      PreCond =
> +          SemaRef.BuildBinOp(CurScope, PreCond.get()->getExprLoc(), BO_LAnd,
> +                             PreCond.get(), IterSpaces[Cnt].PreCond);
>     }
>     auto N = IterSpaces[Cnt].NumIterations;
> +    SourceLocation Loc = N->getExprLoc();
>     AllCountsNeedLessThan32Bits &= C.getTypeSize(N->getType()) < 32;
>     if (LastIteration32.isUsable())
>       LastIteration32 = SemaRef.BuildBinOp(
> -          CurScope, SourceLocation(), BO_Mul, LastIteration32.get(),
> +          CurScope, Loc, BO_Mul, LastIteration32.get(),
>           SemaRef
>               .PerformImplicitConversion(N->IgnoreImpCasts(), N->getType(),
>                                          Sema::AA_Converting,
> @@ -3877,7 +3880,7 @@ CheckOpenMPLoop(OpenMPDirectiveKind DKin
>               .get());
>     if (LastIteration64.isUsable())
>       LastIteration64 = SemaRef.BuildBinOp(
> -          CurScope, SourceLocation(), BO_Mul, LastIteration64.get(),
> +          CurScope, Loc, BO_Mul, LastIteration64.get(),
>           SemaRef
>               .PerformImplicitConversion(N->IgnoreImpCasts(), N->getType(),
>                                          Sema::AA_Converting,
> @@ -3912,7 +3915,8 @@ CheckOpenMPLoop(OpenMPDirectiveKind DKin
>   ExprResult NumIterations = LastIteration;
>   {
>     LastIteration = SemaRef.BuildBinOp(
> -        CurScope, SourceLocation(), BO_Sub, LastIteration.get(),
> +        CurScope, LastIteration.get()->getExprLoc(), BO_Sub,
> +        LastIteration.get(),
>         SemaRef.ActOnIntegerConstant(SourceLocation(), 1).get());
>     if (!LastIteration.isUsable())
>       return 0;
> @@ -3931,7 +3935,7 @@ CheckOpenMPLoop(OpenMPDirectiveKind DKin
> 
>     // Prepare SaveRef + 1.
>     NumIterations = SemaRef.BuildBinOp(
> -        CurScope, SourceLocation(), BO_Add, SaveRef.get(),
> +        CurScope, SaveRef.get()->getExprLoc(), BO_Add, SaveRef.get(),
>         SemaRef.ActOnIntegerConstant(SourceLocation(), 1).get());
>     if (!NumIterations.isUsable())
>       return 0;
> 
> Modified: cfe/trunk/test/OpenMP/for_codegen.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/for_codegen.cpp?rev=290181&r1=290180&r2=290181&view=diff
> ==============================================================================
> --- cfe/trunk/test/OpenMP/for_codegen.cpp (original)
> +++ cfe/trunk/test/OpenMP/for_codegen.cpp Tue Dec 20 10:51:02 2016
> @@ -2,10 +2,13 @@
> // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -emit-pch -o %t %s
> // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
> // RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -fexceptions -fcxx-exceptions -debug-info-kind=line-tables-only -x c++ -emit-llvm %s -o - | FileCheck %s --check-prefix=TERM_DEBUG
> +// RUN: %clang_cc1 -main-file-name for_codegen.cpp %s -o - -emit-llvm -fprofile-instrument=clang -fprofile-instrument-path=for_codegen-test.profraw | FileCheck %s --check-prefix=PROF-INSTR-PATH
> //
> // expected-no-diagnostics
> #ifndef HEADER
> #define HEADER
> +// PROF-INSTR-PATH: constant [25 x i8] c"for_codegen-test.profraw\00"
> +
> // CHECK: [[IDENT_T_TY:%.+]] = type { i32, i32, i32, i32, i8* }
> // CHECK-DAG: [[IMPLICIT_BARRIER_LOC:@.+]] = private unnamed_addr constant %{{.+}} { i32 0, i32 66, i32 0, i32 0, i8*
> // CHECK-DAG: [[I:@.+]] = global i8 1,
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits



More information about the cfe-commits mailing list