Page MenuHomePhabricator

[OpenMP] atomic compare fail : Parser & AST support
Needs ReviewPublic

Authored by koops on Apr 6 2022, 10:22 AM.

Details

Summary

This is a support for " #pragma omp atomic compare fail ". It has Parser & AST support for now.

Diff Detail

Event Timeline

koops created this revision.Apr 6 2022, 10:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2022, 10:22 AM
Herald added a subscriber: arphaman. · View Herald Transcript
koops requested review of this revision.Apr 6 2022, 10:22 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript

Can I please have the detailed log for the build failures? I do not have the necessary setup to test those builds.

Can I please have the detailed log for the build failures? I do not have the necessary setup to test those builds.

You can find it https://buildkite.com/llvm-project/premerge-checks/builds/87307#7ab569e5-cc98-48c0-b88d-1cab7d95dd47.

koops updated this revision to Diff 423956.Apr 20 2022, 10:34 AM
  1. changes in flang/lib/Semantics/check-omp-structure.cpp to avoid build failure
  2. prevent "requires" directive test from failing.
aaron.ballman retitled this revision from atomic compare fail : Parser & AST support to [OpenMP] atomic compare fail : Parser & AST support.Apr 20 2022, 10:36 AM
aaron.ballman edited the summary of this revision. (Show Details)

FWIW, I changed the patch summary as a drive-by because I originally thought this was adding a global pragma named atomic compare fail and was very confused.

tianshilei1992 added inline comments.May 2 2022, 6:01 PM
clang/include/clang/AST/ASTNodeTraverser.h
217

clang-format plz.

228

Why would we want a dedicated function since it is only called once?

clang/include/clang/AST/OpenMPClause.h
2308–2317
2350

clang-format plz

2352

Please refer to https://llvm.org/docs/CodingStandards.html for variable naming style, etc.

koops updated this revision to Diff 429285.May 13 2022, 10:23 AM

Took care of the clang format.

koops added inline comments.May 13 2022, 10:33 AM
clang/include/clang/AST/ASTNodeTraverser.h
228

The code for this method cannot be put into any other method because it handles only OMPFailClause. All other Visit methods handle either the generalized OMPClause or other types of Clauses.

koops added a comment.EditedMay 14 2022, 4:47 AM

I have tried on x64 RH and x64 SuSe. I could not reproduce the failures seen on x64 debian. https://reviews.llvm.org/D118550 also has similar failures on x64 debian. There is a comment " I think the test failures are spurious (but not 100% sure)" So, are these failures pre-existing before the changes in the current support for "atomic compare fail: Parser & Support" were done?

I have tried on x64 RH and x64 SuSe. I could not reproduce the failures seen on x64 debian. https://reviews.llvm.org/D118550 also has similar failures on x64 debian. There is a comment " I think the test failures are spurious (but not 100% sure)" So, are these failures pre-existing before the changes in the current support for "atomic compare fail: Parser & Support" were done?

Those tests should be irrelevant.

clang/include/clang/AST/ASTNodeTraverser.h
228

I mean, it's only used by the function above, no?

231–232
clang/include/clang/Parse/Parser.h
436
clang/lib/AST/OpenMPClause.cpp
417–432

clang-format plz

clang/lib/Basic/OpenMPKinds.cpp
370

maybe something like CK? ck doesn't conform with LLVM standard.

clang/lib/Parse/ParseOpenMP.cpp
3636
3705–3708

ditto

clang/lib/Sema/SemaOpenMP.cpp
12060

ditto

koops updated this revision to Diff 430883.May 19 2022, 11:09 PM

Further changes in the code to confirm to the clang format.

koops added inline comments.May 19 2022, 11:12 PM
clang/include/clang/AST/ASTNodeTraverser.h
228

I agree with you but, I cannot understand any better way of coding it.

koops marked an inline comment as not done.May 19 2022, 11:13 PM

LGTM, but please fix the build error first.

clang/include/clang/AST/ASTNodeTraverser.h
228

Initially I thought to merge them, but this looks fine.

clang/include/clang/AST/OpenMPClause.h
2305

If the code is commented out, plz remove it.

koops updated this revision to Diff 431315.May 23 2022, 2:37 AM

Fixing a minor error : clang formatting of variable names to avoid build errors.

koops updated this revision to Diff 431385.May 23 2022, 8:24 AM

Clang formatting for variables in ParseOpenMP.cpp

koops updated this revision to Diff 431391.May 23 2022, 9:00 AM
tianshilei1992 accepted this revision.May 23 2022, 2:44 PM

LGTM

