-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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
[clang]Avoid to check created local variable multiple time when evaluating #69106
Conversation
…ant evaluating When evaluting variable initialized from compound literal multiple times, it will be converted to evaluting the compound literal itself multiple times, which causes clang crash in assertions. And the further evaluting is not rely on this assertion. Fixes: #69065
@llvm/pr-subscribers-clang Author: Congcong Cai (HerrCai0907) ChangesWhen evaluating variable initialized from compound literal multiple times, it will be converted to evaluting the compound literal itself multiple times, which causes clang crash in assertions. Fixes: #69065 Full diff: https://github.com/llvm/llvm-project/pull/69106.diff 3 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index be7c8bf247f7af5..5578663a90f104a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -393,6 +393,9 @@ Bug Fixes in This Version
operator in C. No longer issuing a confusing diagnostic along the lines of
"incompatible operand types ('foo' and 'foo')" with extensions such as matrix
types. Fixes (`#69008 <https://github.com/llvm/llvm-project/issues/69008>`_)
+- Fix a crash when evaluating comparasion between the field from the same variable
+ which initialized from compound literals in C. Fixes
+ (`#69065 <https://github.com/llvm/llvm-project/issues/69065>`_)
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index e5539dedec02a4b..9948b516745a30b 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -1912,7 +1912,6 @@ APValue &CallStackFrame::createLocal(APValue::LValueBase Base, const void *Key,
assert(Base.getCallIndex() == Index && "lvalue for wrong frame");
unsigned Version = Base.getVersion();
APValue &Result = Temporaries[MapKeyTy(Key, Version)];
- assert(Result.isAbsent() && "local created multiple times");
// If we're creating a local immediately in the operand of a speculative
// evaluation, don't register a cleanup to be run outside the speculative
diff --git a/clang/test/AST/issue69065.c b/clang/test/AST/issue69065.c
new file mode 100644
index 000000000000000..11428932019418d
--- /dev/null
+++ b/clang/test/AST/issue69065.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+// expected-no-diagnostics
+
+struct A {
+ int i;
+};
+struct B {
+ struct A *a;
+};
+const struct B c = {&(struct A){1}};
+
+int main(void) {
+ if ((c.a->i != 1) || (c.a->i)) {
+ return 1;
+ }
+ return 0;
+}
|
I test example in branch llvmorg-16.0.6,ExprConstant.cpp also has this assertion but it will not result in crash.So I think you should not remove this assertion and try to find the reason |
Co-authored-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
Have you find the exact commit to break this case? @mzyKi |
@HerrCai0907 after this commit 610ec95 , clang will crash in test code. |
I think it would be better to disable assert instead of revert this change. |
I agree here, we shouldn't be double-creating variables like this, removing the assert doesn't seem right to me. |
When evaluating variable initialized from compound literal multiple times, it will be converted to evaluting the compound literal itself multiple times, which causes clang crash in assertions.
Further evaluating is not rely on this assertion.
Fixes: #69065