[clang] format string checks for `InitListExpr`

[inclyc]

Summary

this patch enhances clang check format strings defined in list
initialization expressions. Before this patch clang just thought these
expressions are not string literal.

Example:

constexpr const char *foo() { return "%s %d"; }

struct Bar {
  static constexpr char value[] = {'%', 's', '%', 'd', '\0'};
};

constexpr const char *foobar() { return Bar::value; }

constexpr const char *foooobar() { return foobar(); }

int main() {
  printf(foo(), "abc", "def");
  printf(Bar::value, "abc", "def");
  printf(foobar(), "abc", "def");
  printf(foooobar(), "abc", "def");
  return 0;
}

Diagnostics after this patch:

<source>:14:24: warning: format specifies type 'int' but the argument has type 'const char *' [-Wformat]
  printf(foo(), "abc", "def");
         ~~~~~         ^~~~~
<source>:3:42: note: format string is defined here
constexpr const char *foo() { return "%s %d"; }
                                         ^~
                                         %s
<source>:15:29: warning: format specifies type 'int' but the argument has type 'const char *' [-Wformat]
  printf(Bar::value, "abc", "def");
                            ^~~~~
<source>:16:27: warning: format specifies type 'int' but the argument has type 'const char *' [-Wformat]
  printf(foobar(), "abc", "def");
                          ^~~~~
<source>:17:29: warning: format specifies type 'int' but the argument has type 'const char *' [-Wformat]
  printf(foooobar(), "abc", "def");
                            ^~~~~
4 warnings generated.

[inclyc]

typo fix

[inclyc]

.

[inclyc]

.

[tbaeder]

Not a formal review of course, but for the diagnostics, I'm missing something that tells the user what the format string ended up looking like; in your example, the output never mentions that it checked "%s%d".

clang/lib/Sema/SemaChecking.cpp

8707

I'd prefer not to add more FIXME comments.

[inclyc]

There are too many things changed in this patch, I think the function of displaying the evaluation results, FIXME warning, these can be done in subsequent patches.

[inclyc]

Use isa<> to check Expr class

[inclyc]

rebase && qualify const

[inclyc]

rebase

[inclyc]

ping

[aaron.ballman]

In D131314#3707131, @inclyc wrote:

ping

FWIW, we usually only ping a review that hasn't had any activity in a week or more (it's not uncommon for reviews to sit for a few days while people think about them or reviewers are busy on other stuff).

I've not had the chance to do an in-depth review yet, but I have two thoughts I can share early on. I think you can simplify the patch a little bit by treating a string literal and an initializer list the same way (using an iterator to walk over the elements) instead of ginning up a fake StringLiteral AST node (that's a very heavy handed way to implement this). However, even with that simplification, I'm not certain the use cases for the diagnostic happen enough to warrant this large of a change in how we process format strings. I had encouraged such a diagnostic given the equivalence of the construct with string literals, but I was imagining that the support would be a few lines of code rather than anything substantial like this. Perhaps you can find a way to make the changes less invasive, but if not, I think we may want to hold off on this change until a user files an issue pointing out some real world code that would be easier to fix if they had such a diagnostic.

[inclyc]

In D131314#3710331, @aaron.ballman wrote:

In D131314#3707131, @inclyc wrote:

ping

FWIW, we usually only ping a review that hasn't had any activity in a week or more (it's not uncommon for reviews to sit for a few days while people think about them or reviewers are busy on other stuff).

I've not had the chance to do an in-depth review yet, but I have two thoughts I can share early on. I think you can simplify the patch a little bit by treating a string literal and an initializer list the same way (using an iterator to walk over the elements) instead of ginning up a fake StringLiteral AST node (that's a very heavy handed way to implement this). However, even with that simplification, I'm not certain the use cases for the diagnostic happen enough to warrant this large of a change in how we process format strings. I had encouraged such a diagnostic given the equivalence of the construct with string literals, but I was imagining that the support would be a few lines of code rather than anything substantial like this. Perhaps you can find a way to make the changes less invasive, but if not, I think we may want to hold off on this change until a user files an issue pointing out some real world code that would be easier to fix if they had such a diagnostic.

Clang seems to depend class FormatStringLiteral when performing format string checking, which has its SourceLocation, a wrapped pointer. With these information we can generate FixIt Hints directly on source codes. In previous version, we probably haven't even considered checking the format string evaluated from InitListExpr (just consider it is not a string literal). I've tried adding some ways to treat them as equals before, but it seems more invasive than this patch. Actually, I don't think any user would ever define a format string like {'%', 'd', 0} (That's exactly the reason I split format string checks in two patches). So maybe we should abandon this patch because there are no enough benefits to apply this?

but it seems more invasive than this patch.

It may be necessary to separate out the existing parts that check for formatting strings and related location information, because InitListExpr may not have proper information of source code location that can be determined during the format string checking.

[aaron.ballman]

In D131314#3710514, @inclyc wrote:

In D131314#3710331, @aaron.ballman wrote:

In D131314#3707131, @inclyc wrote:

ping

FWIW, we usually only ping a review that hasn't had any activity in a week or more (it's not uncommon for reviews to sit for a few days while people think about them or reviewers are busy on other stuff).

I've not had the chance to do an in-depth review yet, but I have two thoughts I can share early on. I think you can simplify the patch a little bit by treating a string literal and an initializer list the same way (using an iterator to walk over the elements) instead of ginning up a fake StringLiteral AST node (that's a very heavy handed way to implement this). However, even with that simplification, I'm not certain the use cases for the diagnostic happen enough to warrant this large of a change in how we process format strings. I had encouraged such a diagnostic given the equivalence of the construct with string literals, but I was imagining that the support would be a few lines of code rather than anything substantial like this. Perhaps you can find a way to make the changes less invasive, but if not, I think we may want to hold off on this change until a user files an issue pointing out some real world code that would be easier to fix if they had such a diagnostic.

Clang seems to depend class FormatStringLiteral when performing format string checking, which has its SourceLocation, a wrapped pointer. With these information we can generate FixIt Hints directly on source codes. In previous version, we probably haven't even considered checking the format string evaluated from InitListExpr (just consider it is not a string literal). I've tried adding some ways to treat them as equals before, but it seems more invasive than this patch. Actually, I don't think any user would ever define a format string like {'%', 'd', 0} (That's exactly the reason I split format string checks in two patches). So maybe we should abandon this patch because there are no enough benefits to apply this?

I think that might be the best course of action, though I'm really sorry to have asked you to do this work only to turn around and say "nevermind." I just don't see enough users writing their format strings that way for the amount of effort it takes to support diagnosing it.

clang/lib/Sema/SemaChecking.cpp

 };
 
 } // namespace
 
 static void CheckFormatString(
     Sema &S, const FormatStringLiteral *FExpr, const Expr *OrigFormatExpr,
     ArrayRef<const Expr *> Args, Sema::FormatArgumentPassingKind APK,
     unsigned format_idx, unsigned firstDataArg, Sema::FormatStringType Type,
-    bool inFunctionCall, Sema::VariadicCallType CallType,
+    bool NoNote, Sema::VariadicCallType CallType,
     llvm::SmallBitVector &CheckedVarArgs, UncoveredArgHandler &UncoveredArg,
     bool IgnoreStringsWithoutSpecifiers);
 
-static const Expr *maybeConstEvalStringLiteral(ASTContext &Context,
-                                               const Expr *E);
-
-// Determine if an expression is a string literal or constant string.
-// If this function returns false on the arguments to a function expecting a
-// format string, we will usually need to emit a warning.
-// True string literals are then checked by CheckFormatString.
 static StringLiteralCheckType
-checkFormatStringExpr(Sema &S, const Expr *E, ArrayRef<const Expr *> Args,
-                      Sema::FormatArgumentPassingKind APK, unsigned format_idx,
-                      unsigned firstDataArg, Sema::FormatStringType Type,
-                      Sema::VariadicCallType CallType, bool InFunctionCall,
-                      llvm::SmallBitVector &CheckedVarArgs,
-                      UncoveredArgHandler &UncoveredArg, llvm::APSInt Offset,
-                      bool IgnoreStringsWithoutSpecifiers = false) {
+checkVarDecl(Sema &S, const Expr *E, ArrayRef<const Expr *> Args,
+             Sema::VariadicCallType CallType, bool NoNote,
+             UncoveredArgHandler &UncoveredArg, llvm::APSInt Offset, QualType T,
+             const VarDecl *VD, bool IgnoreStringsWithoutSpecifiers);
+
+static StringLiteralCheckType checkFormatStringExprEvaluated(
+    Sema &S, const Expr *E, ArrayRef<const Expr *> Args,
+    Sema::FormatArgumentPassingKind APK, unsigned format_idx,
+    unsigned firstDataArg, Sema::FormatStringType Type,
+    Sema::VariadicCallType CallType, bool NoNote,
+    llvm::SmallBitVector &CheckedVarArgs, UncoveredArgHandler &UncoveredArg,
+    llvm::APSInt Offset, bool IgnoreStringsWithoutSpecifiers = false) {
   if (S.isConstantEvaluated())
     return SLCT_NotALiteral;
 tryAgain:
   assert(Offset.isSigned() && "invalid offset");
 
   if (E->isTypeDependent() || E->isValueDependent())
     return SLCT_NotALiteral;
 
   E = E->IgnoreParenCasts();
 
   if (E->isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNotNull))
     // Technically -Wformat-nonliteral does not warn about this case.
     // The behavior of printf and friends in this case is implementation
     // dependent.  Ideally if the format string cannot be null then
     // it should have a 'nonnull' attribute in the function prototype.
     return SLCT_UncheckedLiteral;
 
   switch (E->getStmtClass()) {
   case Stmt::BinaryConditionalOperatorClass:
   case Stmt::ConditionalOperatorClass: {
     // The expression is a literal if both sub-expressions were, and it was
     // completely checked only if both sub-expressions were checked.
-    const AbstractConditionalOperator *C =
-        cast<AbstractConditionalOperator>(E);
+    const AbstractConditionalOperator *C = cast<AbstractConditionalOperator>(E);
 
     // Determine whether it is necessary to check both sub-expressions, for
     // example, because the condition expression is a constant that can be
     // evaluated at compile time.
     bool CheckLeft = true, CheckRight = true;
 
     bool Cond;
     if (C->getCond()->EvaluateAsBooleanCondition(Cond, S.getASTContext(),
                                                  S.isConstantEvaluated())) {
       if (Cond)
         CheckRight = false;
       else
         CheckLeft = false;
     }
 
     // We need to maintain the offsets for the right and the left hand side
     // separately to check if every possible indexed expression is a valid
     // string literal. They might have different offsets for different string
     // literals in the end.
     StringLiteralCheckType Left;
     if (!CheckLeft)
       Left = SLCT_UncheckedLiteral;
     else {
-      Left = checkFormatStringExpr(S, C->getTrueExpr(), Args, APK, format_idx,
-                                   firstDataArg, Type, CallType, InFunctionCall,
-                                   CheckedVarArgs, UncoveredArg, Offset,
-                                   IgnoreStringsWithoutSpecifiers);
+      Left = checkFormatStringExprEvaluated(
+          S, C->getTrueExpr(), Args, APK, format_idx, firstDataArg, Type,
+          CallType, NoNote, CheckedVarArgs, UncoveredArg, Offset,
       if (Left == SLCT_NotALiteral || !CheckRight) {
         return Left;
       }
     }
 
-    StringLiteralCheckType Right = checkFormatStringExpr(
+    StringLiteralCheckType Right = checkFormatStringExprEvaluated(
         S, C->getFalseExpr(), Args, APK, format_idx, firstDataArg, Type,
-        CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset,
+        CallType, NoNote, CheckedVarArgs, UncoveredArg, Offset,
         IgnoreStringsWithoutSpecifiers);
 
     return (CheckLeft && Left < Right) ? Left : Right;
   }
 
   case Stmt::ImplicitCastExprClass:
     E = cast<ImplicitCastExpr>(E)->getSubExpr();
     goto tryAgain;
case Stmt::PredefinedExprClass:
     return SLCT_UncheckedLiteral;
 
   case Stmt::DeclRefExprClass: {
     const DeclRefExpr *DR = cast<DeclRefExpr>(E);
 
     // As an exception, do not flag errors for variables binding to
     // const string literals.
     if (const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl())) {
-      bool isConstant = false;
-      QualType T = DR->getType();
-
-      if (const ArrayType *AT = S.Context.getAsArrayType(T)) {
-        isConstant = AT->getElementType().isConstant(S.Context);
-      } else if (const PointerType *PT = T->getAs<PointerType>()) {
-        isConstant = T.isConstant(S.Context) &&
-                     PT->getPointeeType().isConstant(S.Context);
-      } else if (T->isObjCObjectPointerType()) {
-        // In ObjC, there is usually no "const ObjectPointer" type,
-        // so don't check if the pointee type is constant.
-        isConstant = T.isConstant(S.Context);
-      }
-
-      if (isConstant) {
-        if (const Expr *Init = VD->getAnyInitializer()) {
-          // Look through initializers like const char c[] = { "foo" }
-          if (const InitListExpr *InitList = dyn_cast<InitListExpr>(Init)) {
-            if (InitList->isStringLiteralInit())
-              Init = InitList->getInit(0)->IgnoreParenImpCasts();
-          }
-          return checkFormatStringExpr(
-              S, Init, Args, APK, format_idx, firstDataArg, Type, CallType,
-              /*InFunctionCall*/ false, CheckedVarArgs, UncoveredArg, Offset);
-        }
-      }
-
-      // When the format argument is an argument of this function, and this
-      // function also has the format attribute, there are several interactions
-      // for which there shouldn't be a warning. For instance, when calling
-      // v*printf from a function that has the printf format attribute, we
-      // should not emit a warning about using `fmt`, even though it's not
-      // constant, because the arguments have already been checked for the
-      // caller of `logmessage`:
-      //
-      //  __attribute__((format(printf, 1, 2)))
-      //  void logmessage(char const *fmt, ...) {
-      //    va_list ap;
-      //    va_start(ap, fmt);
-      //    vprintf(fmt, ap);  /* do not emit a warning about "fmt" */
-      //    ...
-      // }
-      //
-      // Another interaction that we need to support is calling a variadic
-      // format function from a format function that has fixed arguments. For
-      // instance:
-      //
-      //  __attribute__((format(printf, 1, 2)))
-      //  void logstring(char const *fmt, char const *str) {
-      //    printf(fmt, str);  /* do not emit a warning about "fmt" */
-      //  }
-      //
-      // Same (and perhaps more relatably) for the variadic template case:
-      //
-      //  template<typename... Args>
-      //  __attribute__((format(printf, 1, 2)))
-      //  void log(const char *fmt, Args&&... args) {
-      //    printf(fmt, forward<Args>(args)...);
-      //           /* do not emit a warning about "fmt" */
-      //  }
-      //
-      // Due to implementation difficulty, we only check the format, not the
-      // format arguments, in all cases.
-      //
-      if (const auto *PV = dyn_cast<ParmVarDecl>(VD)) {
-        if (const auto *D = dyn_cast<Decl>(PV->getDeclContext())) {
-          for (const auto *PVFormat : D->specific_attrs<FormatAttr>()) {
-            bool IsCXXMember = false;
-            if (const auto *MD = dyn_cast<CXXMethodDecl>(D))
-              IsCXXMember = MD->isInstance();
-
-            bool IsVariadic = false;
-            if (const FunctionType *FnTy = D->getFunctionType())
-              IsVariadic = cast<FunctionProtoType>(FnTy)->isVariadic();
-            else if (const auto *BD = dyn_cast<BlockDecl>(D))
-              IsVariadic = BD->isVariadic();
-            else if (const auto *OMD = dyn_cast<ObjCMethodDecl>(D))
-              IsVariadic = OMD->isVariadic();
-
-            Sema::FormatStringInfo CallerFSI;
-            if (Sema::getFormatStringInfo(PVFormat, IsCXXMember, IsVariadic,
-                                          &CallerFSI)) {
-              // We also check if the formats are compatible.
-              // We can't pass a 'scanf' string to a 'printf' function.
-              if (PV->getFunctionScopeIndex() == CallerFSI.FormatIdx &&
-                  Type == S.GetFormatStringType(PVFormat)) {
-                // Lastly, check that argument passing kinds transition in a
-                // way that makes sense:
-                // from a caller with FAPK_VAList, allow FAPK_VAList
-                // from a caller with FAPK_Fixed, allow FAPK_Fixed
-                // from a caller with FAPK_Fixed, allow FAPK_Variadic
-                // from a caller with FAPK_Variadic, allow FAPK_VAList
-                switch (combineFAPK(CallerFSI.ArgPassingKind, APK)) {
-                case combineFAPK(Sema::FAPK_VAList, Sema::FAPK_VAList):
-                case combineFAPK(Sema::FAPK_Fixed, Sema::FAPK_Fixed):
-                case combineFAPK(Sema::FAPK_Fixed, Sema::FAPK_Variadic):
-                case combineFAPK(Sema::FAPK_Variadic, Sema::FAPK_VAList):
-                  return SLCT_UncheckedLiteral;
-                }
-              }
-            }
-          }
-        }
-      }
+      return checkVarDecl(S, E, Args, APK, format_idx, firstDataArg, Type,
+                          CallType, NoNote, CheckedVarArgs, UncoveredArg,
+                          Offset, DR->getType(), VD,
+                          IgnoreStringsWithoutSpecifiers);
     }
 
     return SLCT_NotALiteral;
   }
 
   case Stmt::CallExprClass:
   case Stmt::CXXMemberCallExprClass: {
     const CallExpr *CE = cast<CallExpr>(E);
-    if (const NamedDecl *ND = dyn_cast_or_null<NamedDecl>(CE->getCalleeDecl())) {
+    if (const NamedDecl *ND =
+            dyn_cast_or_null<NamedDecl>(CE->getCalleeDecl())) {
       bool IsFirst = true;
       StringLiteralCheckType CommonResult;
       for (const auto *FA : ND->specific_attrs<FormatArgAttr>()) {
         const Expr *Arg = CE->getArg(FA->getFormatIdx().getASTIndex());
-        StringLiteralCheckType Result = checkFormatStringExpr(
-            S, Arg, Args, APK, format_idx, firstDataArg, Type, CallType,
-            InFunctionCall, CheckedVarArgs, UncoveredArg, Offset,
+        StringLiteralCheckType Result = checkFormatStringExprEvaluated(
+            S, Arg, Args, APK, format_idx, firstDataArg, Type, CallType, NoNote,
+            CheckedVarArgs, UncoveredArg, Offset,
             IgnoreStringsWithoutSpecifiers);
         if (IsFirst) {
           CommonResult = Result;
           IsFirst = false;
         }
       }
       if (!IsFirst)
         return CommonResult;
 
       if (const auto *FD = dyn_cast<FunctionDecl>(ND)) {
         unsigned BuiltinID = FD->getBuiltinID();
         if (BuiltinID == Builtin::BI__builtin___CFStringMakeConstantString ||
             BuiltinID == Builtin::BI__builtin___NSStringMakeConstantString) {
           const Expr *Arg = CE->getArg(0);
-          return checkFormatStringExpr(
+          return checkFormatStringExprEvaluated(
               S, Arg, Args, APK, format_idx, firstDataArg, Type, CallType,
-              InFunctionCall, CheckedVarArgs, UncoveredArg, Offset,
+              NoNote, CheckedVarArgs, UncoveredArg, Offset,
               IgnoreStringsWithoutSpecifiers);
         }
       }
     }
-    if (const Expr *SLE = maybeConstEvalStringLiteral(S.Context, E))
-      return checkFormatStringExpr(S, SLE, Args, APK, format_idx, firstDataArg,
-                                   Type, CallType, /*InFunctionCall*/ false,
-                                   CheckedVarArgs, UncoveredArg, Offset,
-                                   IgnoreStringsWithoutSpecifiers);
     return SLCT_NotALiteral;
   }
   case Stmt::ObjCMessageExprClass: {
     const auto *ME = cast<ObjCMessageExpr>(E);
     if (const auto *MD = ME->getMethodDecl()) {
       if (const auto *FA = MD->getAttr<FormatArgAttr>()) {
         // As a special case heuristic, if we're using the method -[NSBundle
         // localizedStringForKey:value:table:], ignore any key strings that lack
         // format specifiers. The idea is that if the key doesn't have any
         // format specifiers then its probably just a key to map to the
         // localized strings. If it does have format specifiers though, then its
         // likely that the text of the key is the format string in the
         // programmer's language, and should be checked.
         const ObjCInterfaceDecl *IFace;
         if (MD->isInstanceMethod() && (IFace = MD->getClassInterface()) &&
             IFace->getIdentifier()->isStr("NSBundle") &&
             MD->getSelector().isKeywordSelector(
                 {"localizedStringForKey", "value", "table"})) {
           IgnoreStringsWithoutSpecifiers = true;
         }
 
         const Expr *Arg = ME->getArg(FA->getFormatIdx().getASTIndex());
-        return checkFormatStringExpr(
-            S, Arg, Args, APK, format_idx, firstDataArg, Type, CallType,
-            InFunctionCall, CheckedVarArgs, UncoveredArg, Offset,
+        return checkFormatStringExprEvaluated(
+            S, Arg, Args, APK, format_idx, firstDataArg, Type, CallType, NoNote,
+            CheckedVarArgs, UncoveredArg, Offset,
             IgnoreStringsWithoutSpecifiers);
       }
     }
 
     return SLCT_NotALiteral;
   }
   case Stmt::ObjCStringLiteralClass:
   case Stmt::StringLiteralClass: {
     const StringLiteral *StrE = nullptr;
 
     if (const ObjCStringLiteral *ObjCFExpr = dyn_cast<ObjCStringLiteral>(E))
       StrE = ObjCFExpr->getString();
     else
       StrE = cast<StringLiteral>(E);
 
     if (StrE) {
       if (Offset.isNegative() || Offset > StrE->getLength()) {
         // TODO: It would be better to have an explicit warning for out of
         // bounds literals.
         return SLCT_NotALiteral;
       }
       FormatStringLiteral FStr(StrE, Offset.sextOrTrunc(64).getSExtValue());
       CheckFormatString(S, &FStr, E, Args, APK, format_idx, firstDataArg, Type,
-                        InFunctionCall, CallType, CheckedVarArgs, UncoveredArg,
+                        NoNote, CallType, CheckedVarArgs, UncoveredArg,
                         IgnoreStringsWithoutSpecifiers);
       return SLCT_CheckedLiteral;
     }
 
     return SLCT_NotALiteral;
   }
+
+  case Stmt::InitListExprClass: {
+    const auto *ILE = cast<const InitListExpr>(E);
+    if (ILE->isStringLiteralInit()) {
+      const auto *SL =
+          dyn_cast<const StringLiteral>(ILE->getInit(0)->IgnoreParenImpCasts());
+      return checkFormatStringExprEvaluated(
+          S, SL, Args, APK, format_idx, firstDataArg, Type, CallType, NoNote,
+          CheckedVarArgs, UncoveredArg, Offset);
+    }
+    // looks like {'a', 'b', 'c'}
+    const ArrayRef<clang::Expr *> AR = ILE->inits();
+    SmallString<128> StrBuf;
+    QualType Ty;
+    bool HasCStringEnd = false;
+    for (const auto &Expr : AR) {
+      if (isa<CharacterLiteral>(Expr)) {
+        const auto *CL = cast<const CharacterLiteral>(Expr);
+        // construct a StringRef for this
+        unsigned int CharacterValue = CL->getValue();
+        if (CharacterValue == 0) {
+          HasCStringEnd = true;
+          break;
+        }
+        StrBuf.push_back(CharacterValue);
+        Ty = Expr->getType();
+      } else {
+        // not a CharacterLiteral
+        return SLCT_NotALiteral;
+      }
+    }
+    if (!HasCStringEnd) {
+      // TODO: fire a warning that this InitListExprClass does not end with
+      // '\0', with FixIt hints
+    }
+    const auto SR = StringRef(StrBuf);
+    const auto *SL = StringLiteral::Create(
+        S.Context, SR, StringLiteral::Ordinary,
+        /*Pascal*/ false, S.Context.getStringLiteralArrayType(Ty, SR.size()),
+        SourceLocation());
+    return checkFormatStringExprEvaluated(
+        S, SL, Args, APK, format_idx, firstDataArg, Type, CallType,
+        /*NoNote*/ true, CheckedVarArgs, UncoveredArg, Offset,
+        IgnoreStringsWithoutSpecifiers);
+  }
   case Stmt::BinaryOperatorClass: {
     const BinaryOperator *BinOp = cast<BinaryOperator>(E);
 
     // A string literal + an int offset is still a string literal.
     if (BinOp->isAdditiveOp()) {
       Expr::EvalResult LResult, RResult;
 
       bool LIsInt = BinOp->getLHS()->EvaluateAsInt(
case Stmt::UnaryOperatorClass: {
     return SLCT_NotALiteral;
   }
 
   default:
     return SLCT_NotALiteral;
   }
 }
 
-// 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 StringLiteralCheckType
+checkVarDecl(Sema &S, const Expr *E, ArrayRef<const Expr *> Args,
+             Sema::FormatArgumentPassingKind APK, unsigned format_idx,
+             unsigned firstDataArg, Sema::FormatStringType Type,
+             Sema::VariadicCallType CallType, bool NoNote,
+             llvm::SmallBitVector &CheckedVarArgs,
+             UncoveredArgHandler &UncoveredArg, llvm::APSInt Offset, QualType T,
+             const VarDecl *VD, bool IgnoreStringsWithoutSpecifiers) {
+  bool isConstant = false;
+
+  if (const ArrayType *AT = S.Context.getAsArrayType(T)) {
+    isConstant = AT->getElementType().isConstant(S.Context);
+  } else if (const PointerType *PT = T->getAs<PointerType>()) {
+    isConstant =
+        T.isConstant(S.Context) && PT->getPointeeType().isConstant(S.Context);
+  } else if (T->isObjCObjectPointerType()) {
+    // In ObjC, there is usually no "const ObjectPointer" type,
+    // so don't check if the pointee type is constant.
+    isConstant = T.isConstant(S.Context);
+  }
+
+  if (isConstant) {
+    if (const Expr *Init = VD->getAnyInitializer()) {
+      // Look through initializers like const char c[] = { "foo" }
+      if (const InitListExpr *InitList = dyn_cast<InitListExpr>(Init)) {
+        if (InitList->isStringLiteralInit())
+          Init = InitList->getInit(0)->IgnoreParenImpCasts();
+      }
+      return checkFormatStringExprEvaluated(
+          S, Init, Args, APK, format_idx, firstDataArg, Type, CallType,
+          /*NoNote*/ false, CheckedVarArgs, UncoveredArg, Offset);
+    }
+  }
+
+  // When the format argument is an argument of this function, and this
+  // function also has the format attribute, there are several interactions
+  // for which there shouldn't be a warning. For instance, when calling
+  // v*printf from a function that has the printf format attribute, we
+  // should not emit a warning about using `fmt`, even though it's not
+  // constant, because the arguments have already been checked for the
+  // caller of `logmessage`:
+  //
+  //  __attribute__((format(printf, 1, 2)))
+  //  void logmessage(char const *fmt, ...) {
+  //    va_list ap;
+  //    va_start(ap, fmt);
+  //    vprintf(fmt, ap);  /* do not emit a warning about "fmt" */
+  //    ...
+  // }
+  //
+  // Another interaction that we need to support is calling a variadic
+  // format function from a format function that has fixed arguments. For
+  // instance:
+  //
+  //  __attribute__((format(printf, 1, 2)))
+  //  void logstring(char const *fmt, char const *str) {
+  //    printf(fmt, str);  /* do not emit a warning about "fmt" */
+  //  }
+  //
+  // Same (and perhaps more relatably) for the variadic template case:
+  //
+  //  template<typename... Args>
+  //  __attribute__((format(printf, 1, 2)))
+  //  void log(const char *fmt, Args&&... args) {
+  //    printf(fmt, forward<Args>(args)...);
+  //           /* do not emit a warning about "fmt" */
+  //  }
+  //
+  // Due to implementation difficulty, we only check the format, not the
+  // format arguments, in all cases.
+  //
+  if (const auto *PV = dyn_cast<ParmVarDecl>(VD)) {
+    if (const auto *D = dyn_cast<Decl>(PV->getDeclContext())) {
+      for (const auto *PVFormat : D->specific_attrs<FormatAttr>()) {
+        bool IsCXXMember = false;
+        if (const auto *MD = dyn_cast<CXXMethodDecl>(D))
+          IsCXXMember = MD->isInstance();
+
+        bool IsVariadic = false;
+        if (const FunctionType *FnTy = D->getFunctionType())
+          IsVariadic = cast<FunctionProtoType>(FnTy)->isVariadic();
+        else if (const auto *BD = dyn_cast<BlockDecl>(D))
+          IsVariadic = BD->isVariadic();
+        else if (const auto *OMD = dyn_cast<ObjCMethodDecl>(D))
+          IsVariadic = OMD->isVariadic();
+
+        Sema::FormatStringInfo CallerFSI;
+        if (Sema::getFormatStringInfo(PVFormat, IsCXXMember, IsVariadic,
+                                      &CallerFSI)) {
+          // We also check if the formats are compatible.
+          // We can't pass a 'scanf' string to a 'printf' function.
+          if (PV->getFunctionScopeIndex() == CallerFSI.FormatIdx &&
+              Type == S.GetFormatStringType(PVFormat)) {
+            // Lastly, check that argument passing kinds transition in a
+            // way that makes sense:
+            // from a caller with FAPK_VAList, allow FAPK_VAList
+            // from a caller with FAPK_Fixed, allow FAPK_Fixed
+            // from a caller with FAPK_Fixed, allow FAPK_Variadic
+            // from a caller with FAPK_Variadic, allow FAPK_VAList
+            switch (combineFAPK(CallerFSI.ArgPassingKind, APK)) {
+            case combineFAPK(Sema::FAPK_VAList, Sema::FAPK_VAList):
+            case combineFAPK(Sema::FAPK_Fixed, Sema::FAPK_Fixed):
+            case combineFAPK(Sema::FAPK_Fixed, Sema::FAPK_Variadic):
+            case combineFAPK(Sema::FAPK_Variadic, Sema::FAPK_VAList):
+              return SLCT_UncheckedLiteral;
+            }
+          }
+        }
+      }
+    }
+  }
+  return SLCT_NotALiteral;
+}
+
+// Determine if an expression is a string literal or constant string.
+// If this function returns false on the arguments to a function expecting a
+// format string, we will usually need to emit a warning.
+// True string literals are then checked by CheckFormatString.
+static StringLiteralCheckType
+checkFormatStringExpr(Sema &S, const Expr *E, ArrayRef<const Expr *> Args,
+                      Sema::FormatArgumentPassingKind APK, unsigned format_idx,
+                      unsigned firstDataArg, Sema::FormatStringType Type,
+                      Sema::VariadicCallType CallType, bool NoNote,
+                      llvm::SmallBitVector &CheckedVarArgs,
+                      UncoveredArgHandler &UncoveredArg, llvm::APSInt Offset,
+                      bool IgnoreStringsWithoutSpecifiers = false) {
+  auto T = checkFormatStringExprEvaluated(
+      S, E, Args, APK, format_idx, firstDataArg, Type, CallType, NoNote,
+      CheckedVarArgs, UncoveredArg, Offset, IgnoreStringsWithoutSpecifiers);
+  if (T == SLCT_NotALiteral) {
     Expr::EvalResult Result;
-  if (E->EvaluateAsRValue(Result, Context) && Result.Val.isLValue()) {
+    if (E->EvaluateAsRValue(Result, S.Context) && Result.Val.isLValue()) {
+      if (isa<CallExpr>(E) || isa<CXXMemberCallExpr>(E)) {
         const auto *LVE = Result.Val.getLValueBase().dyn_cast<const Expr *>();
-    if (isa_and_nonnull<StringLiteral>(LVE))
-      return LVE;
+
+        if (LVE)
+          return checkFormatStringExprEvaluated(
+              S, LVE, Args, APK, format_idx, firstDataArg, Type, CallType,
+              /*NoNote*/ false, CheckedVarArgs, UncoveredArg, Offset,
+              IgnoreStringsWithoutSpecifiers);
       }
-  return nullptr;
+
+      const auto *LVVD =
+          Result.Val.getLValueBase().dyn_cast<const ValueDecl *>();
+      if (isa_and_nonnull<const VarDecl>(LVVD))
+        return checkVarDecl(
+            S, E, Args, APK, format_idx, firstDataArg, Type, CallType, NoNote,
+            CheckedVarArgs, UncoveredArg, Offset, LVVD->getType(),
+            dyn_cast<const VarDecl>(LVVD), IgnoreStringsWithoutSpecifiers);
+    }
+  }
+  return T;
 }
 
 Sema::FormatStringType Sema::GetFormatStringType(const FormatAttr *Format) {
   return llvm::StringSwitch<FormatStringType>(Format->getType()->getName())
       .Case("scanf", FST_Scanf)
       .Cases("printf", "printf0", FST_Printf)
       .Cases("NSString", "CFString", FST_NSString)
       .Case("strftime", FST_Strftime)
protected:
   const unsigned NumDataArgs;
   const char *Beg; // Start of format string.
   const Sema::FormatArgumentPassingKind ArgPassingKind;
   ArrayRef<const Expr *> Args;
   unsigned FormatIdx;
   llvm::SmallBitVector CoveredArgs;
   bool usesPositionalArgs = false;
   bool atFirstArg = true;
-  bool inFunctionCall;
+  bool NoNote;
   Sema::VariadicCallType CallType;
   llvm::SmallBitVector &CheckedVarArgs;
   UncoveredArgHandler &UncoveredArg;
 
 public:
   CheckFormatHandler(Sema &s, const FormatStringLiteral *fexpr,
                      const Expr *origFormatExpr,
                      const Sema::FormatStringType type, unsigned firstDataArg,
                      unsigned numDataArgs, const char *beg,
                      Sema::FormatArgumentPassingKind APK,
                      ArrayRef<const Expr *> Args, unsigned formatIdx,
-                     bool inFunctionCall, Sema::VariadicCallType callType,
+                     bool NoNote, Sema::VariadicCallType callType,
                      llvm::SmallBitVector &CheckedVarArgs,
                      UncoveredArgHandler &UncoveredArg)
       : S(s), FExpr(fexpr), OrigFormatExpr(origFormatExpr), FSType(type),
         FirstDataArg(firstDataArg), NumDataArgs(numDataArgs), Beg(beg),
-        ArgPassingKind(APK), Args(Args), FormatIdx(formatIdx),
-        inFunctionCall(inFunctionCall), CallType(callType),
-        CheckedVarArgs(CheckedVarArgs), UncoveredArg(UncoveredArg) {
+        ArgPassingKind(APK), Args(Args), FormatIdx(formatIdx), NoNote(NoNote),
+        CallType(callType), CheckedVarArgs(CheckedVarArgs),
+        UncoveredArg(UncoveredArg) {
     CoveredArgs.resize(numDataArgs);
     CoveredArgs.reset();
   }
 
   void DoneProcessing();
 
   void HandleIncompleteSpecifier(const char *startSpecifier,
                                  unsigned specifierLen) override;
void HandleInvalidPosition(const char *startSpecifier,
                              analyze_format_string::PositionContext p) override;
 
   void HandleZeroPosition(const char *startPos, unsigned posLen) override;
 
   void HandleNullChar(const char *nullCharacter) override;
 
   template <typename Range>
   static void
-  EmitFormatDiagnostic(Sema &S, bool inFunctionCall, const Expr *ArgumentExpr,
+  EmitFormatDiagnostic(Sema &S, bool NoNote, const Expr *ArgumentExpr,
                        const PartialDiagnostic &PDiag, SourceLocation StringLoc,
                        bool IsStringLocation, Range StringRange,
                        ArrayRef<FixItHint> Fixit = None);
 
 protected:
   bool HandleInvalidConversionSpecifier(unsigned argIndex, SourceLocation Loc,
                                         const char *startSpec,
                                         unsigned specifierLen,

 }
 
 template<typename Range>
 void CheckFormatHandler::EmitFormatDiagnostic(PartialDiagnostic PDiag,
                                               SourceLocation Loc,
                                               bool IsStringLocation,
                                               Range StringRange,
                                               ArrayRef<FixItHint> FixIt) {
-  EmitFormatDiagnostic(S, inFunctionCall, Args[FormatIdx], PDiag,
-                       Loc, IsStringLocation, StringRange, FixIt);
+  EmitFormatDiagnostic(S, NoNote, Args[FormatIdx], PDiag, Loc, IsStringLocation,
+                       StringRange, FixIt);
 }
 
 /// If the format string is not within the function call, emit a note
 /// so that the function call and string are in diagnostic messages.
 ///
-/// \param InFunctionCall if true, the format string is within the function
-/// call and only one diagnostic message will be produced.  Otherwise, an
-/// extra note will be emitted pointing to location of the format string.
+/// \param NoNote if true, only one diagnostic message will be produced. Perhaps
+/// the format string is within the function call or is evaluted which does not
+/// have an appropriate source location. Otherwise, an extra note will be
+/// emitted pointing to location of the format string.
 ///
 /// \param ArgumentExpr the expression that is passed as the format string
 /// argument in the function call.  Used for getting locations when two
 /// diagnostics are emitted.
 ///
 /// \param PDiag the callee should already have provided any strings for the
 /// diagnostic message.  This function only adds locations and fixits
 /// to diagnostics.
 ///
 /// \param Loc primary location for diagnostic.  If two diagnostics are
 /// required, one will be at Loc and a new SourceLocation will be created for
 /// the other one.
 ///
 /// \param IsStringLocation if true, Loc points to the format string should be
 /// used for the note.  Otherwise, Loc points to the argument list and will
 /// be used with PDiag.
 ///
 /// \param StringRange some or all of the string to highlight.  This is
 /// templated so it can accept either a CharSourceRange or a SourceRange.
 ///
 /// \param FixIt optional fix it hint for the format string.
 template <typename Range>
 void CheckFormatHandler::EmitFormatDiagnostic(
-    Sema &S, bool InFunctionCall, const Expr *ArgumentExpr,
+    Sema &S, bool NoNote, const Expr *ArgumentExpr,
     const PartialDiagnostic &PDiag, SourceLocation Loc, bool IsStringLocation,
     Range StringRange, ArrayRef<FixItHint> FixIt) {
-  if (InFunctionCall) {
+  if (NoNote) {
     const Sema::SemaDiagnosticBuilder &D = S.Diag(Loc, PDiag);
     D << StringRange;
     D << FixIt;
   } else {
     S.Diag(IsStringLocation ? ArgumentExpr->getExprLoc() : Loc, PDiag)
       << ArgumentExpr->getSourceRange();
 
     const Sema::SemaDiagnosticBuilder &Note =

 class CheckPrintfHandler : public CheckFormatHandler {
 public:
   CheckPrintfHandler(Sema &s, const FormatStringLiteral *fexpr,
                      const Expr *origFormatExpr,
                      const Sema::FormatStringType type, unsigned firstDataArg,
                      unsigned numDataArgs, bool isObjC, const char *beg,
                      Sema::FormatArgumentPassingKind APK,
                      ArrayRef<const Expr *> Args, unsigned formatIdx,
-                     bool inFunctionCall, Sema::VariadicCallType CallType,
+                     bool NoNote, Sema::VariadicCallType CallType,
                      llvm::SmallBitVector &CheckedVarArgs,
                      UncoveredArgHandler &UncoveredArg)
       : CheckFormatHandler(s, fexpr, origFormatExpr, type, firstDataArg,
-                           numDataArgs, beg, APK, Args, formatIdx,
-                           inFunctionCall, CallType, CheckedVarArgs,
-                           UncoveredArg) {}
+                           numDataArgs, beg, APK, Args, formatIdx, NoNote,
+                           CallType, CheckedVarArgs, UncoveredArg) {}
 
   bool isObjCContext() const { return FSType == Sema::FST_NSString; }
 
   /// Returns true if '%@' specifiers are allowed in the format string.
   bool allowsObjCArg() const {
     return FSType == Sema::FST_NSString || FSType == Sema::FST_OSLog ||
            FSType == Sema::FST_OSTrace;
   }

 
 class CheckScanfHandler : public CheckFormatHandler {
 public:
   CheckScanfHandler(Sema &s, const FormatStringLiteral *fexpr,
                     const Expr *origFormatExpr, Sema::FormatStringType type,
                     unsigned firstDataArg, unsigned numDataArgs,
                     const char *beg, Sema::FormatArgumentPassingKind APK,
                     ArrayRef<const Expr *> Args, unsigned formatIdx,
-                    bool inFunctionCall, Sema::VariadicCallType CallType,
+                    bool NoNote, Sema::VariadicCallType CallType,
                     llvm::SmallBitVector &CheckedVarArgs,
                     UncoveredArgHandler &UncoveredArg)
       : CheckFormatHandler(s, fexpr, origFormatExpr, type, firstDataArg,
-                           numDataArgs, beg, APK, Args, formatIdx,
-                           inFunctionCall, CallType, CheckedVarArgs,
-                           UncoveredArg) {}
+                           numDataArgs, beg, APK, Args, formatIdx, NoNote,
+                           CallType, CheckedVarArgs, UncoveredArg) {}
 
   bool HandleScanfSpecifier(const analyze_scanf::ScanfSpecifier &FS,
                             const char *startSpecifier,
                             unsigned specifierLen) override;
 
   bool HandleInvalidScanfConversionSpecifier(
           const analyze_scanf::ScanfSpecifier &FS,
           const char *startSpecifier,
bool CheckScanfHandler::HandleScanfSpecifier(
 
   return true;
 }
 
 static void CheckFormatString(
     Sema &S, const FormatStringLiteral *FExpr, const Expr *OrigFormatExpr,
     ArrayRef<const Expr *> Args, Sema::FormatArgumentPassingKind APK,
     unsigned format_idx, unsigned firstDataArg, Sema::FormatStringType Type,
-    bool inFunctionCall, Sema::VariadicCallType CallType,
+    bool NoNote, Sema::VariadicCallType CallType,
     llvm::SmallBitVector &CheckedVarArgs, UncoveredArgHandler &UncoveredArg,
     bool IgnoreStringsWithoutSpecifiers) {
   // CHECK: is the format string a wide literal?
   if (!FExpr->isAscii() && !FExpr->isUTF8()) {
     CheckFormatHandler::EmitFormatDiagnostic(
-        S, inFunctionCall, Args[format_idx],
+        S, NoNote, Args[format_idx],
         S.PDiag(diag::warn_format_string_is_wide_literal), FExpr->getBeginLoc(),
         /*IsStringLocation*/ true, OrigFormatExpr->getSourceRange());
     return;
   }
 
   // Str - The format string.  NOTE: this is NOT null-terminated!
   StringRef StrRef = FExpr->getString();
   const char *Str = StrRef.data();
if (IgnoreStringsWithoutSpecifiers &&
       !analyze_format_string::parseFormatStringHasFormattingSpecifiers(
           Str, Str + StrLen, S.getLangOpts(), S.Context.getTargetInfo()))
     return;
 
   // Emit a warning if the string literal is truncated and does not contain an
   // embedded null character.
   if (TypeSize <= StrRef.size() && !StrRef.substr(0, TypeSize).contains('\0')) {
     CheckFormatHandler::EmitFormatDiagnostic(
-        S, inFunctionCall, Args[format_idx],
+        S, NoNote, Args[format_idx],
         S.PDiag(diag::warn_printf_format_string_not_null_terminated),
         FExpr->getBeginLoc(),
         /*IsStringLocation=*/true, OrigFormatExpr->getSourceRange());
     return;
   }
 
   // CHECK: empty format string?
   if (StrLen == 0 && numDataArgs > 0) {
     CheckFormatHandler::EmitFormatDiagnostic(
-        S, inFunctionCall, Args[format_idx],
-        S.PDiag(diag::warn_empty_format_string), FExpr->getBeginLoc(),
+        S, NoNote, Args[format_idx], S.PDiag(diag::warn_empty_format_string),
+        FExpr->getBeginLoc(),
         /*IsStringLocation*/ true, OrigFormatExpr->getSourceRange());
     return;
   }
 
   if (Type == Sema::FST_Printf || Type == Sema::FST_NSString ||
       Type == Sema::FST_FreeBSDKPrintf || Type == Sema::FST_OSLog ||
       Type == Sema::FST_OSTrace) {
     CheckPrintfHandler H(
         S, FExpr, OrigFormatExpr, Type, firstDataArg, numDataArgs,
         (Type == Sema::FST_NSString || Type == Sema::FST_OSTrace), Str, APK,
-        Args, format_idx, inFunctionCall, CallType, CheckedVarArgs,
-        UncoveredArg);
+        Args, format_idx, NoNote, CallType, CheckedVarArgs, UncoveredArg);
 
     if (!analyze_format_string::ParsePrintfString(
             H, Str, Str + StrLen, S.getLangOpts(), S.Context.getTargetInfo(),
             Type == Sema::FST_FreeBSDKPrintf))
       H.DoneProcessing();
   } else if (Type == Sema::FST_Scanf) {
     CheckScanfHandler H(S, FExpr, OrigFormatExpr, Type, firstDataArg,
-                        numDataArgs, Str, APK, Args, format_idx, inFunctionCall,
+                        numDataArgs, Str, APK, Args, format_idx, NoNote,
                         CallType, CheckedVarArgs, UncoveredArg);
 
     if (!analyze_format_string::ParseScanfString(
             H, Str, Str + StrLen, S.getLangOpts(), S.Context.getTargetInfo()))
       H.DoneProcessing();
   } // TODO: handle other formats
 }
 

clang/test/SemaCXX/format-strings.cpp

void f() {
   t1.func2("Hello %s"); // expected-warning {{more '%' conversions than data arguments}}
   t::func3("Hello %s"); // expected-warning {{more '%' conversions than data arguments}}
   t::func4("Hello %s"); // expected-warning {{more '%' conversions than data arguments}}
 }
 }
 #if __cplusplus >= 201103L
 namespace evaluated {
 
+struct InitList {
+  static constexpr char value[] = {'%', 's', '%', 'd', '\0'}; // no note here because this is not literal
+};
+
+constexpr const char *init_list_func() { return InitList::value; }
+
+constexpr const char *init_list_func_wrap() { return init_list_func(); }
+
 constexpr const char *basic() { 
   return 
 "%s %d"; // expected-note {{format string is defined here}}
 }
 
 constexpr const char *correct_fmt() { 
   return 
 "%d %d";

 
 constexpr const char *wrap_constexpr() {
   return inner_call();
 }
 
 
 void f() {
   printf(basic(), 1, 2); // expected-warning {{format specifies type 'char *' but the argument has type 'int'}}
-  printf(correct_fmt(), 1, 2);
+  printf(correct_fmt(), 1, 2); // no warning
   printf(string_linebreak(), 1, 2, 3, 4); // expected-warning {{format specifies type 'char *' but the argument has type 'int'}}
   printf(not_literal(), 1, 2, 3, 4); // expected-warning {{format string is not a string literal}}
   printf(wrap_constexpr(), 1, 2); // expected-warning {{format specifies type 'char *' but the argument has type 'int'}}
+  printf(InitList::value, 1, 2); // expected-warning {{format specifies type 'char *' but the argument has type 'int'}}
+  printf(init_list_func(), 1, 2); // expected-warning {{format specifies type 'char *' but the argument has type 'int'}}
+  printf(init_list_func_wrap(), 1, 2); // expected-warning {{format specifies type 'char *' but the argument has type 'int'}}
 }
 
 
-}
+} // namespace evaluated
 #endif