clang/lib/Parse/ParseOpenMP.cpp
3706–3708
This revision is now accepted and ready to land.May 23 2022, 2:44 PM
This revision was landed with ongoing or failed builds.May 24 2022, 9:57 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 24 2022, 9:57 PM
kda added a subscriber: kda.May 25 2022, 8:19 AM

This has broken sanitizer buildbot:
https://lab.llvm.org/buildbot/#/builders/5/builds/24074

Relevant log snippet:

FAIL: Clang :: OpenMP/atomic_messages.cpp (9696 of 66080)
******************** TEST 'Clang :: OpenMP/atomic_messages.cpp' FAILED ********************
Script:
--
: 'RUN: at line 1';   /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/bin/clang -cc1 -internal-isystem /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/lib/clang/15.0.0/include -nostdsysteminc -verify=expected,omp45 -fopenmp -fopenmp-version=45 -ferror-limit 150 /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/test/OpenMP/atomic_messages.cpp -Wuninitialized
: 'RUN: at line 2';   /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/bin/clang -cc1 -internal-isystem /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/lib/clang/15.0.0/include -nostdsysteminc -verify=expected,omp50 -fopenmp -ferror-limit 150 /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/test/OpenMP/atomic_messages.cpp -Wuninitialized
: 'RUN: at line 3';   /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/bin/clang -cc1 -internal-isystem /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/lib/clang/15.0.0/include -nostdsysteminc -DOMP51 -verify=expected,omp50,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 150 /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/test/OpenMP/atomic_messages.cpp -Wuninitialized
: 'RUN: at line 5';   /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/bin/clang -cc1 -internal-isystem /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/lib/clang/15.0.0/include -nostdsysteminc -verify=expected,omp45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 150 /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/test/OpenMP/atomic_messages.cpp -Wuninitialized
: 'RUN: at line 6';   /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/bin/clang -cc1 -internal-isystem /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/lib/clang/15.0.0/include -nostdsysteminc -verify=expected,omp50 -fopenmp-simd -ferror-limit 150 /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/test/OpenMP/atomic_messages.cpp -Wuninitialized
: 'RUN: at line 7';   /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/bin/clang -cc1 -internal-isystem /b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/lib/clang/15.0.0/include -nostdsysteminc -DOMP51 -verify=expected,omp50,omp51 -fopenmp-simd -fopenmp-version=51 -ferror-limit 150 /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/test/OpenMP/atomic_messages.cpp -Wuninitialized
--
Exit Code: 1
Command Output (stderr):
--
==75693==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x561f8672eb0e in (anonymous namespace)::OpenMPAtomicFailChecker::checkSubClause(llvm::ArrayRef<clang::OMPClause*>, clang::SourceLocation*) /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Sema/SemaOpenMP.cpp:12060:11
    #1 0x561f866ef4bc in clang::Sema::ActOnOpenMPAtomicDirective(llvm::ArrayRef<clang::OMPClause*>, clang::Stmt*, clang::SourceLocation, clang::SourceLocation) /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Sema/SemaOpenMP.cpp:12634:22
    #2 0x561f866c0775 in clang::Sema::ActOnOpenMPExecutableDirective(llvm::omp::Directive, clang::DeclarationNameInfo const&, llvm::omp::Directive, llvm::ArrayRef<clang::OMPClause*>, clang::Stmt*, clang::SourceLocation, clang::SourceLocation) /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Sema/SemaOpenMP.cpp:6168:11
    #3 0x561f856da291 in clang::Parser::ParseOpenMPDeclarativeOrExecutableDirective(clang::Parser::ParsedStmtContext, bool) /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Parse/ParseOpenMP.cpp:2922:25
    #4 0x561f8553b46f in clang::Parser::ParseStatementOrDeclarationAfterAttributes(llvm::SmallVector<clang::Stmt*, 32u>&, clang::Parser::ParsedStmtContext, clang::SourceLocation*, clang::ParsedAttributes&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Parse/ParseStmt.cpp:413:12
    #5 0x561f85537f27 in clang::Parser::ParseStatementOrDeclaration(llvm::SmallVector<clang::Stmt*, 32u>&, clang::Parser::ParsedStmtContext, clang::SourceLocation*) /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Parse/ParseStmt.cpp:113:20
    #6 0x561f85551be1 in clang::Parser::ParseCompoundStatementBody(bool) /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Parse/ParseStmt.cpp:1111:11
    #7 0x561f85554cfd in clang::Parser::ParseFunctionStatementBody(clang::Decl*, clang::Parser::ParseScope&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Parse/ParseStmt.cpp:2379:21
    #8 0x561f854cfa8e in clang::Parser::ParseFunctionDefinition(clang::ParsingDeclarator&, clang::Parser::ParsedTemplateInfo const&, clang::Parser::LateParsedAttrList*) /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Parse/Parser.cpp:1407:10
    #9 0x561f855bbc3e in clang::Parser::ParseDeclGroup(clang::ParsingDeclSpec&, clang::DeclaratorContext, clang::SourceLocation*, clang::Parser::ForRangeInit*) /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Parse/ParseDecl.cpp:2097:27
    #10 0x561f854cb272 in clang::Parser::ParseDeclOrFunctionDefInternal(clang::ParsedAttributes&, clang::ParsingDeclSpec&, clang::AccessSpecifier) /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Parse/Parser.cpp:1158:10
    #11 0x561f854ca0b6 in clang::Parser::ParseDeclarationOrFunctionDefinition(clang::ParsedAttributes&, clang::ParsingDeclSpec*, clang::AccessSpecifier) /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Parse/Parser.cpp:1172:12
    #12 0x561f854c754a in clang::Parser::ParseExternalDeclaration(clang::ParsedAttributes&, clang::ParsingDeclSpec*) /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Parse/Parser.cpp:998:12
    #13 0x561f854c0c9a in clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&, clang::Sema::ModuleImportState&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Parse/Parser.cpp:727:12
    #14 0x561f854ae252 in clang::ParseAST(clang::Sema&, bool, bool) /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Parse/ParseAST.cpp:162:20
    #15 0x561f8203140d in clang::FrontendAction::Execute() /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Frontend/FrontendAction.cpp:1032:8
    #16 0x561f81e8da47 in clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Frontend/CompilerInstance.cpp:1036:33
    #17 0x561f82278528 in clang::ExecuteCompilerInvocation(clang::CompilerInstance*) /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:266:25
    #18 0x561f79792507 in cc1_main(llvm::ArrayRef<char const*>, char const*, void*) /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/tools/driver/cc1_main.cpp:248:15
    #19 0x561f7978b484 in ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/tools/driver/driver.cpp:317:12
    #20 0x561f7978a434 in main /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/tools/driver/driver.cpp:388:12
    #21 0x7fada460f09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a) (BuildId: eb6a5dd378d22b1e695984462a799cd4c81cdc22)
    #22 0x561f796fa6d9 in _start (/b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/bin/clang-15+0x480d6d9)
