Skip to content

Commit 660c921

Browse files
committed
[clang][sema] Add nonnull attribute to builtin format functions
Annotate printf/scanf and related builtins with the nonnull attribute on their format string parameters. This enables diagnostics when NULL is passed, matching GCC behavior. Updated existing Sema tests and added new one for coverage.
1 parent 28c9452 commit 660c921

File tree

10 files changed

+139
-21
lines changed

10 files changed

+139
-21
lines changed

clang/include/clang/Basic/Builtins.def

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
// The third value provided to the macro specifies information about attributes
6868
// of the function. These must be kept in sync with the predicates in the
6969
// Builtin::Context class. Currently we have:
70+
// N -> nonnull
7071
// n -> nothrow
7172
// r -> noreturn
7273
// U -> pure

clang/include/clang/Basic/Builtins.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,10 @@ class Context {
392392
bool performsCallback(unsigned ID,
393393
llvm::SmallVectorImpl<int> &Encoding) const;
394394

395+
/// Return true if this builtin has parameters at fixed positions
396+
/// that must be non-null.
397+
bool IsNonNull(unsigned ID, llvm::SmallVectorImpl<int> &Indxs) const;
398+
395399
/// Return true if this function has no side effects and doesn't
396400
/// read memory, except for possibly errno or raising FP exceptions.
397401
///

clang/include/clang/Basic/Builtins.td

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3083,104 +3083,105 @@ def StrLen : LibBuiltin<"string.h"> {
30833083
// FIXME: This list is incomplete.
30843084
def Printf : LibBuiltin<"stdio.h"> {
30853085
let Spellings = ["printf"];
3086-
let Attributes = [PrintfFormat<0>];
3086+
let Attributes = [PrintfFormat<0>, NonNull<[0]>];
30873087
let Prototype = "int(char const*, ...)";
30883088
}
30893089

30903090
// FIXME: The builtin and library function should have the same signature.
30913091
def BuiltinPrintf : Builtin {
30923092
let Spellings = ["__builtin_printf"];
3093-
let Attributes = [NoThrow, PrintfFormat<0>, FunctionWithBuiltinPrefix];
3093+
let Attributes = [NoThrow, PrintfFormat<0>, FunctionWithBuiltinPrefix,
3094+
NonNull<[0]>];
30943095
let Prototype = "int(char const* restrict, ...)";
30953096
}
30963097

30973098
def FPrintf : LibBuiltin<"stdio.h"> {
30983099
let Spellings = ["fprintf"];
3099-
let Attributes = [NoThrow, PrintfFormat<1>];
3100+
let Attributes = [NoThrow, PrintfFormat<1>, NonNull<[1]>];
31003101
let Prototype = "int(FILE* restrict, char const* restrict, ...)";
31013102
let AddBuiltinPrefixedAlias = 1;
31023103
}
31033104

31043105
def SnPrintf : LibBuiltin<"stdio.h"> {
31053106
let Spellings = ["snprintf"];
3106-
let Attributes = [NoThrow, PrintfFormat<2>];
3107+
let Attributes = [NoThrow, PrintfFormat<2>, NonNull<[2]>];
31073108
let Prototype = "int(char* restrict, size_t, char const* restrict, ...)";
31083109
let AddBuiltinPrefixedAlias = 1;
31093110
}
31103111

31113112
def SPrintf : LibBuiltin<"stdio.h"> {
31123113
let Spellings = ["sprintf"];
3113-
let Attributes = [NoThrow, PrintfFormat<1>];
3114+
let Attributes = [NoThrow, PrintfFormat<1>, NonNull<[1]>];
31143115
let Prototype = "int(char* restrict, char const* restrict, ...)";
31153116
let AddBuiltinPrefixedAlias = 1;
31163117
}
31173118

31183119
def VPrintf : LibBuiltin<"stdio.h"> {
31193120
let Spellings = ["vprintf"];
3120-
let Attributes = [NoThrow, VPrintfFormat<0>];
3121+
let Attributes = [NoThrow, VPrintfFormat<0>, NonNull<[0]>];
31213122
let Prototype = "int(char const* restrict, __builtin_va_list)";
31223123
let AddBuiltinPrefixedAlias = 1;
31233124
}
31243125

31253126
def VfPrintf : LibBuiltin<"stdio.h"> {
31263127
let Spellings = ["vfprintf"];
3127-
let Attributes = [NoThrow, VPrintfFormat<1>];
3128+
let Attributes = [NoThrow, VPrintfFormat<1>, NonNull<[1]>];
31283129
let Prototype = "int(FILE* restrict, char const* restrict, __builtin_va_list)";
31293130
let AddBuiltinPrefixedAlias = 1;
31303131
}
31313132

31323133
def VsnPrintf : LibBuiltin<"stdio.h"> {
31333134
let Spellings = ["vsnprintf"];
3134-
let Attributes = [NoThrow, VPrintfFormat<2>];
3135+
let Attributes = [NoThrow, VPrintfFormat<2>, NonNull<[2]>];
31353136
let Prototype = "int(char* restrict, size_t, char const* restrict, __builtin_va_list)";
31363137
let AddBuiltinPrefixedAlias = 1;
31373138
}
31383139

31393140
def VsPrintf : LibBuiltin<"stdio.h"> {
31403141
let Spellings = ["vsprintf"];
3141-
let Attributes = [NoThrow, VPrintfFormat<1>];
3142+
let Attributes = [NoThrow, VPrintfFormat<1>, NonNull<[1]>];
31423143
let Prototype = "int(char* restrict, char const* restrict, __builtin_va_list)";
31433144
let AddBuiltinPrefixedAlias = 1;
31443145
}
31453146

31463147
def Scanf : LibBuiltin<"stdio.h"> {
31473148
let Spellings = ["scanf"];
3148-
let Attributes = [ScanfFormat<0>];
3149+
let Attributes = [ScanfFormat<0>, NonNull<[0]>];
31493150
let Prototype = "int(char const* restrict, ...)";
31503151
let AddBuiltinPrefixedAlias = 1;
31513152
}
31523153

31533154
def FScanf : LibBuiltin<"stdio.h"> {
31543155
let Spellings = ["fscanf"];
3155-
let Attributes = [ScanfFormat<1>];
3156+
let Attributes = [ScanfFormat<1>, NonNull<[1]>];
31563157
let Prototype = "int(FILE* restrict, char const* restrict, ...)";
31573158
let AddBuiltinPrefixedAlias = 1;
31583159
}
31593160

31603161
def SScanf : LibBuiltin<"stdio.h"> {
31613162
let Spellings = ["sscanf"];
3162-
let Attributes = [ScanfFormat<1>];
3163+
let Attributes = [ScanfFormat<1>, NonNull<[1]>];
31633164
let Prototype = "int(char const* restrict, char const* restrict, ...)";
31643165
let AddBuiltinPrefixedAlias = 1;
31653166
}
31663167

31673168
def VScanf : LibBuiltin<"stdio.h"> {
31683169
let Spellings = ["vscanf"];
3169-
let Attributes = [VScanfFormat<0>];
3170+
let Attributes = [VScanfFormat<0>, NonNull<[0]>];
31703171
let Prototype = "int(char const* restrict, __builtin_va_list)";
31713172
let AddBuiltinPrefixedAlias = 1;
31723173
}
31733174

31743175
def VFScanf : LibBuiltin<"stdio.h"> {
31753176
let Spellings = ["vfscanf"];
3176-
let Attributes = [VScanfFormat<1>];
3177+
let Attributes = [VScanfFormat<1>, NonNull<[1]>];
31773178
let Prototype = "int(FILE* restrict, char const* restrict, __builtin_va_list)";
31783179
let AddBuiltinPrefixedAlias = 1;
31793180
}
31803181

31813182
def VSScanf : LibBuiltin<"stdio.h"> {
31823183
let Spellings = ["vsscanf"];
3183-
let Attributes = [VScanfFormat<1>];
3184+
let Attributes = [VScanfFormat<1>, NonNull<[1]>];
31843185
let Prototype = "int(char const* restrict, char const* restrict, __builtin_va_list)";
31853186
let AddBuiltinPrefixedAlias = 1;
31863187
}

clang/include/clang/Basic/BuiltinsBase.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ def Const : Attribute<"c">;
3232
def NoThrow : Attribute<"n">;
3333
def Pure : Attribute<"U">;
3434
def ReturnsTwice : Attribute<"j">;
35-
// FIXME: gcc has nonnull
3635

3736
// builtin-specific attributes
3837
// ---------------------------
@@ -85,6 +84,7 @@ def Consteval : Attribute<"EG">;
8584
// Callback behavior: the first index argument is called with the arguments
8685
// indicated by the remaining indices.
8786
class Callback<list<int> ArgIndices> : MultiIndexAttribute<"C", ArgIndices>;
87+
class NonNull<list<int> ArgIndices> : MultiIndexAttribute<"N", ArgIndices>;
8888

8989
// Prefixes
9090
// ========

clang/lib/Basic/Builtins.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,34 @@ bool Builtin::Context::isScanfLike(unsigned ID, unsigned &FormatIdx,
293293
return isLike(ID, FormatIdx, HasVAListArg, "sS");
294294
}
295295

296+
bool Builtin::Context::IsNonNull(unsigned ID,
297+
llvm::SmallVectorImpl<int> &Indxs) const {
298+
299+
const char *CalleePos = ::strchr(getAttributesString(ID), 'N');
300+
if (!CalleePos)
301+
return false;
302+
303+
++CalleePos;
304+
assert(*CalleePos == '<' &&
305+
"Callback callee specifier must be followed by a '<'");
306+
++CalleePos;
307+
308+
char *EndPos;
309+
int CalleeIdx = ::strtol(CalleePos, &EndPos, 10);
310+
assert(CalleeIdx >= 0 && "Callee index is supposed to be positive!");
311+
Indxs.push_back(CalleeIdx);
312+
313+
while (*EndPos == ',') {
314+
const char *PayloadPos = EndPos + 1;
315+
316+
int PayloadIdx = ::strtol(PayloadPos, &EndPos, 10);
317+
Indxs.push_back(PayloadIdx);
318+
}
319+
320+
assert(*EndPos == '>' && "Callback callee specifier must end with a '>'");
321+
return true;
322+
}
323+
296324
bool Builtin::Context::performsCallback(unsigned ID,
297325
SmallVectorImpl<int> &Encoding) const {
298326
const char *CalleePos = ::strchr(getAttributesString(ID), 'C');

clang/lib/Sema/SemaDecl.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17097,6 +17097,15 @@ void Sema::AddKnownFunctionAttributes(FunctionDecl *FD) {
1709717097
}
1709817098
}
1709917099

17100+
SmallVector<int, 4> Indxs;
17101+
if (Context.BuiltinInfo.IsNonNull(BuiltinID, Indxs) &&
17102+
!FD->hasAttr<NonNullAttr>()) {
17103+
llvm::SmallVector<ParamIdx, 4> ParamIndxs;
17104+
for (int I : Indxs)
17105+
ParamIndxs.push_back(ParamIdx(I + 1, FD));
17106+
FD->addAttr(NonNullAttr::CreateImplicit(Context, ParamIndxs.data(),
17107+
ParamIndxs.size()));
17108+
}
1710017109
if (Context.BuiltinInfo.isReturnsTwice(BuiltinID) &&
1710117110
!FD->hasAttr<ReturnsTwiceAttr>())
1710217111
FD->addAttr(ReturnsTwiceAttr::CreateImplicit(Context,
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
// RUN: %clang_cc1 -fsyntax-only -verify -Wnonnull -Wno-format-security %s
2+
3+
#include <stdarg.h>
4+
#include <stddef.h>
5+
6+
typedef struct _FILE FILE;
7+
8+
int printf(char const* restrict, ...);
9+
int __builtin_printf(char const* restrict, ...);
10+
int fprintf(FILE* restrict, char const* restrict, ...);
11+
int snprintf(char* restrict, size_t, char const* restrict, ...);
12+
int sprintf(char* restrict, char const* restrict, ...);
13+
int vprintf(char const* restrict, __builtin_va_list);
14+
int vfprintf(FILE* restrict, char const* restrict, __builtin_va_list);
15+
int vsnprintf(char* restrict, size_t, char const* restrict, __builtin_va_list);
16+
int vsprintf(char* restrict, char const* restrict, __builtin_va_list);
17+
18+
int scanf(char const* restrict, ...);
19+
int fscanf(FILE* restrict, char const* restrict, ...);
20+
int sscanf(char const* restrict, char const* restrict, ...);
21+
int vscanf(char const* restrict, __builtin_va_list);
22+
int vfscanf(FILE* restrict, char const* restrict, __builtin_va_list);
23+
int vsscanf(char const* restrict, char const* restrict, __builtin_va_list);
24+
25+
26+
void check_format_string(FILE *fp, va_list ap) {
27+
char buf[256];
28+
char* const fmt = NULL;
29+
30+
printf(fmt);
31+
// expected-warning@-1{{null passed to a callee that requires a non-null argument}}
32+
33+
__builtin_printf(NULL, "xxd");
34+
// expected-warning@-1{{null passed to a callee that requires a non-null argument}}
35+
36+
fprintf(fp, NULL, 25);
37+
// expected-warning@-1{{null passed to a callee that requires a non-null argument}}
38+
39+
sprintf(buf, NULL, 42);
40+
// expected-warning@-1{{null passed to a callee that requires a non-null argument}}
41+
42+
snprintf(buf, 10, 0, 42);
43+
// expected-warning@-1{{null passed to a callee that requires a non-null argument}}
44+
45+
vprintf(fmt, ap);
46+
// expected-warning@-1{{null passed to a callee that requires a non-null argument}}
47+
48+
vfprintf(fp, 0, ap);
49+
// expected-warning@-1{{null passed to a callee that requires a non-null argument}}
50+
51+
vsprintf(buf, NULL, ap);
52+
// expected-warning@-1{{null passed to a callee that requires a non-null argument}}
53+
54+
vsnprintf(buf, 10, fmt, ap);
55+
// expected-warning@-1{{null passed to a callee that requires a non-null argument}}
56+
57+
scanf(NULL);
58+
// expected-warning@-1{{null passed to a callee that requires a non-null argument}}
59+
60+
fscanf(fp, NULL);
61+
// expected-warning@-1{{null passed to a callee that requires a non-null argument}}
62+
63+
sscanf(buf, fmt);
64+
// expected-warning@-1{{null passed to a callee that requires a non-null argument}}
65+
66+
vscanf(NULL, ap);
67+
// expected-warning@-1{{null passed to a callee that requires a non-null argument}}
68+
69+
vfscanf(fp, fmt, ap);
70+
// expected-warning@-1{{null passed to a callee that requires a non-null argument}}
71+
72+
vsscanf(buf, NULL, ap);
73+
// expected-warning@-1{{null passed to a callee that requires a non-null argument}}
74+
}

clang/test/Sema/format-strings.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -480,11 +480,9 @@ void pr7981(wint_t c, wchar_t c2) {
480480
#endif
481481
}
482482

483-
// -Wformat-security says NULL is not a string literal
484483
void rdar8269537(void) {
485-
// This is likely to crash in most cases, but -Wformat-nonliteral technically
486-
// doesn't warn in this case.
487-
printf(0); // no-warning
484+
printf(0);
485+
// expected-warning@-1{{null passed to a callee that requires a non-null argument}}
488486
}
489487

490488
// Handle functions with multiple format attributes.

clang/test/SemaCXX/format-strings-0x.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ void f(char **sp, float *fp) {
1414
printf("%a", 1.0);
1515
scanf("%afoobar", fp);
1616
printf(nullptr);
17+
// expected-warning@-1{{null passed to a callee that requires a non-null argument}}
1718
printf(*sp); // expected-warning {{not a string literal}}
1819
// expected-note@-1{{treat the string as an argument to avoid this}}
1920

@@ -32,4 +33,5 @@ void f(char **sp, float *fp) {
3233
printf("init list: %d", { 0 }); // expected-error {{cannot pass initializer list to variadic function; expected type from format string was 'int'}}
3334
printf("void: %d", f(sp, fp)); // expected-error {{cannot pass expression of type 'void' to variadic function; expected type from format string was 'int'}}
3435
printf(0, { 0 }); // expected-error {{cannot pass initializer list to variadic function}}
36+
// expected-warning@-1{{null passed to a callee that requires a non-null argument}}
3537
}

clang/test/SemaObjC/format-strings-objc.m

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ void rdar10743758(id x) {
130130
printf(s2); // expected-warning {{more '%' conversions than data arguments}}
131131

132132
const char * const s3 = (const char *)0;
133-
printf(s3); // no-warning (NULL is a valid format string)
133+
printf(s3); // expected-warning {{null passed to a callee that requires a non-null argument}}
134134

135135
NSString * const ns1 = @"constant string %s"; // expected-note {{format string is defined here}}
136136
NSLog(ns1); // expected-warning {{more '%' conversions than data arguments}}
@@ -259,6 +259,7 @@ void testByValueObjectInFormat(Foo *obj) {
259259
printf("%d %d %d", 1L, *obj, 1L); // expected-error {{cannot pass object with interface type 'Foo' by value to variadic function; expected type from format string was 'int'}} expected-warning 2 {{format specifies type 'int' but the argument has type 'long'}}
260260
printf("%!", *obj); // expected-error {{cannot pass object with interface type 'Foo' by value through variadic function}} expected-warning {{invalid conversion specifier}}
261261
printf(0, *obj); // expected-error {{cannot pass object with interface type 'Foo' by value through variadic function}}
262+
// expected-warning@-1{{null passed to a callee that requires a non-null argument}}
262263

263264
[Bar log2:@"%d", *obj]; // expected-error {{cannot pass object with interface type 'Foo' by value to variadic method; expected type from format string was 'int'}}
264265
}

0 commit comments

Comments
 (0)