Skip to content

Commit e7ce50f

Browse files
[clang] Implement __attribute__((format_matches))
This implements ``__attribute__((format_matches))``, as described in the RFC: https://discourse.llvm.org/t/rfc-format-attribute-attribute-format-like/83076 The ``format`` attribute only allows the compiler to check that a format string matches its arguments. If the format string is passed independently of its arguments, there is no way to have the compiler check it. ``format_matches(flavor, fmtidx, example)`` allows the compiler to check format strings against the ``example`` format string instead of against format arguments. See the changes to AttrDocs.td in this diff for more information. Implementation-wise, this change subclasses CheckPrintfHandler and CheckScanfHandler to allow them to collect specifiers into arrays, and implements comparing that two specifiers are equivalent. Radar-Id: 84936554
1 parent 94d100f commit e7ce50f

File tree

12 files changed

+1127
-186
lines changed

12 files changed

+1127
-186
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,50 @@ Non-comprehensive list of changes in this release
357357

358358
- ``__builtin_reduce_add`` function can now be used in constant expressions.
359359

360+
- There is a new ``format_matches`` attribute to complement the existing
361+
``format`` attribute. ``format_matches`` allows the compiler to verify that
362+
a format string argument is equivalent to a reference format string: it is
363+
useful when a function accepts a format string without its accompanying
364+
arguments to format. For instance:
365+
366+
.. code-block:: c
367+
368+
static int status_code;
369+
static const char *status_string;
370+
371+
void print_status(const char *fmt) {
372+
fprintf(stderr, fmt, status_code, status_string);
373+
// ^ warning: format string is not a string literal [-Wformat-nonliteral]
374+
}
375+
376+
void stuff(void) {
377+
print_status("%s (%#08x)\n");
378+
// order of %s and %x is swapped but there is no diagnostic
379+
}
380+
381+
Before the introducion of ``format_matches``, this code cannot be verified
382+
at compile-time. ``format_matches`` plugs that hole:
383+
384+
.. code-block:: c
385+
386+
__attribute__((format_matches(printf, 1, "%x %s")))
387+
void print_status(const char *fmt) {
388+
fprintf(stderr, fmt, status_code, status_string);
389+
// ^ `fmt` verified as if it was "%x %s" here; no longer triggers
390+
// -Wformat-nonliteral, would warn if arguments did not match "%x %s"
391+
}
392+
393+
void stuff(void) {
394+
print_status("%s (%#08x)\n");
395+
// warning: format specifier 's' is incompatible with 'x'
396+
// warning: format specifier 'x' is incompatible with 's'
397+
}
398+
399+
Like with ``format``, the first argument is the format string flavor and the
400+
second argument is the index of the format string parameter.
401+
``format_matches`` accepts an example valid format string as its third
402+
argument. For more information, see the Clang attributes documentation.
403+
360404
New Compiler Flags
361405
------------------
362406

clang/include/clang/AST/FormatString.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ class ArgType {
292292
};
293293

294294
private:
295-
const Kind K;
295+
Kind K;
296296
QualType T;
297297
const char *Name = nullptr;
298298
bool Ptr = false;
@@ -338,6 +338,7 @@ class ArgType {
338338
}
339339

340340
MatchKind matchesType(ASTContext &C, QualType argTy) const;
341+
MatchKind matchesArgType(ASTContext &C, const ArgType &other) const;
341342

342343
QualType getRepresentativeType(ASTContext &C) const;
343344

clang/include/clang/Basic/Attr.td

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1807,6 +1807,16 @@ def Format : InheritableAttr {
18071807
let Documentation = [FormatDocs];
18081808
}
18091809

1810+
def FormatMatches : InheritableAttr {
1811+
let Spellings = [GCC<"format_matches">];
1812+
let Args = [IdentifierArgument<"Type">, IntArgument<"FormatIdx">, ExprArgument<"ExpectedFormat">];
1813+
let AdditionalMembers = [{
1814+
StringLiteral *getFormatString() const;
1815+
}];
1816+
let Subjects = SubjectList<[ObjCMethod, Block, HasFunctionProto]>;
1817+
let Documentation = [FormatMatchesDocs];
1818+
}
1819+
18101820
def FormatArg : InheritableAttr {
18111821
let Spellings = [GCC<"format_arg">];
18121822
let Args = [ParamIdxArgument<"FormatIdx">];

clang/include/clang/Basic/AttrDocs.td

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3738,6 +3738,103 @@ behavior of the program is undefined.
37383738
}];
37393739
}
37403740

3741+
def FormatMatchesDocs : Documentation {
3742+
let Category = DocCatFunction;
3743+
let Content = [{
3744+
3745+
The ``format`` attribute is the basis for the enforcement of diagnostics in the
3746+
``-Wformat`` family, but it only handles the case where the format string is
3747+
passed along with the arguments it is going to format. It cannot handle the case
3748+
where the format string and the format arguments are passed separately from each
3749+
other. For instance:
3750+
3751+
.. code-block:: c
3752+
3753+
static const char *first_name;
3754+
static double todays_temperature;
3755+
static int wind_speed;
3756+
3757+
void say_hi(const char *fmt) {
3758+
printf(fmt, first_name, todays_temperature); // warning: format string is not a string literal
3759+
printf(fmt, first_name, wind_speed); // warning: format string is not a string literal
3760+
}
3761+
3762+
int main() {
3763+
say_hi("hello %s, it is %g degrees outside");
3764+
say_hi("hello %s, it is %d degrees outside!"); // no diagnostic
3765+
}
3766+
3767+
In this example, ``fmt`` is expected to format a ``const char *`` and a
3768+
``double``, but these values are not passed to ``say_hi``. Without the
3769+
``format`` attribute (which cannot apply in this case), the -Wformat-nonliteral
3770+
diagnostic triggers in the body of ``say_hi``, and incorrect ``say_hi`` call
3771+
sites do not trigger a diagnostic.
3772+
3773+
To complement the ``format`` attribute, Clang also defines the
3774+
``format_matches`` attribute. Its syntax is similar to the ``format``
3775+
attribute's, but instead of taking the index of the first formatted value
3776+
argument, it takes a C string literal with the expected specifiers:
3777+
3778+
.. code-block:: c
3779+
3780+
static const char *first_name;
3781+
static double todays_temperature;
3782+
static int wind_speed;
3783+
3784+
__attribute__((__format_matches__(printf, 1, "%s %g")))
3785+
void say_hi(const char *fmt) {
3786+
printf(fmt, first_name, todays_temperature); // no dignostic
3787+
printf(fmt, first_name, wind_speed); // warning: format specifies type 'int' but the argument has type 'double'
3788+
}
3789+
3790+
int main() {
3791+
say_hi("hello %s, it is %g degrees outside");
3792+
say_hi("it is %g degrees outside, have a good day %s!");
3793+
// warning: format specifies 'double' where 'const char *' is required
3794+
// warning: format specifies 'const char *' where 'double' is required
3795+
}
3796+
3797+
The third argument to ``format_matches`` is expected to evaluate to a **C string
3798+
literal** even when the format string would normally be a different type for the
3799+
given flavor, like a ``CFStringRef`` or a ``NSString *``.
3800+
3801+
In the implementation of a function with the ``format_matches`` attribute,
3802+
format verification works as if the format string was identical to the one
3803+
specified in the attribute.
3804+
3805+
At the call sites of functions with the ``format_matches`` attribute, format
3806+
verification instead compares the two format strings to evaluate their
3807+
equivalence. Each format flavor defines equivalence between format specifiers.
3808+
Generally speaking, two specifiers are equivalent if they format the same type.
3809+
For instance, in the ``printf`` flavor, ``%2i`` and ``%-0.5d`` are compatible.
3810+
When ``-Wformat-signedness`` is disabled, ``%d`` and ``%u`` are compatible. For
3811+
a negative example, ``%ld`` is incompatible with ``%d``.
3812+
3813+
Do note the following un-obvious cases:
3814+
3815+
* Passing ``NULL`` as the format string is accepted without diagnostics.
3816+
* When the format string is not NULL, it cannot _miss_ specifiers, even in
3817+
trailing positions. For instance, ``%d`` is not accepted when the required
3818+
format is ``%d %d %d``.
3819+
* Specifiers for integers as small or smaller than ``int`` (such as ``%hhd``)
3820+
are all mutually compatible because standard argument promotion ensures that
3821+
integers are at least the size of ``int`` when passed as variadic arguments.
3822+
With ``-Wformat-signedness``, mixing specifier for types with a different
3823+
signedness still results in a diagnostic.
3824+
* Format strings expecting a variable modifier (such as ``%*s``) are
3825+
incompatible with format strings that would itemize the variable modifiers
3826+
(such as ``%i %s``), even if the two specify ABI-compatible argument lists.
3827+
* All pointer specifiers, modifiers aside, are mutually incompatible. For
3828+
instance, ``%s`` is not compatible with ``%p``, and ``%p`` is not compatible
3829+
with ``%n``, and ``%hhn`` is incompatible with ``%s``, even if the pointers
3830+
are ABI-compatible or identical on the selected platform. However, ``%0.5s``
3831+
is compatible with ``%s``, since the difference only exists in modifier flags.
3832+
This is not overridable with ``-Wformat-pedantic`` or its inverse, which
3833+
control similar behavior in ``-Wformat``.
3834+
3835+
}];
3836+
}
3837+
37413838
def FlagEnumDocs : Documentation {
37423839
let Category = DocCatDecl;
37433840
let Content = [{

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7687,6 +7687,7 @@ def warn_format_nonliteral_noargs : Warning<
76877687
def warn_format_nonliteral : Warning<
76887688
"format string is not a string literal">,
76897689
InGroup<FormatNonLiteral>, DefaultIgnore;
7690+
def err_format_nonliteral : Error<"format string is not a string literal">;
76907691

76917692
def err_unexpected_interface : Error<
76927693
"unexpected interface name %0: expected expression">;
@@ -9998,6 +9999,8 @@ def note_previous_declaration_as : Note<
99989999

999910000
def warn_printf_insufficient_data_args : Warning<
1000010001
"more '%%' conversions than data arguments">, InGroup<FormatInsufficientArgs>;
10002+
def warn_format_cmp_insufficient_specifiers : Warning<
10003+
"fewer specifiers in format string than expected">, InGroup<FormatInsufficientArgs>;
1000110004
def warn_printf_data_arg_not_used : Warning<
1000210005
"data argument not used by format string">, InGroup<FormatExtraArgs>;
1000310006
def warn_format_invalid_conversion : Warning<
@@ -10115,6 +10118,24 @@ def note_format_fix_specifier : Note<"did you mean to use '%0'?">;
1011510118
def note_printf_c_str: Note<"did you mean to call the %0 method?">;
1011610119
def note_format_security_fixit: Note<
1011710120
"treat the string as an argument to avoid this">;
10121+
def warn_format_cmp_role_mismatch : Warning<
10122+
"format argument is %select{a value|an indirect field width|an indirect "
10123+
"precision|an auxiliary value}0, but it should be %select{a value|an indirect "
10124+
"field width|an indirect precision|an auxiliary value}1">, InGroup<Format>;
10125+
def warn_format_cmp_modifierfor_mismatch : Warning<
10126+
"format argument modifies specifier at position %0, but it should modify "
10127+
"specifier at position %1">, InGroup<Format>;
10128+
def warn_format_cmp_sensitivity_mismatch : Warning<
10129+
"argument sensitivity is %select{unspecified|private|public|sensitive}0, but "
10130+
"it should be %select{unspecified|private|public|sensitive}1">, InGroup<Format>;
10131+
def warn_format_cmp_specifier_mismatch : Warning<
10132+
"format specifier '%0' is incompatible with '%1'">, InGroup<Format>;
10133+
def warn_format_cmp_specifier_sign_mismatch : Warning<
10134+
"signedness of format specifier '%0' is incompatible with '%1'">, InGroup<Format>;
10135+
def warn_format_cmp_specifier_mismatch_pedantic : Extension<
10136+
warn_format_cmp_specifier_sign_mismatch.Summary>, InGroup<FormatPedantic>;
10137+
def note_format_cmp_with : Note<
10138+
"comparing with this %select{specifier|format string}0">;
1011810139

1011910140
def warn_null_arg : Warning<
1012010141
"null passed to a callee that requires a non-null argument">,

clang/include/clang/Sema/Sema.h

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2150,6 +2150,7 @@ class Sema final : public SemaBase {
21502150
FAPK_Fixed, // values to format are fixed (no C-style variadic arguments)
21512151
FAPK_Variadic, // values to format are passed as variadic arguments
21522152
FAPK_VAList, // values to format are passed in a va_list
2153+
FAPK_Elsewhere, // values to format are not passed to this function
21532154
};
21542155

21552156
// Used to grab the relevant information from a FormatAttr and a
@@ -2160,12 +2161,15 @@ class Sema final : public SemaBase {
21602161
FormatArgumentPassingKind ArgPassingKind;
21612162
};
21622163

2163-
/// Given a FunctionDecl's FormatAttr, attempts to populate the
2164-
/// FomatStringInfo parameter with the FormatAttr's correct format_idx and
2165-
/// firstDataArg. Returns true when the format fits the function and the
2166-
/// FormatStringInfo has been populated.
2167-
static bool getFormatStringInfo(const FormatAttr *Format, bool IsCXXMember,
2168-
bool IsVariadic, FormatStringInfo *FSI);
2164+
/// Given a function and its FormatAttr or FormatMatchesAttr info, attempts to
2165+
/// populate the FomatStringInfo parameter with the attribute's correct
2166+
/// format_idx and firstDataArg. Returns true when the format fits the
2167+
/// function and the FormatStringInfo has been populated.
2168+
static bool getFormatStringInfo(const Decl *Function, unsigned FormatIdx,
2169+
unsigned FirstArg, FormatStringInfo *FSI);
2170+
static bool getFormatStringInfo(unsigned FormatIdx, unsigned FirstArg,
2171+
bool IsCXXMember, bool IsVariadic,
2172+
FormatStringInfo *FSI);
21692173

21702174
// Used by C++ template instantiation.
21712175
ExprResult BuiltinShuffleVector(CallExpr *TheCall);
@@ -2188,7 +2192,9 @@ class Sema final : public SemaBase {
21882192
FST_Syslog,
21892193
FST_Unknown
21902194
};
2195+
static FormatStringType GetFormatStringType(StringRef FormatFlavor);
21912196
static FormatStringType GetFormatStringType(const FormatAttr *Format);
2197+
static FormatStringType GetFormatStringType(const FormatMatchesAttr *Format);
21922198

21932199
bool FormatStringHasSArg(const StringLiteral *FExpr);
21942200

@@ -2535,11 +2541,17 @@ class Sema final : public SemaBase {
25352541
VariadicCallType CallType, SourceLocation Loc,
25362542
SourceRange Range,
25372543
llvm::SmallBitVector &CheckedVarArgs);
2544+
bool CheckFormatString(const FormatMatchesAttr *Format,
2545+
ArrayRef<const Expr *> Args, bool IsCXXMember,
2546+
VariadicCallType CallType, SourceLocation Loc,
2547+
SourceRange Range,
2548+
llvm::SmallBitVector &CheckedVarArgs);
25382549
bool CheckFormatArguments(ArrayRef<const Expr *> Args,
2539-
FormatArgumentPassingKind FAPK, unsigned format_idx,
2540-
unsigned firstDataArg, FormatStringType Type,
2541-
VariadicCallType CallType, SourceLocation Loc,
2542-
SourceRange range,
2550+
FormatArgumentPassingKind FAPK,
2551+
const StringLiteral *ReferenceFormatString,
2552+
unsigned format_idx, unsigned firstDataArg,
2553+
FormatStringType Type, VariadicCallType CallType,
2554+
SourceLocation Loc, SourceRange range,
25432555
llvm::SmallBitVector &CheckedVarArgs);
25442556

25452557
void CheckInfNaNFunction(const CallExpr *Call, const FunctionDecl *FDecl);
@@ -4527,6 +4539,11 @@ class Sema final : public SemaBase {
45274539
FormatAttr *mergeFormatAttr(Decl *D, const AttributeCommonInfo &CI,
45284540
IdentifierInfo *Format, int FormatIdx,
45294541
int FirstArg);
4542+
FormatMatchesAttr *mergeFormatMatchesAttr(Decl *D,
4543+
const AttributeCommonInfo &CI,
4544+
IdentifierInfo *Format,
4545+
int FormatIdx,
4546+
StringLiteral *FormatStr);
45304547

45314548
/// AddAlignedAttr - Adds an aligned attribute to a particular declaration.
45324549
void AddAlignedAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E,

clang/lib/AST/AttrImpl.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,4 +270,9 @@ unsigned AlignedAttr::getAlignment(ASTContext &Ctx) const {
270270
return Ctx.getTargetDefaultAlignForAttributeAligned();
271271
}
272272

273+
StringLiteral *FormatMatchesAttr::getFormatString() const {
274+
// This cannot go in headers because StringLiteral and Expr may be incomplete.
275+
return cast<StringLiteral>(getExpectedFormat());
276+
}
277+
273278
#include "clang/AST/AttrImpl.inc"

0 commit comments

Comments
 (0)