Skip to content
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

Regression, SIGSEGV instead of diagnostics when encountering bool operator==(const T&, const T&) #6426

Closed
Erfan-Ahmadi opened this issue Mar 18, 2024 · 4 comments · Fixed by #6439
Assignees
Labels
bug Bug, regression, crash incorrect-code Issues relating to handling of incorrect code

Comments

@Erfan-Ahmadi
Copy link

Description
SIGSEGV instead of diagnostics when encountering bool operator==(const T&, const T&)

Steps to reproduce:
Godbolt link: https://godbolt.org/z/5hsxeK5aE

@Erfan-Ahmadi Erfan-Ahmadi added bug Bug, regression, crash needs-triage Awaiting triage labels Mar 18, 2024
@damyanp damyanp moved this to For MSFT in HLSL Triage Mar 18, 2024
@damyanp damyanp added this to the Next Release milestone Mar 18, 2024
@damyanp damyanp added incorrect-code Issues relating to handling of incorrect code and removed needs-triage Awaiting triage labels Mar 18, 2024
@damyanp
Copy link
Member

damyanp commented Mar 18, 2024

We also find that the compiler crashes in the non-spirv case as well, so we should have a look into this before declaring it spir-v specific.

@damyanp damyanp changed the title [SPIRV] Regression, SIGSEGV instead of diagnostics when encountering bool operator==(const T&, const T&) [SPIR-V] Regression, SIGSEGV instead of diagnostics when encountering bool operator==(const T&, const T&) Mar 18, 2024
@cassiebeckley
Copy link
Collaborator

Minimal reproducible example:

bool operator==(int lhs, int rhs) {
    return true;
}

void main() {}

@llvm-beanz
Copy link
Collaborator

The fix for this is pretty straightforward:

diff --git a/tools/clang/lib/Sema/SemaHLSL.cpp b/tools/clang/lib/Sema/SemaHLSL.cpp
index 31ff6a45851..704824c3822 100644
--- a/tools/clang/lib/Sema/SemaHLSL.cpp
+++ b/tools/clang/lib/Sema/SemaHLSL.cpp
@@ -15690,7 +15690,7 @@ void TryAddShaderAttrFromTargetProfile(Sema &S, FunctionDecl *FD,
 
   // if this FD isn't the entry point, then we shouldn't add
   // a shader attribute to this decl, so just return
-  if (EntryPointName != FD->getIdentifier()->getName()) {
+  if (FD->getIdentifier() && EntryPointName != FD->getIdentifier()->getName()) {
     return;
   }

We should craft some wider test cases too.

@pow2clk pow2clk changed the title [SPIR-V] Regression, SIGSEGV instead of diagnostics when encountering bool operator==(const T&, const T&) Regression, SIGSEGV instead of diagnostics when encountering bool operator==(const T&, const T&) Mar 20, 2024
@github-project-automation github-project-automation bot moved this to 🆕 New in HLSL Roadmap Mar 21, 2024
@damyanp damyanp moved this from 🆕 New to 🏗 In progress in HLSL Roadmap Mar 21, 2024
python3kgae added a commit that referenced this issue Mar 21, 2024
Use identifier name without check the identifier exists will cause
crash.

Fixes #6426

---------

Co-authored-by: Tex Riddell <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@github-project-automation github-project-automation bot moved this from For MSFT to Done in HLSL Triage Mar 21, 2024
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in HLSL Roadmap Mar 21, 2024
@damyanp damyanp reopened this Mar 21, 2024
@damyanp
Copy link
Member

damyanp commented Mar 21, 2024

Re-opening this to track getting the fix cherry-picked into the next point release.

@damyanp damyanp moved this from ✅ Done to 📋 Backlog in HLSL Roadmap Mar 25, 2024
python3kgae added a commit that referenced this issue Mar 26, 2024
Use identifier name without check the identifier exists will cause
crash.

Fixes #6426

---------

Co-authored-by: Tex Riddell <[email protected]>
Co-authored-by: github-actions[bot]
<github-actions[bot]@users.noreply.github.com>
(cherry picked from commit 7581ff4)
@damyanp damyanp moved this from 📋 Backlog to ✅ Done in HLSL Roadmap Mar 26, 2024
@damyanp damyanp closed this as completed Apr 2, 2024
pow2clk pushed a commit to pow2clk/DirectXShaderCompiler that referenced this issue Jul 15, 2024
) (microsoft#6457)

Use identifier name without check the identifier exists will cause
crash.

Fixes microsoft#6426

---------

Co-authored-by: Tex Riddell <[email protected]>
Co-authored-by: github-actions[bot]
<github-actions[bot]@users.noreply.github.com>
(cherry picked from commit 7581ff4)
(cherry picked from commit 11e1318)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug, regression, crash incorrect-code Issues relating to handling of incorrect code
Projects
Archived in project
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants