[PATCH] D150955: [clang][AST] Propagate the value-dependent bit for VAArgExpr.

[Haojian Wu via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd4e935240f02: [clang][AST] Propagate the value-dependent bit 
for VAArgExpr. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150955/new/

https://reviews.llvm.org/D150955

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/ComputeDependence.cpp
  clang/test/AST/ast-dump-recovery.c


Index: clang/test/AST/ast-dump-recovery.c
===
--- clang/test/AST/ast-dump-recovery.c
+++ clang/test/AST/ast-dump-recovery.c
@@ -109,3 +109,11 @@
 b,
   };
 }
+
+// Verify no crash
+void test5_GH62711() {
+  // CHECK:  VAArgExpr {{.*}} 'int' contains-errors
+  // CHECK-NEXT: | `-ImplicitCastExpr {{.*}} '' contains-errors
+  // CHECK-NEXT: |   `-RecoveryExpr {{.*}} '' contains-errors
+  if (__builtin_va_arg(undef, int) << 1);
+}
Index: clang/lib/AST/ComputeDependence.cpp
===
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -227,7 +227,7 @@
   auto D = toExprDependenceAsWritten(
E->getWrittenTypeInfo()->getType()->getDependence()) |
(E->getSubExpr()->getDependence() & ~ExprDependence::Type);
-  return D & ~ExprDependence::Value;
+  return D;
 }
 
 ExprDependence clang::computeDependence(NoInitExpr *E) {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -413,6 +413,9 @@
 - Fix a crash when an enum constant has a dependent-type recovery expression 
for
   C.
   (`#62446 `_).
+- Propagate the value-dependent bit for VAArgExpr. Fixes a crash where a
+  __builtin_va_arg call has invalid arguments.
+  (`#62711 `_).
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/AST/ast-dump-recovery.c
===
--- clang/test/AST/ast-dump-recovery.c
+++ clang/test/AST/ast-dump-recovery.c
@@ -109,3 +109,11 @@
 b,
   };
 }
+
+// Verify no crash
+void test5_GH62711() {
+  // CHECK:  VAArgExpr {{.*}} 'int' contains-errors
+  // CHECK-NEXT: | `-ImplicitCastExpr {{.*}} '' contains-errors
+  // CHECK-NEXT: |   `-RecoveryExpr {{.*}} '' contains-errors
+  if (__builtin_va_arg(undef, int) << 1);
+}
Index: clang/lib/AST/ComputeDependence.cpp
===
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -227,7 +227,7 @@
   auto D = toExprDependenceAsWritten(
E->getWrittenTypeInfo()->getType()->getDependence()) |
(E->getSubExpr()->getDependence() & ~ExprDependence::Type);
-  return D & ~ExprDependence::Value;
+  return D;
 }
 
 ExprDependence clang::computeDependence(NoInitExpr *E) {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -413,6 +413,9 @@
 - Fix a crash when an enum constant has a dependent-type recovery expression for
   C.
   (`#62446 `_).
+- Propagate the value-dependent bit for VAArgExpr. Fixes a crash where a
+  __builtin_va_arg call has invalid arguments.
+  (`#62711 `_).
 
 Bug Fixes to Compiler Builtins
 ^^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150955: [clang][AST] Propagate the value-dependent bit for VAArgExpr.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Thanks for the review.

In D150955#4356580 , @aaron.ballman 
wrote:

> LGTM assuming CI is okay with it -- the patch did not apply so precommit CI 
> never ran. I'm surprised no tests break as a result of this because it 
> changes both C and C++ behavior. The changes should have a release note, too.

This patch was based on my another fix (this probably explains why it failed to 
apply on pre-merge testing CI). I have test it with `ninja check-clang`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150955/new/

https://reviews.llvm.org/D150955

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150955: [clang][AST] Propagate the value-dependent bit for VAArgExpr.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 523875.
hokein added a comment.

add release note


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150955/new/

https://reviews.llvm.org/D150955

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/ComputeDependence.cpp
  clang/test/AST/ast-dump-recovery.c


Index: clang/test/AST/ast-dump-recovery.c
===
--- clang/test/AST/ast-dump-recovery.c
+++ clang/test/AST/ast-dump-recovery.c
@@ -109,3 +109,11 @@
 b,
   };
 }
+
+// Verify no crash
+void test5_GH62711() {
+  // CHECK:  VAArgExpr {{.*}} 'int' contains-errors
+  // CHECK-NEXT: | `-ImplicitCastExpr {{.*}} '' contains-errors
+  // CHECK-NEXT: |   `-RecoveryExpr {{.*}} '' contains-errors
+  if (__builtin_va_arg(undef, int) << 1);
+}
Index: clang/lib/AST/ComputeDependence.cpp
===
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -227,7 +227,7 @@
   auto D = toExprDependenceAsWritten(
E->getWrittenTypeInfo()->getType()->getDependence()) |
(E->getSubExpr()->getDependence() & ~ExprDependence::Type);
-  return D & ~ExprDependence::Value;
+  return D;
 }
 
 ExprDependence clang::computeDependence(NoInitExpr *E) {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -413,6 +413,9 @@
 - Fix a crash when an enum constant has a dependent-type recovery expression 
for
   C.
   (`#62446 `_).
+- Propagate the value-dependent bit for VAArgExpr. Fixes a crash where a
+  __builtin_va_arg call has invalid arguments.
+  (`#62711 `_).
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/AST/ast-dump-recovery.c
===
--- clang/test/AST/ast-dump-recovery.c
+++ clang/test/AST/ast-dump-recovery.c
@@ -109,3 +109,11 @@
 b,
   };
 }
