aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--include/clang/Basic/DiagnosticSemaKinds.td4
-rw-r--r--lib/Sema/SemaChecking.cpp149
-rw-r--r--test/FixIt/format-darwin.m181
3 files changed, 311 insertions, 23 deletions
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index e0dac62403..b10e8cb008 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5410,6 +5410,10 @@ def warn_scanf_nonzero_width : Warning<
def warn_printf_conversion_argument_type_mismatch : Warning<
"format specifies type %0 but the argument has type %1">,
InGroup<Format>;
+def warn_format_argument_needs_cast : Warning<
+ "values of type '%0' should not be used as format arguments; add an explicit "
+ "cast to %1 instead">,
+ InGroup<Format>;
def warn_printf_positional_arg_exceeds_data_args : Warning <
"data argument position '%0' exceeds the number of data arguments (%1)">,
InGroup<Format>;
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index bd6f047c3f..5d54ca361d 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -1967,7 +1967,7 @@ public:
PartialDiagnostic PDiag,
SourceLocation StringLoc,
bool IsStringLocation, Range StringRange,
- FixItHint Fixit = FixItHint());
+ ArrayRef<FixItHint> Fixit = ArrayRef<FixItHint>());
protected:
bool HandleInvalidConversionSpecifier(unsigned argIndex, SourceLocation Loc,
@@ -1994,7 +1994,7 @@ protected:
template <typename Range>
void EmitFormatDiagnostic(PartialDiagnostic PDiag, SourceLocation StringLoc,
bool IsStringLocation, Range StringRange,
- FixItHint Fixit = FixItHint());
+ ArrayRef<FixItHint> Fixit = ArrayRef<FixItHint>());
void CheckPositionalAndNonpositionalArgs(
const analyze_format_string::FormatSpecifier *FS);
@@ -2185,7 +2185,7 @@ void CheckFormatHandler::EmitFormatDiagnostic(PartialDiagnostic PDiag,
SourceLocation Loc,
bool IsStringLocation,
Range StringRange,
- FixItHint FixIt) {
+ ArrayRef<FixItHint> FixIt) {
EmitFormatDiagnostic(S, inFunctionCall, Args[FormatIdx], PDiag,
Loc, IsStringLocation, StringRange, FixIt);
}
@@ -2224,15 +2224,27 @@ void CheckFormatHandler::EmitFormatDiagnostic(Sema &S, bool InFunctionCall,
SourceLocation Loc,
bool IsStringLocation,
Range StringRange,
- FixItHint FixIt) {
- if (InFunctionCall)
- S.Diag(Loc, PDiag) << StringRange << FixIt;
- else {
+ ArrayRef<FixItHint> FixIt) {
+ if (InFunctionCall) {
+ const Sema::SemaDiagnosticBuilder &D = S.Diag(Loc, PDiag);
+ D << StringRange;
+ for (ArrayRef<FixItHint>::iterator I = FixIt.begin(), E = FixIt.end();
+ I != E; ++I) {
+ D << *I;
+ }
+ } else {
S.Diag(IsStringLocation ? ArgumentExpr->getExprLoc() : Loc, PDiag)
<< ArgumentExpr->getSourceRange();
- S.Diag(IsStringLocation ? Loc : StringRange.getBegin(),
- diag::note_format_string_defined)
- << StringRange << FixIt;
+
+ const Sema::SemaDiagnosticBuilder &Note =
+ S.Diag(IsStringLocation ? Loc : StringRange.getBegin(),
+ diag::note_format_string_defined);
+
+ Note << StringRange;
+ for (ArrayRef<FixItHint>::iterator I = FixIt.begin(), E = FixIt.end();
+ I != E; ++I) {
+ Note << *I;
+ }
}
}
@@ -2585,6 +2597,30 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier
return checkFormatExpr(FS, startSpecifier, specifierLen, Arg);
}
+static bool requiresParensToAddCast(const Expr *E) {
+ // FIXME: We should have a general way to reason about operator
+ // precedence and whether parens are actually needed here.
+ // Take care of a few common cases where they aren't.
+ const Expr *Inside = E->IgnoreImpCasts();
+ if (const PseudoObjectExpr *POE = dyn_cast<PseudoObjectExpr>(Inside))
+ Inside = POE->getSyntacticForm()->IgnoreImpCasts();
+
+ switch (Inside->getStmtClass()) {
+ case Stmt::ArraySubscriptExprClass:
+ case Stmt::CallExprClass:
+ case Stmt::DeclRefExprClass:
+ case Stmt::MemberExprClass:
+ case Stmt::ObjCIvarRefExprClass:
+ case Stmt::ObjCMessageExprClass:
+ case Stmt::ObjCPropertyRefExprClass:
+ case Stmt::ParenExprClass:
+ case Stmt::UnaryOperatorClass:
+ return false;
+ default:
+ return true;
+ }
+}
+
bool
CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
const char *StartSpecifier,
@@ -2598,7 +2634,9 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
ObjCContext);
if (!AT.isValid())
return true;
- if (AT.matchesType(S.Context, E->getType()))
+
+ QualType IntendedTy = E->getType();
+ if (AT.matchesType(S.Context, IntendedTy))
return true;
// Look through argument promotions for our error message's reported type.
@@ -2609,6 +2647,7 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
if (ICE->getCastKind() == CK_IntegralCast ||
ICE->getCastKind() == CK_FloatingCast) {
E = ICE->getSubExpr();
+ IntendedTy = E->getType();
// Check if we didn't match because of an implicit cast from a 'char'
// or 'short' to an 'int'. This is done because printf is a varargs
@@ -2616,15 +2655,28 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
if (ICE->getType() == S.Context.IntTy ||
ICE->getType() == S.Context.UnsignedIntTy) {
// All further checking is done on the subexpression.
- if (AT.matchesType(S.Context, E->getType()))
+ if (AT.matchesType(S.Context, IntendedTy))
return true;
}
}
}
+ if (S.Context.getTargetInfo().getTriple().isOSDarwin()) {
+ // Special-case some of Darwin's platform-independence types.
+ if (const TypedefType *UserTy = IntendedTy->getAs<TypedefType>()) {
+ StringRef Name = UserTy->getDecl()->getName();
+ IntendedTy = llvm::StringSwitch<QualType>(Name)
+ .Case("NSInteger", S.Context.LongTy)
+ .Case("NSUInteger", S.Context.UnsignedLongTy)
+ .Case("SInt32", S.Context.IntTy)
+ .Case("UInt32", S.Context.UnsignedIntTy)
+ .Default(IntendedTy);
+ }
+ }
+
// We may be able to offer a FixItHint if it is a supported type.
PrintfSpecifier fixedFS = FS;
- bool success = fixedFS.fixType(E->getType(), S.getLangOpts(),
+ bool success = fixedFS.fixType(IntendedTy, S.getLangOpts(),
S.Context, ObjCContext);
if (success) {
@@ -2633,16 +2685,67 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
llvm::raw_svector_ostream os(buf);
fixedFS.toString(os);
- EmitFormatDiagnostic(
- S.PDiag(diag::warn_printf_conversion_argument_type_mismatch)
- << AT.getRepresentativeTypeName(S.Context) << E->getType()
- << E->getSourceRange(),
- E->getLocStart(),
- /*IsStringLocation*/false,
- getSpecifierRange(StartSpecifier, SpecifierLen),
- FixItHint::CreateReplacement(
- getSpecifierRange(StartSpecifier, SpecifierLen),
- os.str()));
+ CharSourceRange SpecRange = getSpecifierRange(StartSpecifier, SpecifierLen);
+
+ if (IntendedTy != E->getType()) {
+ // The canonical type for formatting this value is different from the
+ // actual type of the expression. (This occurs, for example, with Darwin's
+ // NSInteger on 32-bit platforms, where it is typedef'd as 'int', but
+ // should be printed as 'long' for 64-bit compatibility.)
+ // Rather than emitting a normal format/argument mismatch, we want to
+ // add a cast to the recommended type (and correct the format string
+ // if necessary).
+ SmallString<16> CastBuf;
+ llvm::raw_svector_ostream CastFix(CastBuf);
+ CastFix << "(";
+ IntendedTy.print(CastFix, S.Context.getPrintingPolicy());
+ CastFix << ")";
+
+ SmallVector<FixItHint,4> Hints;
+ if (!AT.matchesType(S.Context, IntendedTy))
+ Hints.push_back(FixItHint::CreateReplacement(SpecRange, os.str()));
+
+ if (const CStyleCastExpr *CCast = dyn_cast<CStyleCastExpr>(E)) {
+ // If there's already a cast present, just replace it.
+ SourceRange CastRange(CCast->getLParenLoc(), CCast->getRParenLoc());
+ Hints.push_back(FixItHint::CreateReplacement(CastRange, CastFix.str()));
+
+ } else if (!requiresParensToAddCast(E)) {
+ // If the expression has high enough precedence,
+ // just write the C-style cast.
+ Hints.push_back(FixItHint::CreateInsertion(E->getLocStart(),
+ CastFix.str()));
+ } else {
+ // Otherwise, add parens around the expression as well as the cast.
+ CastFix << "(";
+ Hints.push_back(FixItHint::CreateInsertion(E->getLocStart(),
+ CastFix.str()));
+
+ SourceLocation After = S.PP.getLocForEndOfToken(E->getLocEnd());
+ Hints.push_back(FixItHint::CreateInsertion(After, ")"));
+ }
+
+ // We extract the name from the typedef because we don't want to show
+ // the underlying type in the diagnostic.
+ const TypedefType *UserTy = cast<TypedefType>(E->getType());
+ StringRef Name = UserTy->getDecl()->getName();
+
+ // Finally, emit the diagnostic.
+ EmitFormatDiagnostic(S.PDiag(diag::warn_format_argument_needs_cast)
+ << Name << IntendedTy
+ << E->getSourceRange(),
+ E->getLocStart(), /*IsStringLocation=*/false,
+ SpecRange, Hints);
+ } else {
+ EmitFormatDiagnostic(
+ S.PDiag(diag::warn_printf_conversion_argument_type_mismatch)
+ << AT.getRepresentativeTypeName(S.Context) << IntendedTy
+ << E->getSourceRange(),
+ E->getLocStart(),
+ /*IsStringLocation*/false,
+ SpecRange,
+ FixItHint::CreateReplacement(SpecRange, os.str()));
+ }
} else {
const CharSourceRange &CSR = getSpecifierRange(StartSpecifier,
SpecifierLen);
diff --git a/test/FixIt/format-darwin.m b/test/FixIt/format-darwin.m
new file mode 100644
index 0000000000..a006d4a8c7
--- /dev/null
+++ b/test/FixIt/format-darwin.m
@@ -0,0 +1,181 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -fsyntax-only -fblocks -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fsyntax-only -fblocks -verify %s
+
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -fdiagnostics-parseable-fixits -fblocks %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fdiagnostics-parseable-fixits -fblocks %s 2>&1 | FileCheck %s
+
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -fdiagnostics-parseable-fixits -fblocks %s 2>&1 | FileCheck -check-prefix=CHECK-32 %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fdiagnostics-parseable-fixits -fblocks %s 2>&1 | FileCheck -check-prefix=CHECK-64 %s
+
+int printf(const char * restrict, ...);
+
+#if __LP64__
+typedef long NSInteger;
+typedef unsigned long NSUInteger;
+typedef int SInt32;
+typedef unsigned int UInt32;
+
+#else
+
+typedef int NSInteger;
+typedef unsigned int NSUInteger;
+typedef long SInt32;
+typedef unsigned long UInt32;
+#endif
+
+NSInteger getNSInteger();
+NSUInteger getNSUInteger();
+SInt32 getSInt32();
+UInt32 getUInt32();
+
+void testCorrectionInAllCases() {
+ printf("%s", getNSInteger()); // expected-warning{{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+ printf("%s", getNSUInteger()); // expected-warning{{values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead}}
+ printf("%s", getSInt32()); // expected-warning{{values of type 'SInt32' should not be used as format arguments; add an explicit cast to 'int' instead}}
+ printf("%s", getUInt32()); // expected-warning{{values of type 'UInt32' should not be used as format arguments; add an explicit cast to 'unsigned int' instead}}
+
+ // CHECK: fix-it:"{{.*}}":{32:11-32:13}:"%ld"
+ // CHECK: fix-it:"{{.*}}":{32:16-32:16}:"(long)"
+
+ // CHECK: fix-it:"{{.*}}":{33:11-33:13}:"%lu"
+ // CHECK: fix-it:"{{.*}}":{33:16-33:16}:"(unsigned long)"
+
+ // CHECK: fix-it:"{{.*}}":{34:11-34:13}:"%d"
+ // CHECK: fix-it:"{{.*}}":{34:16-34:16}:"(int)"
+
+ // CHECK: fix-it:"{{.*}}":{35:11-35:13}:"%u"
+ // CHECK: fix-it:"{{.*}}":{35:16-35:16}:"(unsigned int)"
+}
+
+@interface Foo {
+@public
+ NSInteger _value;
+}
+- (NSInteger)getInteger;
+
+@property NSInteger value;
+@end
+
+struct Bar {
+ NSInteger value;
+};
+
+
+void testParens(Foo *obj, struct Bar *record) {
+ NSInteger arr[4] = {0};
+ NSInteger i = 0;
+
+ // These cases match the cases in CheckPrintfHandler::checkFormatExpr.
+ printf("%s", arr[0]); // expected-warning{{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+ printf("%s", getNSInteger()); // expected-warning{{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+ printf("%s", i); // expected-warning{{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+ printf("%s", obj->_value); // expected-warning{{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+ printf("%s", [obj getInteger]); // expected-warning{{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+ printf("%s", obj.value); // expected-warning{{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+ printf("%s", record->value); // expected-warning{{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+ printf("%s", (i ? i : i)); // expected-warning{{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+ printf("%s", *arr); // expected-warning{{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+
+ // CHECK-NOT: fix-it:{{.*}}:")"
+
+ printf("%s", i ? i : i); // expected-warning{{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+
+ // CHECK: fix-it:"{{.*}}":{81:11-81:13}:"%ld"
+ // CHECK: fix-it:"{{.*}}":{81:16-81:16}:"(long)("
+ // CHECK: fix-it:"{{.*}}":{81:25-81:25}:")"
+}
+
+
+#if __LP64__
+
+void testWarn() {
+ printf("%d", getNSInteger()); // expected-warning{{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+ printf("%u", getNSUInteger()); // expected-warning{{values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead}}
+ printf("%ld", getSInt32()); // expected-warning{{values of type 'SInt32' should not be used as format arguments; add an explicit cast to 'int' instead}}
+ printf("%lu", getUInt32()); // expected-warning{{values of type 'UInt32' should not be used as format arguments; add an explicit cast to 'unsigned int' instead}}
+
+ // CHECK-64: fix-it:"{{.*}}":{92:11-92:13}:"%ld"
+ // CHECK-64: fix-it:"{{.*}}":{92:16-92:16}:"(long)"
+
+ // CHECK-64: fix-it:"{{.*}}":{93:11-93:13}:"%lu"
+ // CHECK-64: fix-it:"{{.*}}":{93:16-93:16}:"(unsigned long)"
+
+ // CHECK-64: fix-it:"{{.*}}":{94:11-94:14}:"%d"
+ // CHECK-64: fix-it:"{{.*}}":{94:17-94:17}:"(int)"
+
+ // CHECK-64: fix-it:"{{.*}}":{95:11-95:14}:"%u"
+ // CHECK-64: fix-it:"{{.*}}":{95:17-95:17}:"(unsigned int)"
+}
+
+void testPreserveHex() {
+ printf("%x", getNSInteger()); // expected-warning{{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+ printf("%x", getNSUInteger()); // expected-warning{{values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead}}
+
+ // CHECK-64: fix-it:"{{.*}}":{111:11-111:13}:"%lx"
+ // CHECK-64: fix-it:"{{.*}}":{111:16-111:16}:"(long)"
+
+ // CHECK-64: fix-it:"{{.*}}":{112:11-112:13}:"%lx"
+ // CHECK-64: fix-it:"{{.*}}":{112:16-112:16}:"(unsigned long)"
+}
+
+void testNoWarn() {
+ printf("%ld", getNSInteger()); // no-warning
+ printf("%lu", getNSUInteger()); // no-warning
+ printf("%d", getSInt32()); // no-warning
+ printf("%u", getUInt32()); // no-warning
+}
+
+#else
+
+void testWarn() {
+ printf("%ld", getNSInteger()); // expected-warning{{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+ printf("%lu", getNSUInteger()); // expected-warning{{values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead}}
+ printf("%d", getSInt32()); // expected-warning{{values of type 'SInt32' should not be used as format arguments; add an explicit cast to 'int' instead}}
+ printf("%u", getUInt32()); // expected-warning{{values of type 'UInt32' should not be used as format arguments; add an explicit cast to 'unsigned int' instead}}
+
+ // CHECK-32: fix-it:"{{.*}}":{131:17-131:17}:"(long)"
+
+ // CHECK-32: fix-it:"{{.*}}":{132:17-132:17}:"(unsigned long)"
+
+ // CHECK-32: fix-it:"{{.*}}":{133:16-133:16}:"(int)"
+
+ // CHECK-32: fix-it:"{{.*}}":{134:16-134:16}:"(unsigned int)"
+}
+
+void testPreserveHex() {
+ printf("%lx", getNSInteger()); // expected-warning{{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+ printf("%lx", getNSUInteger()); // expected-warning{{values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead}}
+
+ // CHECK-32: fix-it:"{{.*}}":{146:17-146:17}:"(long)"
+
+ // CHECK-32: fix-it:"{{.*}}":{147:17-147:17}:"(unsigned long)"
+}
+
+void testNoWarn() {
+ printf("%d", getNSInteger()); // no-warning
+ printf("%u", getNSUInteger()); // no-warning
+ printf("%ld", getSInt32()); // no-warning
+ printf("%lu", getUInt32()); // no-warning
+}
+
+#endif
+
+
+void testCasts() {
+ printf("%s", (NSInteger)0); // expected-warning{{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+ printf("%s", (NSUInteger)0); // expected-warning{{values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead}}
+ printf("%s", (SInt32)0); // expected-warning{{values of type 'SInt32' should not be used as format arguments; add an explicit cast to 'int' instead}}
+ printf("%s", (UInt32)0); // expected-warning{{values of type 'UInt32' should not be used as format arguments; add an explicit cast to 'unsigned int' instead}}
+
+ // CHECK: fix-it:"{{.*}}":{165:11-165:13}:"%ld"
+ // CHECK: fix-it:"{{.*}}":{165:16-165:27}:"(long)"
+
+ // CHECK: fix-it:"{{.*}}":{166:11-166:13}:"%lu"
+ // CHECK: fix-it:"{{.*}}":{166:16-166:28}:"(unsigned long)"
+
+ // CHECK: fix-it:"{{.*}}":{167:11-167:13}:"%d"
+ // CHECK: fix-it:"{{.*}}":{167:16-167:24}:"(int)"
+
+ // CHECK: fix-it:"{{.*}}":{168:11-168:13}:"%u"
+ // CHECK: fix-it:"{{.*}}":{168:16-168:24}:"(unsigned int)"
+}
generated by cgit v1.2.3-18-g5258 (git 2.32.0) at 2025-08-06 20:04:27 +0000