llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Akash Manna (akash-manna-sky)
<details>
<summary>Changes</summary>
Fixes #<!-- -->215454
A declaration with no declarator, like `__typeof__(e);`, is accepted with just
a warning, but it produces no Decl, so `ActOnDeclStmt` returns `StmtError()`.
On its own that's harmless: the statement is simply dropped. Since cad09404cc80
(#<!-- -->113760), though, `ParseCompoundStatementBody` returns `StmtError()`
for a `({ ... })` whose last statement is invalid, and that turned the whole
statement expression into an `ExprError` with no diagnostic behind it.
Everything downstream assumes an `ExprError` was already reported. So `({
foo(); __typeof__(e); });` compiled with a warning and the call to `foo()`
vanished from the IR, and in an `if` condition the parser's recovery produced a
`RecoveryExpr` with zero errors emitted, which let CodeGen run and hit the
`!isValueDependent()` assertion in `EvaluateAsInt`. Plain C, no templates
involved.
The parser now appends a null statement in place of the dropped one instead of
failing. The statement expression gets built with type `void`, which is what
GCC does for this input, and the earlier statement can't be picked up as the
value anymore, which is the thing #<!-- -->113760 was actually guarding
against. Using it as an `if` condition now gives the normal "statement requires
expression of scalar type" error, and side effects in the body are preserved.
The one visible change to existing behaviour is that a statement expression
whose last statement had a real error now has type `void`, so a use that needs
a value gets a follow-on conversion error rather than being silently discarded.
---
Full diff: https://github.com/llvm/llvm-project/pull/224682.diff
4 Files Affected:
- (modified) clang/docs/ReleaseNotes.md (+2)
- (modified) clang/lib/Parse/ParseStmt.cpp (+11-9)
- (added) clang/test/CodeGen/GH215454.c (+32)
- (modified) clang/test/SemaCXX/gh113468.cpp (+1-1)
``````````diff
diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md
index f4a34a37aff52..fe279a88cd942 100644
--- a/clang/docs/ReleaseNotes.md
+++ b/clang/docs/ReleaseNotes.md
@@ -547,6 +547,8 @@ features cannot lower the translation-unit ABI level;
- Fixed a crash when an `asm` label names the register for a global variable
of incomplete type. (#GH219746)
- Fixed an ICE hat occurred when using `__imag int/float` as lvalue in
assignment. (#GH119498)
- Fixed an assertion failure in `-Wsign-compare` when a negated or
complemented vector of unsigned integers was compared against a signed
constant. (#GH203575)
+- Fixed a crash in code generation and silently dropped side effects when the
last statement of a GNU statement
+ expression is a declaration that declares nothing, such as `({ f();
__typeof__(x); })`. (#GH215454)
#### Bug Fixes to Compiler Builtins
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index 14bea6a1a7948..15791273e71f7 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -1185,7 +1185,7 @@ StmtResult Parser::ParseCompoundStatementBody(bool
isStmtExpr) {
ParsedStmtContext::Compound |
(isStmtExpr ? ParsedStmtContext::InStmtExpr : ParsedStmtContext());
- bool LastIsError = false;
+ bool LastIsInvalid = false;
while (!tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) &&
Tok.isNot(tok::eof)) {
if (Tok.is(tok::annot_pragma_unused)) {
@@ -1242,14 +1242,16 @@ StmtResult Parser::ParseCompoundStatementBody(bool
isStmtExpr) {
if (R.isUsable())
Stmts.push_back(R.get());
- LastIsError = R.isInvalid();
- }
- // StmtExpr needs to do copy initialization for last statement.
- // If last statement is invalid, the last statement in `Stmts` will be
- // incorrect. Then the whole compound statement should also be marked as
- // invalid to prevent subsequent errors.
- if (isStmtExpr && LastIsError && !Stmts.empty())
- return StmtError();
+ LastIsInvalid = R.isInvalid();
+ }
+ // The last statement of a statement expression is its value and was already
+ // copy-initialized when parsed. If it was dropped, the statement now at the
+ // end must not become the value, so replace the dropped one with a null
+ // statement. Don't return StmtError here: an invalid statement does not
+ // imply an error was diagnosed (e.g. `__typeof__(x);` only warns), and an
+ // undiagnosed ExprError silently drops the statement expression.
+ if (isStmtExpr && LastIsInvalid)
+ Stmts.push_back(Actions.ActOnNullStmt(PrevTokLocation).get());
// Warn the user that using option `-ffp-eval-method=source` on a
// 32-bit target and feature `sse` disabled, or using
diff --git a/clang/test/CodeGen/GH215454.c b/clang/test/CodeGen/GH215454.c
new file mode 100644
index 0000000000000..aceba1d41474a
--- /dev/null
+++ b/clang/test/CodeGen/GH215454.c
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -std=gnu99 -verify -emit-llvm-only %s
+// RUN: %clang_cc1 -std=gnu99 -DCODEGEN -triple x86_64-unknown-linux-gnu
-emit-llvm -o - %s | FileCheck %s
+
+// A declaration that declares nothing as the last statement of a statement
+// expression made the whole statement expression invalid without an error,
+// which dropped the call below or crashed CodeGen on a RecoveryExpr.
+
+void foo(void);
+
+// CHECK-LABEL: define{{.*}} void @keeps_side_effects(
+// CHECK: call void @foo()
+void keeps_side_effects(int e) {
+ ({ foo(); __typeof__(e); }); // expected-warning {{declaration does not
declare anything}}
+}
+
+#ifndef CODEGEN
+void d2(int e) {
+ if (({ ; __typeof__(e); })) {} // expected-warning {{declaration does not
declare anything}} \
+ // expected-error {{statement requires
expression of scalar type ('void' invalid)}}
+}
+
+// Reproducer from the issue; the unclosed '({' makes recovery run to EOF.
+#define c(a, b)
\
+ {;__typeof__(b);}
+void d(int e) {if((c(, e);); // expected-warning {{'(' and '{' tokens
introducing statement expression appear in different macro expansion contexts}}
\
+ // expected-note {{'{' token is here}} \
+ // expected-warning {{declaration does not
declare anything}} \
+ // expected-error {{unexpected ';' before ')'}} \
+ // expected-note {{to match this '{'}}
+} // expected-error {{expected expression}} \
+ // expected-error@+2 {{expected '}'}}
+#endif
diff --git a/clang/test/SemaCXX/gh113468.cpp b/clang/test/SemaCXX/gh113468.cpp
index 94551986b0efa..f4252abd59c44 100644
--- a/clang/test/SemaCXX/gh113468.cpp
+++ b/clang/test/SemaCXX/gh113468.cpp
@@ -1,7 +1,7 @@
// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
constexpr int expr() {
- if (({
+ if (({ // expected-error {{value of type 'void' is not contextually
convertible to 'bool'}}
int f;
f = 0;
if (f)
``````````
</details>
https://github.com/llvm/llvm-project/pull/224682
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
