[Clang] [Frontend] fix crash on parsing ternary operator with vector_size condition

[pskrgag]

Fix clang crash on parsing ternary operator with incompatible or pointer types as result.

Closes: #101718

[llvmbot]

@llvm/pr-subscribers-clang

Author: Pavel Skripkin (pskrgag)

Changes

Fix clang crash on parsing ternary operator with incompatible or pointer types as result.

Closes: #101718

---

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

3 Files Affected:

• (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+2)
• (modified) clang/lib/Sema/SemaExprCXX.cpp (+10)
• (modified) clang/test/SemaCXX/vector-size-conditional.cpp (+6-2)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 581434d33c5c9..848c5ffda4ea5 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8187,6 +8187,8 @@ def err_conditional_vector_has_void : Error<
   "GNU vector conditional operand cannot be %select{void|a throw expression}0">;
 def err_conditional_vector_operand_type
     : Error<"enumeration type %0 is not allowed in a vector conditional">;
+def err_conditional_vector_result_pointer_type
+    : Error<"pointer type %0 is not allowed in a vector conditional">;
 def err_conditional_vector_cond_result_mismatch
     : Error<"cannot mix vectors and extended vectors in a vector conditional">;
 def err_conditional_vector_mismatched
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 1b56b4cabd133..a43fc36fa8047 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -6719,6 +6719,16 @@ QualType Sema::CheckVectorConditionalTypes(ExprResult &Cond, ExprResult &LHS,
             : UsualArithmeticConversions(LHS, RHS, QuestionLoc,
                                          ACK_Conditional);
 
+    if (ResultElementTy.isNull()) {
+      Diag(QuestionLoc, diag::err_conditional_vector_mismatched)
+          << LHSType << RHSType;
+      return {};
+    }
+    if (ResultElementTy->isPointerType()) {
+      Diag(QuestionLoc, diag::err_conditional_vector_result_pointer_type)
+          << ResultElementTy;
+      return {};
+    }
     if (ResultElementTy->isEnumeralType()) {
       Diag(QuestionLoc, diag::err_conditional_vector_operand_type)
           << ResultElementTy;
diff --git a/clang/test/SemaCXX/vector-size-conditional.cpp b/clang/test/SemaCXX/vector-size-conditional.cpp
index afed1cdeec087..51a22504a0d33 100644
--- a/clang/test/SemaCXX/vector-size-conditional.cpp
+++ b/clang/test/SemaCXX/vector-size-conditional.cpp
@@ -110,6 +110,10 @@ void Operands() {
   (void)(four_shorts ? four_shorts : uss); // expected-error {{cannot convert between scalar type 'unsigned short' and vector type 'FourShorts'}}
   (void)(four_ints ? four_floats : us);    // expected-error {{cannot convert between scalar type 'unsigned int' and vector type 'FourFloats'}}
   (void)(four_ints ? four_floats : sint);  // expected-error {{cannot convert between scalar type 'int' and vector type 'FourFloats'}}
+  (void)(four_ints ? &four_floats : &sint);  // expected-error {{vector operands to the vector conditional must be the same type ('FourFloats *' and 'int *')}}
+  (void)(four_ints ? &four_ints : &four_ints);  // expected-error {{pointer type 'FourInts *' is not allowed in a vector conditional}}
+  (void)(four_ints ? 0 : &four_ints);  // expected-error {{vector operands to the vector conditional must be the same type ('int' and 'FourInts *')}}
+  (void)(four_ints ? &four_ints: 0);  // expected-error {{vector operands to the vector conditional must be the same type ('FourInts *' and 'int')}}
 }
 
 template <typename T1, typename T2>
@@ -169,10 +173,10 @@ void all_dependent(Cond C, LHS L, RHS R) {
 void Templates() {
   dependent_cond(two_ints);
   dependent_operand(two_floats);
-  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int)))) unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double)))) double' (vector of 4 'double' values))}}}
+  // expected-error@169 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int)))) unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double)))) double' (vector of 4 'double' values))}}}
   all_dependent(four_ints, four_uints, four_doubles); // expected-note {{in instantiation of}}
 
-  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int)))) unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int)))) unsigned int' (vector of 2 'unsigned int' values))}}}
+  // expected-error@169 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int)))) unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int)))) unsigned int' (vector of 2 'unsigned int' values))}}}
   all_dependent(four_ints, four_uints, two_uints); // expected-note {{in instantiation of}}
   all_dependent(four_ints, four_uints, four_uints);
 }

[AaronBallman]

Thank you for working on this! Please be sure to add a release note to clang/docs/ReleaseNotes.rst so users know about the fix.

[AaronBallman]

The rules seem to be different between C and C++ in GCC, and I can't quite suss out what Clang's rules actually are.

https://godbolt.org/z/q3Wea5sYd

CC @fhahn @simoll

[AaronBallman]

To save ourselves headaches in the future, I would recommend a slightly different change here. On line 169, put a comment: // #dep_conditional and on these two lines, switch the comment to // expected-error@#dep_conditional

Then we can modify the test without needing to update these two diagnostics due to line number differences.

[mizvekov]

So we are checking if we are dealing with the same type just above.

If we don't support different types, then the call to UsualArithmeticConversions is useless, so it can be removed and this can be further simplified:

Suggested change

	if (ResultElementTy.isNull()) {
	Diag(QuestionLoc, diag::err_conditional_vector_mismatched)
	<< LHSType << RHSType;
	return {};
	}
	if (!Context.hasSameType(LHSType, RHSType)) {
	Diag(QuestionLoc, diag::err_conditional_vector_mismatched)
	<< LHSType << RHSType;
	return {};
	}
	QualType ResultElementTy = Context.getCommonSugaredType(LHSType, RHSType);

[pskrgag]

Yeah, it looks better, thanks!

I am actually not sure if we support different types or not. vector_size is an extension and there is no clear wording what is allowed and what is not...