[HLSL] Bug fix crash using Array Parameters when De-sugaring is the same as canonicalizing

[spall]

Fixes this crash: https://hlsl.godbolt.org/z/9aP74s4bP
Which happens because the de-sugared type is the same as the canonicalized type.
Check if the de-sugared type is canonical before getting the ArrayParameterType of the canonical type.
Add AST test to ensure crash doesn't happen.

[llvmbot]

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Sarah Spall (spall)

Changes

Fixes this crash: https://hlsl.godbolt.org/z/9aP74s4bP
Which happens because the de-sugared type is the same as the canonicalized type.
Check if the de-sugared type is canonical before getting the ArrayParameterType of the canonical type.
Add AST test to ensure crash doesn't happen.

---

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

2 Files Affected:

• (modified) clang/lib/AST/ASTContext.cpp (+3-2)
• (modified) clang/test/AST/HLSL/TypdefArrayParam.hlsl (+11)

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index b1b9d56ccca9f..5aaf4ce5c9155 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -3902,7 +3902,8 @@ QualType ASTContext::getArrayParameterType(QualType Ty) const {
   if (Ty->isArrayParameterType())
     return Ty;
   assert(Ty->isConstantArrayType() && "Ty must be an array type.");
-  const auto *ATy = cast<ConstantArrayType>(Ty.getDesugaredType(*this));
+  QualType DTy = Ty.getDesugaredType(*this);
+  const auto *ATy = cast<ConstantArrayType>(DTy);
   llvm::FoldingSetNodeID ID;
   ATy->Profile(ID, *this, ATy->getElementType(), ATy->getZExtSize(),
                ATy->getSizeExpr(), ATy->getSizeModifier(),
@@ -3914,7 +3915,7 @@ QualType ASTContext::getArrayParameterType(QualType Ty) const {
     return QualType(AT, 0);
 
   QualType Canonical;
-  if (!Ty.isCanonical()) {
+  if (!DTy.isCanonical()) {
     Canonical = getArrayParameterType(getCanonicalType(Ty));
 
     // Get the new insert position for the node we care about.
diff --git a/clang/test/AST/HLSL/TypdefArrayParam.hlsl b/clang/test/AST/HLSL/TypdefArrayParam.hlsl
index c6ae168f84064..37f7a66de23a1 100644
--- a/clang/test/AST/HLSL/TypdefArrayParam.hlsl
+++ b/clang/test/AST/HLSL/TypdefArrayParam.hlsl
@@ -55,3 +55,14 @@ void call2() {
   uint4 C[2] = {A,A};
   uint32_t D = Accumulate(C);
 }
+
+typedef int Foo[2];
+
+// CHECK-LABEL: call3
+// CHECK: ArraySubscriptExpr {{.*}} 'int' lvalue
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'int *' <ArrayToPointerDecay>
+// CHECK-NEXT: DeclRefExpr {{.*}} 'int[2]' lvalue ParmVar {{.*}} 'F' 'int[2]'
+// CHECK-NEXT: IntegerLiteral {{.*}} 'int' 0
+int call3(Foo F) {
+  return F[0];
+}

[Icohedron]

Looks fine to me, but this change doesn't only affect HLSL does it?
At a glance, it doesn't look like the function getArrayParameterType in ASTContext.cpp is HLSL-specific.
I wonder if a similar bug occurs within C/C++.

[spall]

Looks fine to me, but this change doesn't only affect HLSL does it? The function getArrayParameterType in ASTContext.cpp doesn't look like it is HLSL-specific. I wonder if a similar bug occurs within C/C++.

I believe this function is only called in an HLSL context. and ArrayParameterTypes are a purely hlsl construct.