+
+// Verify no crash
+void test5_GH62711() {
+  // CHECK:  VAArgExpr {{.*}} 'int' contains-errors
+  // CHECK-NEXT: | `-ImplicitCastExpr {{.*}} '' contains-errors
+  // CHECK-NEXT: |   `-RecoveryExpr {{.*}} '' contains-errors
+  if (__builtin_va_arg(undef, int) << 1);
+}
Index: clang/lib/AST/ComputeDependence.cpp
===
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -227,7 +227,7 @@
   auto D = toExprDependenceAsWritten(
E->getWrittenTypeInfo()->getType()->getDependence()) |
(E->getSubExpr()->getDependence() & ~ExprDependence::Type);
-  return D & ~ExprDependence::Value;
+  return D;
 }
 
 ExprDependence clang::computeDependence(NoInitExpr *E) {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -413,6 +413,9 @@
 - Fix a crash when an enum constant has a dependent-type recovery expression for
   C.
   (`#62446 `_).
+- Propagate the value-dependent bit for VAArgExpr. Fixes a crash where a
+  __builtin_va_arg call has invalid arguments.
+  (`#62711 `_).
 
 Bug Fixes to Compiler Builtins
 ^^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150955: [clang][AST] Propagate the value-dependent bit for VAArgExpr.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM assuming CI is okay with it -- the patch did not apply so precommit CI 
never ran. I'm surprised no tests break as a result of this because it changes 
both C and C++ behavior. The changes should have a release note, too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150955/new/

https://reviews.llvm.org/D150955

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150955: [clang][AST] Propagate the value-dependent bit for VAArgExpr.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: All.
hokein requested review of this revision.
Herald added a project: clang.

Fixes https://github.com/llvm/llvm-project/issues/62711

We never set the value-dependent bit for the VAArgExpr before this
patch, this was fine becase the dependent-type TypoExpr was always
resolved before checking the operands (see 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaExpr.cpp#L21173-L21180)

Now we have enabled the dependence by default for C, the typo expr is
not early resolved before checking rather than delayed (share the same
codepath with C++).

The fix is to propagate the value-dependent bit for VAArgExpr where it contains
a TypoExpr, so that the AST node can be handled properly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150955

Files:
  clang/lib/AST/ComputeDependence.cpp
  clang/test/AST/ast-dump-recovery.c


Index: clang/test/AST/ast-dump-recovery.c
===
--- clang/test/AST/ast-dump-recovery.c
+++ clang/test/AST/ast-dump-recovery.c
@@ -109,3 +109,11 @@
 b,
   };
 }
+
+// Verify no crash
+void test5_GH62711() {
+  // CHECK:  VAArgExpr {{.*}} 'int' contains-errors
+  // CHECK-NEXT: | `-ImplicitCastExpr {{.*}} '' contains-errors
+  // CHECK-NEXT: |   `-RecoveryExpr {{.*}} '' contains-errors
+  if (__builtin_va_arg(undef, int) << 1);
+}
Index: clang/lib/AST/ComputeDependence.cpp
===
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -227,7 +227,7 @@
   auto D = toExprDependenceAsWritten(
E->getWrittenTypeInfo()->getType()->getDependence()) |
(E->getSubExpr()->getDependence() & ~ExprDependence::Type);
-  return D & ~ExprDependence::Value;
+  return D;
 }
 
 ExprDependence clang::computeDependence(NoInitExpr *E) {


Index: clang/test/AST/ast-dump-recovery.c
===
--- clang/test/AST/ast-dump-recovery.c
+++ clang/test/AST/ast-dump-recovery.c
@@ -109,3 +109,11 @@
 b,
   };
 }
+
+// Verify no crash
+void test5_GH62711() {
+  // CHECK:  VAArgExpr {{.*}} 'int' contains-errors
+  // CHECK-NEXT: | `-ImplicitCastExpr {{.*}} '' contains-errors
+  // CHECK-NEXT: |   `-RecoveryExpr {{.*}} '' contains-errors
+  if (__builtin_va_arg(undef, int) << 1);
+}
Index: clang/lib/AST/ComputeDependence.cpp
===
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -227,7 +227,7 @@
   auto D = toExprDependenceAsWritten(
E->getWrittenTypeInfo()->getType()->getDependence()) |
(E->getSubExpr()->getDependence() & ~ExprDependence::Type);
-  return D & ~ExprDependence::Value;
+  return D;
 }
 
 ExprDependence clang::computeDependence(NoInitExpr *E) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits