[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support
Aaron Ballman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed May 25 09:42:12 PDT 2022
aaron.ballman added a comment.
In D123235#3537580 <https://reviews.llvm.org/D123235#3537580>, @koops wrote:
> 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.
================
Comment at: clang/include/clang/AST/ASTNodeTraverser.h:217-218
void Visit(const OMPClause *C) {
+ if (OMPFailClause::classof(C)) {
+ Visit(static_cast<const OMPFailClause *>(C));
+ return;
----------------
This is an anti-pattern, it should be:
```
if (const auto *OMPC = dyn_cast<OMPFailClause>(C))
Visit(OMPC);
```
================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:3641
+ if (CKind == OMPC_unknown) {
+ Diag(diag::err_omp_expected_clause) << ("atomic compare fail");
+ return Clause;
----------------
================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:3647-3649
+ if (MemoryOrderClause) {
+ MemOrderLoc = Tok.getLocation();
+ }
----------------
The coding style has us dropping braces here.
================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:3652
+ if (Tok.is(tok::r_paren)) {
+ FailClause->initFailClause(LParenLoc,MemoryOrderClause,MemOrderLoc);
+ ConsumeAnyToken();
----------------
Formatting (the whole patch should be run through clang-format: https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting)
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:12055-12057
+ if (C->getClauseKind() == OMPC_fail) {
+ NoOfFails++;
+ const OMPFailClause *FC = static_cast<const OMPFailClause *>(C);
----------------
This looks like another anti-pattern; why is this not:
```
if (const auto *FC = dyn_cast<OMPFailClause>(C)) {
...
```
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:12632-12633
+ SourceLocation ErrorLoc, NoteLoc;
+ NoteLoc = ErrorLoc = Body->getBeginLoc();
+ ErrorLoc = SubClauseLoc;
+ if (!Checker.checkSubClause(Clauses,&ErrorLoc)) {
----------------
Why are you assigning to `ErrorLoc` and then immediately overwriting it?
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:12634
+ ErrorLoc = SubClauseLoc;
+ if (!Checker.checkSubClause(Clauses,&ErrorLoc)) {
+ unsigned ErrorNo = Checker.getErrorDesc();
----------------
Formatting is off here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D123235/new/
https://reviews.llvm.org/D123235
More information about the llvm-commits
mailing list