From 68d20a23c9c4f86658cf081912f5c95155179c88 Mon Sep 17 00:00:00 2001 From: Cyndy Ishida Date: Fri, 6 Jun 2025 13:45:16 -0700 Subject: [PATCH 1/2] [clang][dep-scan] Resolve lexer crash from a permutation of invalid tokens (#142452) Sometimes, when a user writes invalid code, the minimization used for scanning can create a stream of tokens that is invalid at lex time. This patch protects against the case where there are valid (non-c++20) import directives discovered in the middle of an invalid `import` declaration. Mostly authored by: @akyrtzi resolves: rdar://152335844 (cherry picked from commit 897b0301d2e2ff28d3976fe95b64be5a85815908) --- clang/lib/Lex/DependencyDirectivesScanner.cpp | 9 +++++++++ .../Lex/DependencyDirectivesScannerTest.cpp | 17 ++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/clang/lib/Lex/DependencyDirectivesScanner.cpp b/clang/lib/Lex/DependencyDirectivesScanner.cpp index 31a4c0f52b465..f9c580bd93690 100644 --- a/clang/lib/Lex/DependencyDirectivesScanner.cpp +++ b/clang/lib/Lex/DependencyDirectivesScanner.cpp @@ -496,7 +496,15 @@ bool Scanner::lexModuleDirectiveBody(DirectiveKind Kind, const char *&First, const char *const End) { const char *DirectiveLoc = Input.data() + CurDirToks.front().Offset; for (;;) { + // Keep a copy of the First char incase it needs to be reset. + const char *Previous = First; const dependency_directives_scan::Token &Tok = lexToken(First, End); + if ((Tok.is(tok::hash) || Tok.is(tok::at)) && + (Tok.Flags & clang::Token::StartOfLine)) { + CurDirToks.pop_back(); + First = Previous; + return false; + } if (Tok.is(tok::eof)) return reportError( DirectiveLoc, @@ -846,6 +854,7 @@ bool Scanner::lexPPLine(const char *&First, const char *const End) { if (*First == '@') return lexAt(First, End); + // Handle module directives for C++20 modules. if (*First == 'i' || *First == 'e' || *First == 'm') return lexModule(First, End); diff --git a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp index 513e184be09ec..3496a3e31ba15 100644 --- a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp +++ b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp @@ -661,13 +661,28 @@ TEST(MinimizeSourceToDependencyDirectivesTest, EmptyIncludesAndImports) { Out)); } -TEST(MinimizeSourceToDependencyDirectivesTest, AtImportFailures) { +TEST(MinimizeSourceToDependencyDirectivesTest, ImportFailures) { SmallVector Out; ASSERT_TRUE(minimizeSourceToDependencyDirectives("@import A\n", Out)); ASSERT_FALSE( minimizeSourceToDependencyDirectives("@import MACRO(A);\n", Out)); ASSERT_FALSE(minimizeSourceToDependencyDirectives("@import \" \";\n", Out)); + + ASSERT_FALSE(minimizeSourceToDependencyDirectives("import \n" + "@import Foo;", + Out)); + EXPECT_STREQ("@import Foo;\n", Out.data()); + + ASSERT_FALSE( + minimizeSourceToDependencyDirectives("import \n" + "#import \n" + "@;\n" + "#pragma clang module import Foo", + Out)); + EXPECT_STREQ("#import \n" + "#pragma clang module import Foo\n", + Out.data()); } TEST(MinimizeSourceToDependencyDirectivesTest, RawStringLiteral) { From 9f9f7d1769179dfd06f6c2133a64182eb674b6f3 Mon Sep 17 00:00:00 2001 From: Cyndy Ishida Date: Mon, 2 Jun 2025 15:59:16 -0700 Subject: [PATCH 2/2] [clang] Rename diag notes that assumed precompiled dependencies are pch's, NFCI (#142161) (cherry picked from commit 883130e33325282cfd31b68f5db52891442c20b7) --- .../clang/Basic/DiagnosticSerializationKinds.td | 5 +++-- clang/lib/Serialization/ASTReader.cpp | 14 +++++++------- clang/test/Modules/module-file-modified.c | 1 + clang/test/Modules/validate-file-content.m | 2 +- clang/test/PCH/modified-module-dependency.m | 2 +- clang/test/PCH/validate-file-content.m | 2 +- 6 files changed, 14 insertions(+), 12 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td b/clang/include/clang/Basic/DiagnosticSerializationKinds.td index eb27de5921d6a..fcc32b7ef0eac 100644 --- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td +++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td @@ -24,8 +24,9 @@ def err_fe_ast_file_modified : Error< DefaultFatal; def err_fe_pch_file_overridden : Error< "file '%0' from the precompiled header has been overridden">; -def note_pch_required_by : Note<"'%0' required by '%1'">; -def note_pch_rebuild_required : Note<"please rebuild precompiled header '%0'">; +def note_ast_file_required_by : Note<"'%0' required by '%1'">; +def note_ast_file_rebuild_required + : Note<"please rebuild precompiled file '%0'">; def note_module_cache_path : Note< "after modifying system headers, please delete the module cache at '%0'">; diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 8a04e70f7debf..591622f45394e 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -2680,25 +2680,25 @@ InputFile ASTReader::getInputFile(ModuleFile &F, unsigned ID, bool Complain) { while (!ImportStack.back()->ImportedBy.empty()) ImportStack.push_back(ImportStack.back()->ImportedBy[0]); - // The top-level PCH is stale. - StringRef TopLevelPCHName(ImportStack.back()->FileName); + // The top-level AST file is stale. + StringRef TopLevelASTFileName(ImportStack.back()->FileName); Diag(diag::err_fe_ast_file_modified) << *Filename << moduleKindForDiagnostic(ImportStack.back()->Kind) - << TopLevelPCHName << FileChange.Kind + << TopLevelASTFileName << FileChange.Kind << (FileChange.Old && FileChange.New) << llvm::itostr(FileChange.Old.value_or(0)) << llvm::itostr(FileChange.New.value_or(0)); // Print the import stack. if (ImportStack.size() > 1) { - Diag(diag::note_pch_required_by) + Diag(diag::note_ast_file_required_by) << *Filename << ImportStack[0]->FileName; for (unsigned I = 1; I < ImportStack.size(); ++I) - Diag(diag::note_pch_required_by) - << ImportStack[I-1]->FileName << ImportStack[I]->FileName; + Diag(diag::note_ast_file_required_by) + << ImportStack[I - 1]->FileName << ImportStack[I]->FileName; } - Diag(diag::note_pch_rebuild_required) << TopLevelPCHName; + Diag(diag::note_ast_file_rebuild_required) << TopLevelASTFileName; } IsOutOfDate = true; diff --git a/clang/test/Modules/module-file-modified.c b/clang/test/Modules/module-file-modified.c index 85018b521e288..57160f34a46cf 100644 --- a/clang/test/Modules/module-file-modified.c +++ b/clang/test/Modules/module-file-modified.c @@ -8,4 +8,5 @@ #include "a.h" int foo = 0; // redefinition of 'foo' // CHECK: fatal error: file {{.*}} has been modified since the module file {{.*}} was built +// CHECK: note: please rebuild precompiled file // REQUIRES: shell diff --git a/clang/test/Modules/validate-file-content.m b/clang/test/Modules/validate-file-content.m index 327a68a9b641e..9977aa4665f04 100644 --- a/clang/test/Modules/validate-file-content.m +++ b/clang/test/Modules/validate-file-content.m @@ -29,5 +29,5 @@ // CHECK: file '[[M_H:.*[/\\]m\.h]]' has been modified since the precompiled header '[[A_PCH:.*/a\.pch]]' was built: content changed // CHECK: '[[M_H]]' required by '[[M_PCM:.*[/\\]m.*\.pcm]]' // CHECK: '[[M_PCM]]' required by '[[A_PCH]]' -// CHECK: please rebuild precompiled header '[[A_PCH]]' +// CHECK: please rebuild precompiled file '[[A_PCH]]' // expected-no-diagnostics diff --git a/clang/test/PCH/modified-module-dependency.m b/clang/test/PCH/modified-module-dependency.m index 44c14b3a0231c..a4710dea51169 100644 --- a/clang/test/PCH/modified-module-dependency.m +++ b/clang/test/PCH/modified-module-dependency.m @@ -17,4 +17,4 @@ // CHECK: file '[[TEST_H:.*[/\\]test\.h]]' has been modified since the precompiled header '[[PREFIX_PCH:.*/prefix\.pch]]' was built // CHECK: '[[TEST_H]]' required by '[[TEST_PCM:.*[/\\]test\.pcm]]' // CHECK: '[[TEST_PCM]]' required by '[[PREFIX_PCH]]' -// CHECK: please rebuild precompiled header '[[PREFIX_PCH]]' +// CHECK: please rebuild precompiled file '[[PREFIX_PCH]]' diff --git a/clang/test/PCH/validate-file-content.m b/clang/test/PCH/validate-file-content.m index aba4944a28e91..b98979341b76a 100644 --- a/clang/test/PCH/validate-file-content.m +++ b/clang/test/PCH/validate-file-content.m @@ -25,5 +25,5 @@ // RUN: FileCheck %s < %t/stderr // // CHECK: file '[[M_H:.*[/\\]m\.h]]' has been modified since the precompiled header '[[A_PCH:.*/a\.pch]]' was built: content changed -// CHECK: please rebuild precompiled header '[[A_PCH]]' +// CHECK: please rebuild precompiled file '[[A_PCH]]' // expected-no-diagnostics