[clang] add APValue type check in `TryPrintAsStringLiteral`

[inclyc]

Summary

Fixes https://github.com/llvm/llvm-project/issues/57013

https://reviews.llvm.org/D115031 improved printing of non-type template
parameter args. But checking if the end of Inits is 0 without checking
if APValue is an integer, causes clang to segfault. This patch adds
the code to check the type. (May not be a proper bugfix.)

[inclyc]

.

[shafik]

clang/lib/AST/APValue.cpp

660

I believe to be fully correct we also need to add:

if (!Val.isInt())
  return false;

[shafik]

I wonder if APValue needs a member function to verify the underlying type of an Array

[inclyc]

Address reviewer's comments

[aaron.ballman]

Can you add test coverage for these changes?

[inclyc]

address comments

[inclyc]

clang/test/SemaCXX/try-print-as-string-literal-type-check.cpp

8

The ArrayRef<APValue> Inits points to here does not ends with Int, so !Inits.back().getInt() gets assertion failed.

[lichray]

LGTM

[aaron.ballman]

LGTM!

I think this change should be backported to Clang 15.x since the breakage happened in that release and the fix is pretty straightforward. After this lands, I'll file the issue for it to get that process started. Thank you for the fix!

clang/lib/AST/APValue.cpp

static bool TryPrintAsStringLiteral(raw_ostream &Out,
   if (Inits.empty())
     return false;
 
   QualType Ty = ATy->getElementType();
   if (!Ty->isAnyCharacterType())
     return false;
 
   // Nothing we can do about a sequence that is not null-terminated
-  if (!Inits.back().getInt().isZero())
+  if (!Inits.back().isInt() || !Inits.back().getInt().isZero())
     return false;
-  else
-    Inits = Inits.drop_back();
+
 
   llvm::SmallString<40> Buf;
   Buf.push_back('"');
 
   // Better than printing a two-digit sequence of 10 integers.
   constexpr size_t MaxN = 36;
   StringRef Ellipsis;
   if (Inits.size() > MaxN && !Policy.EntireContentsOfLargeArray) {
     Ellipsis = "[...]";
     Inits =
         Inits.take_front(std::min(MaxN - Ellipsis.size() / 2, Inits.size()));
   }
 
   for (auto &Val : Inits) {
+    if (!Val.isInt())
+      return false;
     int64_t Char64 = Val.getInt().getExtValue();
     if (!isASCII(Char64))
       return false; // Bye bye, see you in integers.
     auto Ch = static_cast<unsigned char>(Char64);
     // The diagnostic message is 'quoted'
     StringRef Escaped = escapeCStyle<EscapeChar::SingleAndDouble>(Ch);
     if (Escaped.empty()) {
       if (!isPrintable(Ch))
         return false;

clang/test/SemaCXX/try-print-as-string-literal-type-check.cpp

+// RUN: %clang_cc1 %s -std=c++20 -fsyntax-only -verify
+// expected-no-diagnostics
+
+// Reported by: https://github.com/llvm/llvm-project/issues/57013
+// The following code should not crash clang
+struct X {
+  char arr[2];
+  constexpr X() {}
+  constexpr void modify() {
+    arr[0] = 0;
+  }
+};
+constexpr X f(X t) {
+    t.modify();
+    return t;
+}
+auto x = f(X());