Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions clang/include/clang/Basic/Builtins.h
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,10 @@ class Context {
bool performsCallback(unsigned ID,
llvm::SmallVectorImpl<int> &Encoding) const;

/// Return true if this builtin has parameters that must be non-null.
/// The parameter indices are appended into 'Indxs'.
bool isNonNull(unsigned ID, llvm::SmallVectorImpl<int> &Indxs) const;

/// Return true if this function has no side effects and doesn't
/// read memory, except for possibly errno or raising FP exceptions.
///
Expand Down
31 changes: 16 additions & 15 deletions clang/include/clang/Basic/Builtins.td
Original file line number Diff line number Diff line change
Expand Up @@ -3095,104 +3095,105 @@ def StrLen : LibBuiltin<"string.h"> {
// FIXME: This list is incomplete.
def Printf : LibBuiltin<"stdio.h"> {
let Spellings = ["printf"];
let Attributes = [PrintfFormat<0>];
let Attributes = [PrintfFormat<0>, NonNull<[0]>];
let Prototype = "int(char const*, ...)";
}

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

def FPrintf : LibBuiltin<"stdio.h"> {
let Spellings = ["fprintf"];
let Attributes = [NoThrow, PrintfFormat<1>];
let Attributes = [NoThrow, PrintfFormat<1>, NonNull<[0, 1]>];
let Prototype = "int(FILE* restrict, char const* restrict, ...)";
let AddBuiltinPrefixedAlias = 1;
}

def SnPrintf : LibBuiltin<"stdio.h"> {
let Spellings = ["snprintf"];
let Attributes = [NoThrow, PrintfFormat<2>];
let Attributes = [NoThrow, PrintfFormat<2>, NonNull<[2]>];
let Prototype = "int(char* restrict, size_t, char const* restrict, ...)";
let AddBuiltinPrefixedAlias = 1;
}

def SPrintf : LibBuiltin<"stdio.h"> {
let Spellings = ["sprintf"];
let Attributes = [NoThrow, PrintfFormat<1>];
let Attributes = [NoThrow, PrintfFormat<1>, NonNull<[0, 1]>];
let Prototype = "int(char* restrict, char const* restrict, ...)";
let AddBuiltinPrefixedAlias = 1;
}

def VPrintf : LibBuiltin<"stdio.h"> {
let Spellings = ["vprintf"];
let Attributes = [NoThrow, VPrintfFormat<0>];
let Attributes = [NoThrow, VPrintfFormat<0>, NonNull<[0]>];
let Prototype = "int(char const* restrict, __builtin_va_list)";
let AddBuiltinPrefixedAlias = 1;
}

def VfPrintf : LibBuiltin<"stdio.h"> {
let Spellings = ["vfprintf"];
let Attributes = [NoThrow, VPrintfFormat<1>];
let Attributes = [NoThrow, VPrintfFormat<1>, NonNull<[0, 1]>];
let Prototype = "int(FILE* restrict, char const* restrict, __builtin_va_list)";
let AddBuiltinPrefixedAlias = 1;
}

def VsnPrintf : LibBuiltin<"stdio.h"> {
let Spellings = ["vsnprintf"];
let Attributes = [NoThrow, VPrintfFormat<2>];
let Attributes = [NoThrow, VPrintfFormat<2>, NonNull<[2]>];
let Prototype = "int(char* restrict, size_t, char const* restrict, __builtin_va_list)";
let AddBuiltinPrefixedAlias = 1;
}

def VsPrintf : LibBuiltin<"stdio.h"> {
let Spellings = ["vsprintf"];
let Attributes = [NoThrow, VPrintfFormat<1>];
let Attributes = [NoThrow, VPrintfFormat<1>, NonNull<[0, 1]>];
let Prototype = "int(char* restrict, char const* restrict, __builtin_va_list)";
let AddBuiltinPrefixedAlias = 1;
}

def Scanf : LibBuiltin<"stdio.h"> {
let Spellings = ["scanf"];
let Attributes = [ScanfFormat<0>];
let Attributes = [ScanfFormat<0>, NonNull<[0]>];
let Prototype = "int(char const* restrict, ...)";
let AddBuiltinPrefixedAlias = 1;
}

def FScanf : LibBuiltin<"stdio.h"> {
let Spellings = ["fscanf"];
let Attributes = [ScanfFormat<1>];
let Attributes = [ScanfFormat<1>, NonNull<[0, 1]>];
let Prototype = "int(FILE* restrict, char const* restrict, ...)";
let AddBuiltinPrefixedAlias = 1;
}

def SScanf : LibBuiltin<"stdio.h"> {
let Spellings = ["sscanf"];
let Attributes = [ScanfFormat<1>];
let Attributes = [ScanfFormat<1>, NonNull<[0, 1]>];
let Prototype = "int(char const* restrict, char const* restrict, ...)";
let AddBuiltinPrefixedAlias = 1;
}

def VScanf : LibBuiltin<"stdio.h"> {
let Spellings = ["vscanf"];
let Attributes = [VScanfFormat<0>];
let Attributes = [VScanfFormat<0>, NonNull<[0]>];
let Prototype = "int(char const* restrict, __builtin_va_list)";
let AddBuiltinPrefixedAlias = 1;
}

def VFScanf : LibBuiltin<"stdio.h"> {
let Spellings = ["vfscanf"];
let Attributes = [VScanfFormat<1>];
let Attributes = [VScanfFormat<1>, NonNull<[0, 1]>];
let Prototype = "int(FILE* restrict, char const* restrict, __builtin_va_list)";
let AddBuiltinPrefixedAlias = 1;
}

def VSScanf : LibBuiltin<"stdio.h"> {
let Spellings = ["vsscanf"];
let Attributes = [VScanfFormat<1>];
let Attributes = [VScanfFormat<1>, NonNull<[0, 1]>];
let Prototype = "int(char const* restrict, char const* restrict, __builtin_va_list)";
let AddBuiltinPrefixedAlias = 1;
}
Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/Basic/BuiltinsBase.td
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ def Const : Attribute<"c">;
def NoThrow : Attribute<"n">;
def Pure : Attribute<"U">;
def ReturnsTwice : Attribute<"j">;
// FIXME: gcc has nonnull

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

// Prefixes
// ========
Expand Down
48 changes: 33 additions & 15 deletions clang/lib/Basic/Builtins.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -293,30 +293,48 @@ bool Builtin::Context::isScanfLike(unsigned ID, unsigned &FormatIdx,
return isLike(ID, FormatIdx, HasVAListArg, "sS");
}

bool Builtin::Context::performsCallback(unsigned ID,
SmallVectorImpl<int> &Encoding) const {
const char *CalleePos = ::strchr(getAttributesString(ID), 'C');
if (!CalleePos)
return false;

++CalleePos;
assert(*CalleePos == '<' &&
"Callback callee specifier must be followed by a '<'");
++CalleePos;
static void parseCommaSeparatedIndices(const char *CurrPos,
llvm::SmallVectorImpl<int> &Indxs) {
assert(*CurrPos == '<' && "Expected '<' to start index list");
++CurrPos;

char *EndPos;
int CalleeIdx = ::strtol(CalleePos, &EndPos, 10);
assert(CalleeIdx >= 0 && "Callee index is supposed to be positive!");
Encoding.push_back(CalleeIdx);
int PosIdx = ::strtol(CurrPos, &EndPos, 10);
assert(PosIdx >= 0 && "Index is supposed to be positive!");
Indxs.push_back(PosIdx);

while (*EndPos == ',') {
const char *PayloadPos = EndPos + 1;

int PayloadIdx = ::strtol(PayloadPos, &EndPos, 10);
Encoding.push_back(PayloadIdx);
Indxs.push_back(PayloadIdx);
}

assert(*EndPos == '>' && "Callback callee specifier must end with a '>'");
assert(*EndPos == '>' && "Index list must end with '>'");
}

bool Builtin::Context::isNonNull(unsigned ID,
llvm::SmallVectorImpl<int> &Indxs) const {

const char *AttrPos = ::strchr(getAttributesString(ID), 'N');
if (!AttrPos)
return false;

++AttrPos;
parseCommaSeparatedIndices(AttrPos, Indxs);

return true;
}

bool Builtin::Context::performsCallback(unsigned ID,
SmallVectorImpl<int> &Encoding) const {
const char *CalleePos = ::strchr(getAttributesString(ID), 'C');
if (!CalleePos)
return false;

++CalleePos;
parseCommaSeparatedIndices(CalleePos, Encoding);

return true;
}

Expand Down
9 changes: 9 additions & 0 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17141,6 +17141,15 @@ void Sema::AddKnownFunctionAttributes(FunctionDecl *FD) {
}
}

SmallVector<int, 4> Indxs;
if (Context.BuiltinInfo.isNonNull(BuiltinID, Indxs) &&
!FD->hasAttr<NonNullAttr>()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the hasAttr<NonNullAttr>() check be per parameter?

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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. __attribute__((nonnull(idx1, idx2))) int printf(...);). Since I didn’t find a parameter-specific equivalent to AddKnownFunctionAttributes, I went with this approach. I could alternatively iterate over the ParmVarDecls and attach attributes individually if needed.

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,
Expand Down
8 changes: 4 additions & 4 deletions clang/test/Analysis/stream.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,13 @@ void check_fputs(void) {

void check_fprintf(void) {
FILE *fp = tmpfile();
fprintf(fp, "ABC"); // expected-warning {{Stream pointer might be NULL}}
fprintf(fp, "ABC"); // expected-warning {{Null pointer passed to 1st parameter expecting 'nonnull'}}
fclose(fp);
}

void check_fscanf(void) {
FILE *fp = tmpfile();
fscanf(fp, "ABC"); // expected-warning {{Stream pointer might be NULL}}
fscanf(fp, "ABC"); // expected-warning {{Null pointer passed to 1st parameter expecting 'nonnull'}}
fclose(fp);
}

Expand Down Expand Up @@ -152,13 +152,13 @@ void f_dopen(int fd) {

void f_vfprintf(int fd, va_list args) {
FILE *F = fdopen(fd, "r");
vfprintf(F, "%d", args); // expected-warning {{Stream pointer might be NULL}}
vfprintf(F, "%d", args); // expected-warning {{Null pointer passed to 1st parameter expecting 'nonnull'}}
fclose(F);
}

void f_vfscanf(int fd, va_list args) {
FILE *F = fdopen(fd, "r");
vfscanf(F, "%u", args); // expected-warning {{Stream pointer might be NULL}}
vfscanf(F, "%u", args); // expected-warning {{Null pointer passed to 1st parameter expecting 'nonnull'}}
fclose(F);
}

Expand Down
79 changes: 79 additions & 0 deletions clang/test/Sema/format-strings-nonnull.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// 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(buf, 10, 0, 42);
// expected-warning@-1{{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(buf, 10, fmt, ap);
// 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(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}}
}
49 changes: 49 additions & 0 deletions clang/test/Sema/format-strings-nonnull.cpp
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}}
}
Loading