Skip to content

[clang][Sema] Fix crash introduced in b2cd9db589335d5885c04df83003a623cf2f05ff #66954

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

Merged
merged 2 commits into from
Sep 21, 2023

Conversation

hazohelet
Copy link
Member

Old iterator is invalidated upon SmallVector elements additions. Stores index instead of iterator to avoid this.

Fixes #66938

Old iterator is invalidaed upon SmallVector elements additions.
Stores index instead of iterator to avoid this.

Fixes llvm#66938
@hazohelet hazohelet added the clang Clang issues not falling into any other category label Sep 20, 2023
@llvmbot llvmbot added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Sep 20, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 20, 2023

@llvm/pr-subscribers-clang

Changes

Old iterator is invalidated upon SmallVector elements additions. Stores index instead of iterator to avoid this.

Fixes #66938


Full diff: https://github.com/llvm/llvm-project/pull/66954.diff

2 Files Affected:

  • (modified) clang/lib/Sema/SemaConcept.cpp (+5-2)
  • (modified) clang/test/SemaTemplate/concepts.cpp (+16)
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index dacdd07c8069950..80788f04e2241c5 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -185,7 +185,7 @@ calculateConstraintSatisfaction(Sema &S, const Expr *ConstraintExpr,
   ConstraintExpr = ConstraintExpr->IgnoreParenImpCasts();
 
   if (LogicalBinOp BO = ConstraintExpr) {
-    auto EffectiveDetailEnd = Satisfaction.Details.end();
+    size_t EffectiveDetailEndIndex = Satisfaction.Details.size();
     ExprResult LHSRes = calculateConstraintSatisfaction(
         S, BO.getLHS(), Satisfaction, Evaluator);
 
@@ -228,9 +228,12 @@ calculateConstraintSatisfaction(Sema &S, const Expr *ConstraintExpr,
     // The following code removes the irrelevant diagnostic information.
     // FIXME: We should probably delay the addition of diagnostic information
     // until we know the entire expression is false.
-    if (BO.isOr() && IsRHSSatisfied)
+    if (BO.isOr() && IsRHSSatisfied) {
+      auto EffectiveDetailEnd =
+          Satisfaction.Details.begin() + EffectiveDetailEndIndex;
       Satisfaction.Details.erase(EffectiveDetailEnd,
                                  Satisfaction.Details.end());
+    }
 
     return BO.recreateBinOp(S, LHSRes, RHSRes);
   }
diff --git a/clang/test/SemaTemplate/concepts.cpp b/clang/test/SemaTemplate/concepts.cpp
index 68050e0f09e248a..e98ebcc9203a430 100644
--- a/clang/test/SemaTemplate/concepts.cpp
+++ b/clang/test/SemaTemplate/concepts.cpp
@@ -1048,3 +1048,19 @@ namespace GH66612 {
   // expected-note@-1{{because 'int' does not satisfy 'Container'}}
   // expected-note@#66612GH_END{{because 'end' would be invalid: reference to overloaded function could not be resolved; did you mean to call it?}}
 }
+
+namespace GH66938 {
+template <class>
+concept True = true;
+
+template <class>
+concept False = false;
+
+template <class T>
+void cand(T t)
+  requires False<T> || False<T> || False<T> || False<T> || False<T> ||
+           False<T> || False<T> || False<T> || False<T> || True<T>
+{}
+
+void test() { cand(42); }
+}

Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 suggestion, else LGTM.

if (BO.isOr() && IsRHSSatisfied)
if (BO.isOr() && IsRHSSatisfied) {
auto EffectiveDetailEnd =
Satisfaction.Details.begin() + EffectiveDetailEndIndex;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Satisfaction.Details.begin() + EffectiveDetailEndIndex;
std::advance(Satisfaction.Details.begin(), EffectiveDetailEndIndex);

@shafik
Copy link
Collaborator

shafik commented Sep 21, 2023

Thanks for the quick fix!

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

@hazohelet hazohelet merged commit 4af62db into llvm:main Sep 21, 2023
@hazohelet hazohelet deleted the crash-fix-concept branch February 24, 2024 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clang 18 llvm::report_bad_alloc_error in Sema::CheckConstraintSatisfaction
5 participants