-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[Clang] prevent crash on unterminated __has_embed #163107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-clang Author: Oleksandr T. (a-tarasyuk) ChangesFixes #162953 This PR addresses the issue of Clang crashing on unterminated Full diff: https://github.com/llvm/llvm-project/pull/163107.diff 4 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 65b086caf3652..7e65ba74dc6fd 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -405,6 +405,7 @@ Bug Fixes in This Version
a function without arguments caused us to try to access a non-existent argument.
(#GH159080)
- Fixed a failed assertion with empty filename arguments in ``__has_embed``. (#GH159898)
+- Fixed a crash triggered by unterminated ``__has_embed``. (#GH162953)
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 360593d0f33df..67906ca0f461a 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -3648,14 +3648,14 @@ Preprocessor::LexEmbedParameters(Token &CurTok, bool ForHasEmbed) {
std::pair<tok::TokenKind, SourceLocation> Matches) {
Diag(CurTok, diag::err_expected) << Expected;
Diag(Matches.second, diag::note_matching) << Matches.first;
- if (CurTok.isNot(tok::eod))
+ if (CurTok.isNot(EndTokenKind))
DiscardUntilEndOfDirective(CurTok);
};
auto ExpectOrDiagAndSkipToEOD = [&](tok::TokenKind Kind) {
if (CurTok.isNot(Kind)) {
Diag(CurTok, diag::err_expected) << Kind;
- if (CurTok.isNot(tok::eod))
+ if (CurTok.isNot(EndTokenKind))
DiscardUntilEndOfDirective(CurTok);
return false;
}
@@ -3746,7 +3746,7 @@ Preprocessor::LexEmbedParameters(Token &CurTok, bool ForHasEmbed) {
if (Result.isNegative()) {
Diag(CurTok, diag::err_requires_positive_value)
<< toString(Result, 10) << /*positive*/ 0;
- if (CurTok.isNot(tok::eod))
+ if (CurTok.isNot(EndTokenKind))
DiscardUntilEndOfDirective(CurTok);
return std::nullopt;
}
@@ -3889,7 +3889,7 @@ Preprocessor::LexEmbedParameters(Token &CurTok, bool ForHasEmbed) {
}
if (!ForHasEmbed) {
Diag(ParamStartLoc, diag::err_pp_unknown_parameter) << 1 << Parameter;
- if (CurTok.isNot(tok::eod))
+ if (CurTok.isNot(EndTokenKind))
DiscardUntilEndOfDirective(CurTok);
return std::nullopt;
}
diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp
index dec1956ea0f9a..dd80ae586a1f6 100644
--- a/clang/lib/Lex/PPMacroExpansion.cpp
+++ b/clang/lib/Lex/PPMacroExpansion.cpp
@@ -1262,16 +1262,11 @@ EmbedResult Preprocessor::EvaluateHasEmbed(Token &Tok, IdentifierInfo *II) {
std::optional<LexEmbedParametersResult> Params =
this->LexEmbedParameters(Tok, /*ForHasEmbed=*/true);
- assert((Params || Tok.is(tok::eod)) &&
- "expected success or to be at the end of the directive");
if (!Params)
return EmbedResult::Invalid;
- if (Params->UnrecognizedParams > 0)
- return EmbedResult::NotFound;
-
- if (!Tok.is(tok::r_paren)) {
+ if (Tok.isNot(tok::r_paren)) {
Diag(this->getLocForEndOfToken(FilenameLoc), diag::err_pp_expected_after)
<< II << tok::r_paren;
Diag(LParenLoc, diag::note_matching) << tok::l_paren;
@@ -1280,6 +1275,9 @@ EmbedResult Preprocessor::EvaluateHasEmbed(Token &Tok, IdentifierInfo *II) {
return EmbedResult::Invalid;
}
+ if (Params->UnrecognizedParams > 0)
+ return EmbedResult::NotFound;
+
SmallString<128> FilenameBuffer;
StringRef Filename = this->getSpelling(FilenameTok, FilenameBuffer);
if (Filename.empty())
diff --git a/clang/test/Preprocessor/embed___has_embed_parsing_errors.c b/clang/test/Preprocessor/embed___has_embed_parsing_errors.c
index 9c512a4882e2d..a389fa257b164 100644
--- a/clang/test/Preprocessor/embed___has_embed_parsing_errors.c
+++ b/clang/test/Preprocessor/embed___has_embed_parsing_errors.c
@@ -250,3 +250,10 @@
#if __has_embed("") // expected-error {{empty filename}}
#endif
+
+// expected-error@+4 {{missing ')' after '__has_embed'}} \
+ expected-error@+4 {{expected value in expression}} \
+ expected-error@+4 {{unterminated conditional directive}} \
+ expected-note@+4 {{to match this '('}}
+#if __has_embed (__FILE__ limit(1) foo
+int a = __has_embed (__FILE__);
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A test NIT, otherwise LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Fixes #162953
This PR addresses the issue of Clang crashing on unterminated
__has_embed
by guarding allDiscardUntilEndOfDirective
paths with the contextual end token and diagnosing the missing)
before reporting unknown parameters.