Skip to content

Conversation

@AaronBallman
Copy link
Collaborator

@AaronBallman AaronBallman commented Jun 25, 2025

In one case, we have a null pointer check that's unnecessary because the only caller of the function already asserts the value is non-null.

In the other case, we've got an anti-pattern of is followed by get. The logic was easier to repair by changing get to cast.

Neither case is a functional change.

Fixes #145525

In one case, we have a null pointer check that's unnecessary because
the only caller of the function already asserts the value is non-null.

In the other case, we've got an anti-pattern of `is` followed by `get`.
The logic was easier to repair by changing `get` to `cast`.

Neither case is a functional change.
@AaronBallman AaronBallman added clang:frontend Language frontend issues, e.g. anything involving "Sema" HLSL HLSL Language Support labels Jun 25, 2025
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels Jun 25, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 25, 2025

@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-hlsl

Author: Aaron Ballman (AaronBallman)

Changes

In one case, we have a null pointer check that's unnecessary because the only caller of the function already asserts the value is non-null.

In the other case, we've got an anti-pattern of is followed by get. The logic was easier to repair by changing get to cast.

Neither case is a functional change.


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

2 Files Affected:

  • (modified) clang/lib/CodeGen/CGHLSLRuntime.cpp (+1-2)
  • (modified) clang/lib/Sema/SemaHLSL.cpp (+1-1)
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index 3103f1798e14e..85e602481e647 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -589,8 +589,7 @@ void CGHLSLRuntime::initializeBufferFromBinding(const HLSLBufferDecl *BufDecl,
   auto *NonUniform = llvm::ConstantInt::get(Int1Ty, false);
   auto *Index = llvm::ConstantInt::get(CGM.IntTy, 0);
   auto *RangeSize = llvm::ConstantInt::get(CGM.IntTy, 1);
-  auto *Space =
-      llvm::ConstantInt::get(CGM.IntTy, RBA ? RBA->getSpaceNumber() : 0);
+  auto *Space = llvm::ConstantInt::get(CGM.IntTy, RBA->getSpaceNumber());
   Value *Name = nullptr;
 
   llvm::Intrinsic::ID IntrinsicID =
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 506da8a361edf..4d61ced9b0787 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -2418,7 +2418,7 @@ static bool CheckFloatOrHalfRepresentation(Sema *S, SourceLocation Loc,
                                            clang::QualType PassedType) {
   clang::QualType BaseType =
       PassedType->isVectorType()
-          ? PassedType->getAs<clang::VectorType>()->getElementType()
+          ? PassedType->castAs<clang::VectorType>()->getElementType()
           : PassedType;
   if (!BaseType->isHalfType() && !BaseType->isFloat32Type())
     return S->Diag(Loc, diag::err_builtin_invalid_arg_type)

@AaronBallman
Copy link
Collaborator Author

Windows precommit CI failures appear to be unrelated:

[6099/6110] Running clang_tools regression tests
llvm-lit.py: C:\_work\llvm-project\llvm-project\llvm\utils\lit\lit\llvm\config.py:57: note: using lit tools: C:\Program Files\Git\usr\bin
llvm-lit.py: C:\_work\llvm-project\llvm-project\llvm\utils\lit\lit\llvm\config.py:520: note: using clang: c:\_work\llvm-project\llvm-project\build\bin\clang.exe
llvm-lit.py: C:\_work\llvm-project\llvm-project\llvm\utils\lit\lit\llvm\config.py:312: fatal: Couldn't find the include dir for Clang ('c:\_work\llvm-project\llvm-project\build\bin\clang.exe')
c:\_work\llvm-project\llvm-project\build\bin\clang.exe
[6101/6110] Linking CXX executable bin\obj2yaml.exe
FAILED: tools/clang/tools/extra/CMakeFiles/check-clang-tools 
cmd.exe /C "cd /D C:\_work\llvm-project\llvm-project\build\tools\clang\tools\extra && C:\Python39\python.exe C:/_work/llvm-project/llvm-project/build/./bin/llvm-lit.py -v --xunit-xml-output C:/_work/llvm-project/llvm-project/build/test-results.xml --use-unique-output-file-name --timeout=1200 --time-tests C:/_work/llvm-project/llvm-project/build/tools/clang/tools/extra/include-cleaner/test C:/_work/llvm-project/llvm-project/build/tools/clang/tools/extra/clangd/test/../unittests C:/_work/llvm-project/llvm-project/build/tools/clang/tools/extra/clangd/test C:/_work/llvm-project/llvm-project/build/tools/clang/tools/extra/test"
[6103/6110] Linking CXX executable bin\clang.exe

@AaronBallman AaronBallman merged commit d3fd792 into llvm:main Jun 26, 2025
8 checks passed
@AaronBallman AaronBallman deleted the aballman-sa-hlsl-fixes branch June 26, 2025 10:55
Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

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

Thank you for looking into this one.

anthonyhatran pushed a commit to anthonyhatran/llvm-project that referenced this pull request Jun 26, 2025
In one case, we have a null pointer check that's unnecessary because the
only caller of the function already asserts the value is non-null.

In the other case, we've got an anti-pattern of `is` followed by `get`.
The logic was easier to repair by changing `get` to `cast`.

Neither case is a functional change.

Fixes llvm#145525
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:codegen IR generation bugs: mangling, exceptions, etc. clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category HLSL HLSL Language Support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CGHLSLRuntime::initializeBufferFromBinding unconditionally dereferencing RBA after a nullptr check

4 participants