SUMMARY: MemorySanitizer: use-of-uninitialized-value /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Sema/SemaOpenMP.cpp:12060:11 in (anonymous namespace)::OpenMPAtomicFailChecker::checkSubClause(llvm::ArrayRef<clang::OMPClause*>, clang::SourceLocation*)
Exiting
--
********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
********************
Failed Tests (1):
  Clang :: OpenMP/atomic_messages.cpp

Hello Kevin,

I am not sure why it is indicating an uninitialized variable at that point. The code at " llvm-project/clang/lib/Sema/SemaOpenMP.cpp:12060 " is as below:

for (const OMPClause *C : Clauses) {
  if (C->getClauseKind() == OMPC_fail) {
    NoOfFails++;
    const OMPFailClause *FC = static_cast<const OMPFailClause *>(C);
    const OMPClause *MemOrderC = FC->const_getMemoryOrderClause();
    /* Clauses contains OMPC_fail and the subclause */
    if (MemOrderC) {

Here " if (MemOrderC) { " is the 12060 line.

Regards,
--Sunil

I am not sure why it is indicating an uninitialized variable at that point. The code at " llvm-project/clang/lib/Sema/SemaOpenMP.cpp:12060 " is as below:

FWIW, it's outright crashing for me on Windows.

FAIL: Clang :: OpenMP/atomic_messages.cpp (255 of 15035)
******************** TEST 'Clang :: OpenMP/atomic_messages.cpp' FAILED ********************
Script:
--
: 'RUN: at line 1';   f:\source\llvm-project\llvm\out\build\x64-debug\bin\clang.exe -cc1 -internal-isystem f:\source\llvm-project\llvm\out\build\x64-debug\lib\clang\15.0.0\include -nostdsysteminc -verify=expected,omp45 -fopenmp -fopenmp-version=45 -ferror-limit 150 F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp -Wuninitialized
: 'RUN: at line 2';   f:\source\llvm-project\llvm\out\build\x64-debug\bin\clang.exe -cc1 -internal-isystem f:\source\llvm-project\llvm\out\build\x64-debug\lib\clang\15.0.0\include -nostdsysteminc -verify=expected,omp50 -fopenmp -ferror-limit 150 F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp -Wuninitialized
: 'RUN: at line 3';   f:\source\llvm-project\llvm\out\build\x64-debug\bin\clang.exe -cc1 -internal-isystem f:\source\llvm-project\llvm\out\build\x64-debug\lib\clang\15.0.0\include -nostdsysteminc -DOMP51 -verify=expected,omp50,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 150 F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp -Wuninitialized
: 'RUN: at line 5';   f:\source\llvm-project\llvm\out\build\x64-debug\bin\clang.exe -cc1 -internal-isystem f:\source\llvm-project\llvm\out\build\x64-debug\lib\clang\15.0.0\include -nostdsysteminc -verify=expected,omp45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 150 F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp -Wuninitialized
: 'RUN: at line 6';   f:\source\llvm-project\llvm\out\build\x64-debug\bin\clang.exe -cc1 -internal-isystem f:\source\llvm-project\llvm\out\build\x64-debug\lib\clang\15.0.0\include -nostdsysteminc -verify=expected,omp50 -fopenmp-simd -ferror-limit 150 F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp -Wuninitialized
: 'RUN: at line 7';   f:\source\llvm-project\llvm\out\build\x64-debug\bin\clang.exe -cc1 -internal-isystem f:\source\llvm-project\llvm\out\build\x64-debug\lib\clang\15.0.0\include -nostdsysteminc -DOMP51 -verify=expected,omp50,omp51 -fopenmp-simd -fopenmp-version=51 -ferror-limit 150 F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp -Wuninitialized
--
Exit Code: 3221225477

Command Output (stdout):
--
$ ":" "RUN: at line 1"
$ "f:\source\llvm-project\llvm\out\build\x64-debug\bin\clang.exe" "-cc1" "-internal-isystem" "f:\source\llvm-project\llvm\out\build\x64-debug\lib\clang\15.0.0\include" "-nostdsysteminc" "-verify=expected,omp45" "-fopenmp" "-fopenmp-version=45" "-ferror-limit" "150" "F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp" "-Wuninitialized"
$ ":" "RUN: at line 2"
$ "f:\source\llvm-project\llvm\out\build\x64-debug\bin\clang.exe" "-cc1" "-internal-isystem" "f:\source\llvm-project\llvm\out\build\x64-debug\lib\clang\15.0.0\include" "-nostdsysteminc" "-verify=expected,omp50" "-fopenmp" "-ferror-limit" "150" "F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp" "-Wuninitialized"
$ ":" "RUN: at line 3"
$ "f:\source\llvm-project\llvm\out\build\x64-debug\bin\clang.exe" "-cc1" "-internal-isystem" "f:\source\llvm-project\llvm\out\build\x64-debug\lib\clang\15.0.0\include" "-nostdsysteminc" "-DOMP51" "-verify=expected,omp50,omp51" "-fopenmp" "-fopenmp-version=51" "-ferror-limit" "150" "F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp" "-Wuninitialized"
# command stderr:
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.      Program arguments: f:\\source\\llvm-project\\llvm\\out\\build\\x64-debug\\bin\\clang.exe -cc1 -internal-isystem f:\\source\\llvm-project\\llvm\\out\\build\\x64-debug\\lib\\clang\\15.0.0\\include -nostdsysteminc -DOMP51 -verify=expected,omp50,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 150 F:\\source\\llvm-project\\clang\\test\\OpenMP\\atomic_messages.cpp -Wuninitialized
1.      F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp:968:1: at annotation token
2.      F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp:930:13: parsing function body 'mixed'
3.      F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp:930:13: in compound statement ('{}')
 #0 0x00007ff61281a32b clang::OMPClause::getClauseKind(void) const F:\source\llvm-project\clang\include\clang\AST\OpenMPClause.h:83:0
 #1 0x00007ff617bcf805 `anonymous namespace'::OpenMPAtomicFailChecker::checkSubClause F:\source\llvm-project\clang\lib\Sema\SemaOpenMP.cpp:12061:0
 #2 0x00007ff617b5d38d clang::Sema::ActOnOpenMPAtomicDirective(class llvm::ArrayRef<class clang::OMPClause *>, class clang::Stmt *, class clang::SourceLocation, class clang::SourceLocation) F:\source\llvm-project\clang\lib\Sema\SemaOpenMP.cpp:12634:0
 #3 0x00007ff617b50901 clang::Sema::ActOnOpenMPExecutableDirective(enum llvm::omp::Directive, struct clang::DeclarationNameInfo const &, enum llvm::omp::Directive, class llvm::ArrayRef<class clang::OMPClause *>, class clang::Stmt *, class clang::SourceLocation, class clang::SourceLocation) F:\source\llvm-project\clang\lib\Sema\SemaOpenMP.cpp:6168:0
 #4 0x00007ff616c2fba8 clang::Parser::ParseOpenMPDeclarativeOrExecutableDirective(enum clang::Parser::ParsedStmtContext, bool) F:\source\llvm-project\clang\lib\Parse\ParseOpenMP.cpp:2932:0
 #5 0x00007ff616be46b2 clang::Parser::ParseStatementOrDeclarationAfterAttributes(class llvm::SmallVector<class clang::Stmt *, 32> &, enum clang::Parser::ParsedStmtContext, class clang::SourceLocation *, class clang::ParsedAttributes &) F:\source\llvm-project\clang\lib\Parse\ParseStmt.cpp:413:0
 #6 0x00007ff616be3342 clang::Parser::ParseStatementOrDeclaration(class llvm::SmallVector<class clang::Stmt *, 32> &, enum clang::Parser::ParsedStmtContext, class clang::SourceLocation *) F:\source\llvm-project\clang\lib\Parse\ParseStmt.cpp:115:0
 #7 0x00007ff616be6cc9 clang::Parser::ParseCompoundStatementBody(bool) F:\source\llvm-project\clang\lib\Parse\ParseStmt.cpp:1111:0
 #8 0x00007ff616bedb92 clang::Parser::ParseFunctionStatementBody(class clang::Decl *, class clang::Parser::ParseScope &) F:\source\llvm-project\clang\lib\Parse\ParseStmt.cpp:2382:0
 #9 0x00007ff616aef1ec clang::Parser::ParseFunctionDefinition(class clang::ParsingDeclarator &, struct clang::Parser::ParsedTemplateInfo const &, class clang::Parser::LateParsedAttrList *) F:\source\llvm-project\clang\lib\Parse\Parser.cpp:1407:0
#10 0x00007ff616b3778c clang::Parser::ParseDeclGroup(class clang::ParsingDeclSpec &, enum clang::DeclaratorContext, class clang::SourceLocation *, struct clang::Parser::ForRangeInit *) F:\source\llvm-project\clang\lib\Parse\ParseDecl.cpp:2097:0
#11 0x00007ff616aede14 clang::Parser::ParseDeclOrFunctionDefInternal(class clang::ParsedAttributes &, class clang::ParsingDeclSpec &, enum clang::AccessSpecifier) F:\source\llvm-project\clang\lib\Parse\Parser.cpp:1158:0
#12 0x00007ff616aed6c2 clang::Parser::ParseDeclarationOrFunctionDefinition(class clang::ParsedAttributes &, class clang::ParsingDeclSpec *, enum clang::AccessSpecifier) F:\source\llvm-project\clang\lib\Parse\Parser.cpp:1172:0
#13 0x00007ff616aed0c8 clang::Parser::ParseExternalDeclaration(class clang::ParsedAttributes &, class clang::ParsingDeclSpec *) F:\source\llvm-project\clang\lib\Parse\Parser.cpp:998:0
#14 0x00007ff616ae7799 clang::Parser::ParseTopLevelDecl(class clang::OpaquePtr<class clang::DeclGroupRef> &, enum clang::Sema::ModuleImportState &) F:\source\llvm-project\clang\lib\Parse\Parser.cpp:727:0
#15 0x00007ff616ae3900 clang::ParseAST(class clang::Sema &, bool, bool) F:\source\llvm-project\clang\lib\Parse\ParseAST.cpp:162:0
#16 0x00007ff613805dd7 clang::ASTFrontendAction::ExecuteAction(void) F:\source\llvm-project\clang\lib\Frontend\FrontendAction.cpp:1141:0
#17 0x00007ff61380578e clang::FrontendAction::Execute(void) F:\source\llvm-project\clang\lib\Frontend\FrontendAction.cpp:1036:0
#18 0x00007ff61378a66a clang::CompilerInstance::ExecuteAction(class clang::FrontendAction &) F:\source\llvm-project\clang\lib\Frontend\CompilerInstance.cpp:1036:0
#19 0x00007ff613a002b8 clang::ExecuteCompilerInvocation(class clang::CompilerInstance *) F:\source\llvm-project\clang\lib\FrontendTool\ExecuteCompilerInvocation.cpp:266:0
#20 0x00007ff60f72e4c8 cc1_main(class llvm::ArrayRef<char const *>, char const *, void *) F:\source\llvm-project\clang\tools\driver\cc1_main.cpp:248:0
#21 0x00007ff60f719135 ExecuteCC1Tool F:\source\llvm-project\clang\tools\driver\driver.cpp:317:0
#22 0x00007ff60f7199ca main F:\source\llvm-project\clang\tools\driver\driver.cpp:388:0
#23 0x00007ff61997b2b9 invoke_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:79:0
#24 0x00007ff61997b15e __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288:0
#25 0x00007ff61997b01e __scrt_common_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:331:0
#26 0x00007ff61997b34e mainCRTStartup D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_main.cpp:17:0
#27 0x00007ffec5937034 (C:\WINDOWS\System32\KERNEL32.DLL+0x17034)
#28 0x00007ffec6502651 (C:\WINDOWS\SYSTEM32\ntdll.dll+0x52651)

error: command failed with exit status: 3221225477

--

I made some comments on things I saw when looking through the review but didn't spot a smoking gun. I think the issue is possibly the fact that this patch uses static_cast instead of the usual casting infrastructure, but I'm not positive.

Please revert the changes while investigating how to resolve the crashes though.

clang/include/clang/AST/ASTNodeTraverser.h
217–218

This is an anti-pattern, it should be:

if (const auto *OMPC = dyn_cast<OMPFailClause>(C))
  Visit(OMPC);
clang/lib/Parse/ParseOpenMP.cpp
3651
3657–3659

The coding style has us dropping braces here.

3662

Formatting (the whole patch should be run through clang-format: https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting)

clang/lib/Sema/SemaOpenMP.cpp
12064–12066

This looks like another anti-pattern; why is this not:

if (const auto *FC = dyn_cast<OMPFailClause>(C)) {
  ...
12650–12651

Why are you assigning to ErrorLoc and then immediately overwriting it?

12652

Formatting is off here.

Please revert the changes while investigating how to resolve the crashes though.

I just noticed that you needed someone to commit on your behalf for this, so I went ahead and did the revert myself in 69da3b6aead2e7a18a2578aad661d6d36b8d30cf.

Please revert the changes while investigating how to resolve the crashes though.

I just noticed that you needed someone to commit on your behalf for this, so I went ahead and did the revert myself in 69da3b6aead2e7a18a2578aad661d6d36b8d30cf.

There was a follow-up I also needed to revert in 9368bf9023eee0dc6fcfa007e157fe30e1540fcc.

koops added a comment.Wed, Jun 8, 8:51 PM
This comment was removed by koops.
koops reopened this revision.Thu, Jun 9, 9:00 AM
This revision is now accepted and ready to land.Thu, Jun 9, 9:00 AM
koops requested review of this revision.Thu, Jun 9, 9:00 AM
koops updated this revision to Diff 435831.Fri, Jun 10, 2:27 AM

Changes suggested by aaron.ballman to avoid failures on windows.

Precommit CI on Linux seems to be failing, but I can't quite tell whether the failures are related or not. I suspect they're unrelated, but they're with libomp and other OpenMP tests so it's not fully clear. Can you investigate those?

I tried your patch locally on Windows and it seems to still crash:

FAIL: Clang :: OpenMP/atomic_messages.cpp (1412 of 15074)
******************** TEST 'Clang :: OpenMP/atomic_messages.cpp' FAILED ********************
Script:
--
: 'RUN: at line 1';   f:\source\llvm-project\llvm\out\build\x64-debug\bin\clang.exe -cc1 -internal-isystem f:\source\llvm-project\llvm\out\build\x64-debug\lib\clang\15.0.0\include -nostdsysteminc -verify=expected,omp45 -fopenmp -fopenmp-version=45 -ferror-limit 150 F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp -Wuninitialized
: 'RUN: at line 2';   f:\source\llvm-project\llvm\out\build\x64-debug\bin\clang.exe -cc1 -internal-isystem f:\source\llvm-project\llvm\out\build\x64-debug\lib\clang\15.0.0\include -nostdsysteminc -verify=expected,omp50 -fopenmp -ferror-limit 150 F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp -Wuninitialized
: 'RUN: at line 3';   f:\source\llvm-project\llvm\out\build\x64-debug\bin\clang.exe -cc1 -internal-isystem f:\source\llvm-project\llvm\out\build\x64-debug\lib\clang\15.0.0\include -nostdsysteminc -DOMP51 -verify=expected,omp50,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 150 F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp -Wuninitialized
: 'RUN: at line 5';   f:\source\llvm-project\llvm\out\build\x64-debug\bin\clang.exe -cc1 -internal-isystem f:\source\llvm-project\llvm\out\build\x64-debug\lib\clang\15.0.0\include -nostdsysteminc -verify=expected,omp45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 150 F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp -Wuninitialized
: 'RUN: at line 6';   f:\source\llvm-project\llvm\out\build\x64-debug\bin\clang.exe -cc1 -internal-isystem f:\source\llvm-project\llvm\out\build\x64-debug\lib\clang\15.0.0\include -nostdsysteminc -verify=expected,omp50 -fopenmp-simd -ferror-limit 150 F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp -Wuninitialized
: 'RUN: at line 7';   f:\source\llvm-project\llvm\out\build\x64-debug\bin\clang.exe -cc1 -internal-isystem f:\source\llvm-project\llvm\out\build\x64-debug\lib\clang\15.0.0\include -nostdsysteminc -DOMP51 -verify=expected,omp50,omp51 -fopenmp-simd -fopenmp-version=51 -ferror-limit 150 F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp -Wuninitialized
--
Exit Code: 3221225477

Command Output (stdout):
--
$ ":" "RUN: at line 1"
$ "f:\source\llvm-project\llvm\out\build\x64-debug\bin\clang.exe" "-cc1" "-internal-isystem" "f:\source\llvm-project\llvm\out\build\x64-debug\lib\clang\15.0.0\include" "-nostdsysteminc" "-verify=expected,omp45" "-fopenmp" "-fopenmp-version=45" "-ferror-limit" "150" "F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp" "-Wuninitialized"
$ ":" "RUN: at line 2"
$ "f:\source\llvm-project\llvm\out\build\x64-debug\bin\clang.exe" "-cc1" "-internal-isystem" "f:\source\llvm-project\llvm\out\build\x64-debug\lib\clang\15.0.0\include" "-nostdsysteminc" "-verify=expected,omp50" "-fopenmp" "-ferror-limit" "150" "F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp" "-Wuninitialized"
$ ":" "RUN: at line 3"
$ "f:\source\llvm-project\llvm\out\build\x64-debug\bin\clang.exe" "-cc1" "-internal-isystem" "f:\source\llvm-project\llvm\out\build\x64-debug\lib\clang\15.0.0\include" "-nostdsysteminc" "-DOMP51" "-verify=expected,omp50,omp51" "-fopenmp" "-fopenmp-version=51" "-ferror-limit" "150" "F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp" "-Wuninitialized"
# command stderr:
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.      Program arguments: f:\\source\\llvm-project\\llvm\\out\\build\\x64-debug\\bin\\clang.exe -cc1 -internal-isystem f:\\source\\llvm-project\\llvm\\out\\build\\x64-debug\\lib\\clang\\15.0.0\\include -nostdsysteminc -DOMP51 -verify=expected,omp50,omp51 -fopenmp -fopenmp-version=51 -ferror-limit 150 F:\\source\\llvm-project\\clang\\test\\OpenMP\\atomic_messages.cpp -Wuninitialized
1.      F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp:968:1: at annotation token
2.      F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp:930:13: parsing function body 'mixed'
3.      F:\source\llvm-project\clang\test\OpenMP\atomic_messages.cpp:930:13: in compound statement ('{}')
 #0 0x00007ff6c208caab clang::OMPClause::getClauseKind(void) const F:\source\llvm-project\clang\include\clang\AST\OpenMPClause.h:83:0
 #1 0x00007ff6c7492153 `anonymous namespace'::OpenMPAtomicFailChecker::checkSubClause F:\source\llvm-project\clang\lib\Sema\SemaOpenMP.cpp:12069:0
 #2 0x00007ff6c741fc3d clang::Sema::ActOnOpenMPAtomicDirective(class llvm::ArrayRef<class clang::OMPClause *>, class clang::Stmt *, class clang::SourceLocation, class clang::SourceLocation) F:\source\llvm-project\clang\lib\Sema\SemaOpenMP.cpp:12652:0
 #3 0x00007ff6c74130f1 clang::Sema::ActOnOpenMPExecutableDirective(enum llvm::omp::Directive, struct clang::DeclarationNameInfo const &, enum llvm::omp::Directive, class llvm::ArrayRef<class clang::OMPClause *>, class clang::Stmt *, class clang::SourceLocation, class clang::SourceLocation) F:\source\llvm-project\clang\lib\Sema\SemaOpenMP.cpp:6172:0
 #4 0x00007ff6c64e6d78 clang::Parser::ParseOpenMPDeclarativeOrExecutableDirective(enum clang::Parser::ParsedStmtContext, bool) F:\source\llvm-project\clang\lib\Parse\ParseOpenMP.cpp:2932:0
 #5 0x00007ff6c649b8e2 clang::Parser::ParseStatementOrDeclarationAfterAttributes(class llvm::SmallVector<class clang::Stmt *, 32> &, enum clang::Parser::ParsedStmtContext, class clang::SourceLocation *, class clang::ParsedAttributes &) F:\source\llvm-project\clang\lib\Parse\ParseStmt.cpp:413:0
 #6 0x00007ff6c649a572 clang::Parser::ParseStatementOrDeclaration(class llvm::SmallVector<class clang::Stmt *, 32> &, enum clang::Parser::ParsedStmtContext, class clang::SourceLocation *) F:\source\llvm-project\clang\lib\Parse\ParseStmt.cpp:115:0
 #7 0x00007ff6c649def9 clang::Parser::ParseCompoundStatementBody(bool) F:\source\llvm-project\clang\lib\Parse\ParseStmt.cpp:1111:0
 #8 0x00007ff6c64a4dc2 clang::Parser::ParseFunctionStatementBody(class clang::Decl *, class clang::Parser::ParseScope &) F:\source\llvm-project\clang\lib\Parse\ParseStmt.cpp:2382:0
 #9 0x00007ff6c63a642c clang::Parser::ParseFunctionDefinition(class clang::ParsingDeclarator &, struct clang::Parser::ParsedTemplateInfo const &, class clang::Parser::LateParsedAttrList *) F:\source\llvm-project\clang\lib\Parse\Parser.cpp:1407:0
#10 0x00007ff6c63ee9ac clang::Parser::ParseDeclGroup(class clang::ParsingDeclSpec &, enum clang::DeclaratorContext, class clang::SourceLocation *, struct clang::Parser::ForRangeInit *) F:\source\llvm-project\clang\lib\Parse\ParseDecl.cpp:2098:0
#11 0x00007ff6c63a5054 clang::Parser::ParseDeclOrFunctionDefInternal(class clang::ParsedAttributes &, class clang::ParsingDeclSpec &, enum clang::AccessSpecifier) F:\source\llvm-project\clang\lib\Parse\Parser.cpp:1158:0
#12 0x00007ff6c63a4902 clang::Parser::ParseDeclarationOrFunctionDefinition(class clang::ParsedAttributes &, class clang::ParsingDeclSpec *, enum clang::AccessSpecifier) F:\source\llvm-project\clang\lib\Parse\Parser.cpp:1172:0
#13 0x00007ff6c63a4308 clang::Parser::ParseExternalDeclaration(class clang::ParsedAttributes &, class clang::ParsingDeclSpec *) F:\source\llvm-project\clang\lib\Parse\Parser.cpp:998:0
#14 0x00007ff6c639e9d9 clang::Parser::ParseTopLevelDecl(class clang::OpaquePtr<class clang::DeclGroupRef> &, enum clang::Sema::ModuleImportState &) F:\source\llvm-project\clang\lib\Parse\Parser.cpp:727:0
#15 0x00007ff6c639ab40 clang::ParseAST(class clang::Sema &, bool, bool) F:\source\llvm-project\clang\lib\Parse\ParseAST.cpp:162:0
#16 0x00007ff6c308ae97 clang::ASTFrontendAction::ExecuteAction(void) F:\source\llvm-project\clang\lib\Frontend\FrontendAction.cpp:1137:0
#17 0x00007ff6c308a84e clang::FrontendAction::Execute(void) F:\source\llvm-project\clang\lib\Frontend\FrontendAction.cpp:1032:0
#18 0x00007ff6c300f59a clang::CompilerInstance::ExecuteAction(class clang::FrontendAction &) F:\source\llvm-project\clang\lib\Frontend\CompilerInstance.cpp:1033:0
#19 0x00007ff6c32856d8 clang::ExecuteCompilerInvocation(class clang::CompilerInstance *) F:\source\llvm-project\clang\lib\FrontendTool\ExecuteCompilerInvocation.cpp:266:0
#20 0x00007ff6bef3da48 cc1_main(class llvm::ArrayRef<char const *>, char const *, void *) F:\source\llvm-project\clang\tools\driver\cc1_main.cpp:248:0
#21 0x00007ff6bef26e65 ExecuteCC1Tool F:\source\llvm-project\clang\tools\driver\driver.cpp:317:0
#22 0x00007ff6bef276fa clang_main(int, char **) F:\source\llvm-project\clang\tools\driver\driver.cpp:388:0
#23 0x00007ff6bef6859c main F:\source\llvm-project\llvm\out\build\x64-Debug\tools\clang\tools\driver\clang-driver.cpp:11:0
#24 0x00007ff6c92600b9 invoke_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:79:0
#25 0x00007ff6c925ff5e __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288:0
#26 0x00007ff6c925fe1e __scrt_common_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:331:0
#27 0x00007ff6c926014e mainCRTStartup D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_main.cpp:17:0
#28 0x00007ff826487034 (C:\WINDOWS\System32\KERNEL32.DLL+0x17034)
#29 0x00007ff828362651 (C:\WINDOWS\SYSTEM32\ntdll.dll+0x52651)

error: command failed with exit status: 3221225477

--

********************