-
Notifications
You must be signed in to change notification settings - Fork 74
Crash with invalid preamble and concepts #1726
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
Comments
zyn0217
added a commit
to llvm/llvm-project
that referenced
this issue
Sep 1, 2023
…tFailure We're expecting a SubstitutionDiagnostic in diagnoseUnsatisfiedRequirement if the status of ExprRequirement is SubstFailure. Previously, the Requirement was created with Expr on SubstFailure by mistake, which could lead to the assertion failure in the subsequent diagnosis. Fixes clangd/clangd#1726 Fixes #64723 Fixes #64172 In addition, this patch also fixes an invalid test from D129499. Reviewed By: erichkeane Differential Revision: https://reviews.llvm.org/D158061
zyn0217
added a commit
to zyn0217/llvm-project
that referenced
this issue
Sep 4, 2023
…tFailure We're expecting a SubstitutionDiagnostic in diagnoseUnsatisfiedRequirement if the status of ExprRequirement is SubstFailure. Previously, the Requirement was created with Expr on SubstFailure by mistake, which could lead to the assertion failure in the subsequent diagnosis. Fixes clangd/clangd#1726 Fixes llvm#64723 Fixes llvm#64172 In addition, this patch also fixes an invalid test from D129499. Reviewed By: erichkeane Differential Revision: https://reviews.llvm.org/D158061
tru
pushed a commit
to llvm/llvm-project-release-prs
that referenced
this issue
Sep 5, 2023
…tFailure We're expecting a SubstitutionDiagnostic in diagnoseUnsatisfiedRequirement if the status of ExprRequirement is SubstFailure. Previously, the Requirement was created with Expr on SubstFailure by mistake, which could lead to the assertion failure in the subsequent diagnosis. Fixes clangd/clangd#1726 Fixes llvm/llvm-project#64723 Fixes llvm/llvm-project#64172 In addition, this patch also fixes an invalid test from D129499. Reviewed By: erichkeane Differential Revision: https://reviews.llvm.org/D158061
avillega
pushed a commit
to avillega/llvm-project
that referenced
this issue
Sep 11, 2023
…tFailure We're expecting a SubstitutionDiagnostic in diagnoseUnsatisfiedRequirement if the status of ExprRequirement is SubstFailure. Previously, the Requirement was created with Expr on SubstFailure by mistake, which could lead to the assertion failure in the subsequent diagnosis. Fixes clangd/clangd#1726 Fixes llvm#64723 Fixes llvm#64172 In addition, this patch also fixes an invalid test from D129499. Reviewed By: erichkeane Differential Revision: https://reviews.llvm.org/D158061
qihangkong
pushed a commit
to rvgpu/llvm
that referenced
this issue
Apr 18, 2024
…tFailure We're expecting a SubstitutionDiagnostic in diagnoseUnsatisfiedRequirement if the status of ExprRequirement is SubstFailure. Previously, the Requirement was created with Expr on SubstFailure by mistake, which could lead to the assertion failure in the subsequent diagnosis. Fixes clangd/clangd#1726 Fixes llvm/llvm-project#64723 Fixes llvm/llvm-project#64172 In addition, this patch also fixes an invalid test from D129499. Reviewed By: erichkeane Differential Revision: https://reviews.llvm.org/D158061
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
(I think I should file the report to llvm repository, but I couldn't reproduce the crash directly with clang since it would stop building the AST once the including header is invalid/missing.)
The problematic code was observed from gnu.gold when I was randomly looking for something without a configured compile_commands.json.
Relevant output:
And the stacktrace:
The stacktrace shows that we're extracting
SubstitutionDiagnostic
from anExprRequirement
, while the stored pointer inExprRequirement
actually points to anExpr
, not aSubstitutionDiagnostic
. After some digging, I can conclude that this is caused by D129499, in which theExprRequirement
is constructed withSS_ExprSubstitutionFailure
but without specifying anySubstitutionDiagnostic
which is expected in Sema if the status is set toSS_ExprSubstitutionFailure
.Then I patched the code with a
SubstitutionDiagnostic
, and the crash was indeed solved. Now I can see the real diagnostic indicating that,I've tracked down the
SubstExpr
and found that the substitution failure happened inSema::BuildDeclarationNameExpr
, where a RecoveryExpr might be built due to the invalidity of VD, and then we'll exercise theisSFINAEContext
code path, resulting in an empty invalid expression instead of a RecoveryExpr, while it would be returned from calculateConstraintSatisfaction without storing diagnostic information inSatisfaction
parameter. (I suppose this could somehow relate to the deferred concept instantiation strategy, but I'm not sure.)Regardless, before sending out the patch, I think it'd be better to get rid of headers from the code and reproduce this crash in clang. Any kind help/comment would be appreciated.
The text was updated successfully, but these errors were encountered: