-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[clang][sema] Add nonnull attribute to builtin format functions #160988
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
4179480
c7a6ed9
775fcf1
265c9a6
70f1cb6
e055def
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17141,6 +17141,15 @@ void Sema::AddKnownFunctionAttributes(FunctionDecl *FD) { | |
| } | ||
| } | ||
|
|
||
| SmallVector<int, 4> Indxs; | ||
| if (Context.BuiltinInfo.isNonNull(BuiltinID, Indxs) && | ||
| !FD->hasAttr<NonNullAttr>()) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't the This reads as if __attribute__((nonnull)) void printf(const char*, ...)Would not get the attribute annotation, and similarly void printf(const char* __attribute__((nonnull)), ...)would have the attribute added again?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, ideally it could check at the parameter level, but this works equally well because nonnull can be applied at the function level (e.g. |
||
| llvm::SmallVector<ParamIdx, 4> ParamIndxs; | ||
| for (int I : Indxs) | ||
| ParamIndxs.push_back(ParamIdx(I + 1, FD)); | ||
| FD->addAttr(NonNullAttr::CreateImplicit(Context, ParamIndxs.data(), | ||
| ParamIndxs.size())); | ||
| } | ||
| if (Context.BuiltinInfo.isReturnsTwice(BuiltinID) && | ||
| !FD->hasAttr<ReturnsTwiceAttr>()) | ||
| FD->addAttr(ReturnsTwiceAttr::CreateImplicit(Context, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| // RUN: %clang_cc1 -fsyntax-only --std=c23 -verify -Wnonnull -Wno-format-security %s | ||
|
|
||
| #define NULL (void*)0 | ||
|
|
||
| typedef struct _FILE FILE; | ||
| typedef __SIZE_TYPE__ size_t; | ||
| typedef __builtin_va_list va_list; | ||
| int printf(char const* restrict, ...); | ||
| int __builtin_printf(char const* restrict, ...); | ||
| int fprintf(FILE* restrict, char const* restrict, ...); | ||
| int snprintf(char* restrict, size_t, char const* restrict, ...); | ||
| int sprintf(char* restrict, char const* restrict, ...); | ||
| int vprintf(char const* restrict, __builtin_va_list); | ||
| int vfprintf(FILE* restrict, char const* restrict, __builtin_va_list); | ||
| int vsnprintf(char* restrict, size_t, char const* restrict, __builtin_va_list); | ||
| int vsprintf(char* restrict, char const* restrict, __builtin_va_list); | ||
|
|
||
| int scanf(char const* restrict, ...); | ||
| int fscanf(FILE* restrict, char const* restrict, ...); | ||
| int sscanf(char const* restrict, char const* restrict, ...); | ||
| int vscanf(char const* restrict, __builtin_va_list); | ||
| int vfscanf(FILE* restrict, char const* restrict, __builtin_va_list); | ||
| int vsscanf(char const* restrict, char const* restrict, __builtin_va_list); | ||
|
|
||
|
|
||
| void check_format_string(FILE *fp, va_list ap) { | ||
| char buf[256]; | ||
| int num; | ||
| char* const fmt = NULL; | ||
|
|
||
| printf(fmt); | ||
| // expected-warning@-1{{null passed to a callee that requires a non-null argument}} | ||
|
|
||
| __builtin_printf(NULL, "xxd"); | ||
| // expected-warning@-1{{null passed to a callee that requires a non-null argument}} | ||
|
|
||
| fprintf(fp, NULL, 25); | ||
| // expected-warning@-1{{null passed to a callee that requires a non-null argument}} | ||
|
|
||
| sprintf(NULL, NULL, 42); | ||
| // expected-warning@-1{{null passed to a callee that requires a non-null argument}} | ||
| // expected-warning@-2{{null passed to a callee that requires a non-null argument}} | ||
|
|
||
| snprintf(NULL, 10, 0, 42); | ||
| // expected-warning@-1{{null passed to a callee that requires a non-null argument}} | ||
| // expected-warning@-2{{null passed to a callee that requires a non-null argument}} | ||
|
|
||
| vprintf(fmt, ap); | ||
| // expected-warning@-1{{null passed to a callee that requires a non-null argument}} | ||
|
|
||
| vfprintf(fp, 0, ap); | ||
| // expected-warning@-1{{null passed to a callee that requires a non-null argument}} | ||
|
|
||
| vsprintf(buf, nullptr, ap); | ||
| // expected-warning@-1{{null passed to a callee that requires a non-null argument}} | ||
|
|
||
| vsnprintf(NULL, 10, fmt, ap); | ||
| // expected-warning@-1{{null passed to a callee that requires a non-null argument}} | ||
| // expected-warning@-2{{null passed to a callee that requires a non-null argument}} | ||
|
|
||
| scanf(NULL); | ||
| // expected-warning@-1{{null passed to a callee that requires a non-null argument}} | ||
|
|
||
| fscanf(nullptr, nullptr); | ||
| // expected-warning@-1{{null passed to a callee that requires a non-null argument}} | ||
| // expected-warning@-2{{null passed to a callee that requires a non-null argument}} | ||
|
|
||
| sscanf(NULL, "%d %s", &num, buf); | ||
| // expected-warning@-1{{null passed to a callee that requires a non-null argument}} | ||
| sscanf(buf, fmt); | ||
| // expected-warning@-1{{null passed to a callee that requires a non-null argument}} | ||
|
|
||
| vscanf(NULL, ap); | ||
| // expected-warning@-1{{null passed to a callee that requires a non-null argument}} | ||
|
|
||
| vfscanf(fp, fmt, ap); | ||
| // expected-warning@-1{{null passed to a callee that requires a non-null argument}} | ||
|
|
||
| vsscanf(buf, NULL, ap); | ||
| // expected-warning@-1{{null passed to a callee that requires a non-null argument}} | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| // RUN: %clang_cc1 -fsyntax-only -verify -Wnonnull %s | ||
|
|
||
| #ifdef __cplusplus | ||
| # define EXTERN_C extern "C" | ||
| #else | ||
| # define EXTERN_C extern | ||
| #endif | ||
|
|
||
| typedef struct _FILE FILE; | ||
| typedef __SIZE_TYPE__ size_t; | ||
| typedef __builtin_va_list va_list; | ||
|
|
||
| EXTERN_C int printf(const char *, ...); | ||
| EXTERN_C int fprintf(FILE *, const char *restrict, ...); | ||
| EXTERN_C int sprintf(char* restrict, char const* res, ...); | ||
| EXTERN_C int vfprintf(FILE* restrict, char const* res, __builtin_va_list); | ||
|
|
||
| EXTERN_C int scanf(char const *restrict, ...); | ||
| EXTERN_C int fscanf(FILE* restrict, char const* res, ...); | ||
| EXTERN_C int sscanf(char const* restrict, char const* res, ...); | ||
|
|
||
| void test(FILE *fp, va_list ap) { | ||
| char buf[256]; | ||
| int num; | ||
|
|
||
| __builtin_printf(__null, "x"); | ||
| // expected-warning@-1 {{null passed to a callee that requires a non-null argument}} | ||
|
|
||
| printf(__null, "xxd"); | ||
| // expected-warning@-1 {{null passed to a callee that requires a non-null argument}} | ||
|
|
||
| fprintf(fp, __null, 42); | ||
| // expected-warning@-1 {{null passed to a callee that requires a non-null argument}} | ||
|
|
||
| sprintf(buf, __null); | ||
| // expected-warning@-1 {{null passed to a callee that requires a non-null argument}} | ||
|
|
||
| scanf(__null); | ||
| // expected-warning@-1 {{null passed to a callee that requires a non-null argument}} | ||
|
|
||
| fscanf(fp, __null); | ||
| // expected-warning@-1 {{null passed to a callee that requires a non-null argument}} | ||
|
|
||
| vfprintf(__null, "xxd", ap); | ||
| // expected-warning@-1 {{null passed to a callee that requires a non-null argument}} | ||
|
|
||
| sscanf(__null, "%d", &num); | ||
| // expected-warning@-1 {{null passed to a callee that requires a non-null argument}} | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this one is wrong (as are a few others); the first argument can be null if the second argument is 0. This is a case where we'd want
nonnull_if_nonzero.You should verify these annotations against the latest C standard draft: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3685.pdf