[clang] [clang] Constant-evaluate format strings as last resort (PR #135864)

[via cfe-commits]

================
@@ -6331,26 +6321,79 @@ static StringLiteralCheckType checkFormatStringExpr(
       }
     }
 
-    return SLCT_NotALiteral;
+    // try to constant-evaluate the string
+    break;
   }
 
   default:
+    // try to constant-evaluate the string
+    break;
+  }
+
+  const StringLiteral *FakeLiteral = nullptr;
+  switch (constEvalStringAsLiteral(S, E, FakeLiteral)) {
+  case SLCER_NotEvaluated:
     return SLCT_NotALiteral;
+
+  case SLCER_NotNullTerminated:
+    S.Diag(Args[format_idx]->getBeginLoc(),
+           diag::warn_printf_format_string_not_null_terminated)
+        << Args[format_idx]->getSourceRange();
+    if (!InFunctionCall)
+      S.Diag(E->getBeginLoc(), diag::note_format_string_defined);
+    // Stop checking, as this might just mean we're missing a chunk of the
+    // format string and there would be other spurious format issues.
+    return SLCT_UncheckedLiteral;
+
+  case SLCER_Evaluated:
+    InFunctionCall = false;
+    E = FakeLiteral;
+    goto tryAgain;
   }
 }
 
-// If this expression can be evaluated at compile-time,
-// check if the result is a StringLiteral and return it
-// otherwise return nullptr
-static const Expr *maybeConstEvalStringLiteral(ASTContext &Context,
-                                               const Expr *E) {
+static StringLiteralConstEvalResult
+constEvalStringAsLiteral(Sema &S, const Expr *E, const StringLiteral *&SL) {
+  // As a last resort, try to constant-evaluate the format string. If it
+  // evaluates to a string literal in the first place, we can point to that
+  // string literal in source and use that.
   Expr::EvalResult Result;
-  if (E->EvaluateAsRValue(Result, Context) && Result.Val.isLValue()) {
+  if (E->EvaluateAsRValue(Result, S.Context) && Result.Val.isLValue()) {
     const auto *LVE = Result.Val.getLValueBase().dyn_cast<const Expr *>();
-    if (isa_and_nonnull<StringLiteral>(LVE))
-      return LVE;
+    if (auto *BaseSL = dyn_cast_or_null<StringLiteral>(LVE)) {
+      SL = BaseSL;
+      return SLCER_Evaluated;
+    }
   }
-  return nullptr;
+
+  // Otherwise, try to evaluate the expression as a string constant.
+  std::string FormatString;
+  if (!E->tryEvaluateString(S.Context, FormatString)) {
+    return FormatString.empty() ? SLCER_NotEvaluated : SLCER_NotNullTerminated;
+  }
+
+  std::unique_ptr<llvm::MemoryBuffer> MemBuf;
+  {
+    llvm::SmallString<80> EscapedString;
+    {
+      llvm::raw_svector_ostream OS(EscapedString);
+      OS << '"';
+      OS.write_escaped(FormatString);
----------------
apple-fcloutier wrote:

I tried with `-fixit` and had the same result. For clang-tidy, I tried this:

```
% cat /tmp/test.c
__attribute__((format(printf, 1, 2)))
int printf(const char *, ...);

int main() {
	const char buf[] = {'"', '%', 's', '"', 0};
	printf(buf, 123);
	printf("%s", 123);
}
% bin/clang-tidy --fix /tmp/test.c
....
2 warnings generated.
/tmp/test.c:6:17: warning: format specifies type 'char *' but the argument has type 'int' [clang-diagnostic-format]
    6 |     printf(buf, 123);
      |            ~~~  ^~~
note: format string resolved to a constant string
/tmp/test.c:7:18: warning: format specifies type 'char *' but the argument has type 'int' [clang-diagnostic-format]
    7 |     printf("%s", 123);
      |             ~~   ^~~
      |             %d
/tmp/test.c:7:13: note: FIX-IT applied suggested code changes
    7 |     printf("%s", 123);
      |             ^
clang-tidy applied 1 of 1 suggested fixes.
```

In words: it fixes the `printf("%s", 123)` line to use `%d` and leaves alone the other printf alone without throwing a fuss (claiming "clang-tidy applied 1 of 1 suggested fixes"). Clang-tidy shows the "format string resolved to a constant string" note but not the scratch space contents. It's not ideal, but it's quite reasonable IMO.

https://github.com/llvm/llvm-project/pull/135864