[clang] 81b4664 - [-Wunsafe-buffer-usage] Add check for custom printf/scanf functions (#173096)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 2 11:10:35 PST 2026
Author: Ziqing Luo
Date: 2026-01-02T11:10:31-08:00
New Revision: 81b46646fb5eb34559ef1e31d0ee83a69c18a301
URL: https://github.com/llvm/llvm-project/commit/81b46646fb5eb34559ef1e31d0ee83a69c18a301
DIFF: https://github.com/llvm/llvm-project/commit/81b46646fb5eb34559ef1e31d0ee83a69c18a301.diff
LOG: [-Wunsafe-buffer-usage] Add check for custom printf/scanf functions (#173096)
This commit adds support for functions annotated with
`__attribute__((__format__(__printf__, ...)))` (or `__scanf__`). These
functions will be treated the same way as printf/scanf functions in the
standard C library by `-Wunsafe-buffer-usage`
rdar://143233737
Added:
Modified:
clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
index fae5f8b8aa8e3..f9bba5d54e9c7 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -40,6 +40,7 @@ WARNING_GADGET(UnsafeBufferUsageCtorAttr)
WARNING_GADGET(DataInvocation)
WARNING_GADGET(UniquePtrArrayAccess)
WARNING_OPTIONAL_GADGET(UnsafeLibcFunctionCall)
+WARNING_OPTIONAL_GADGET(UnsafeFormatAttributedFunctionCall)
WARNING_OPTIONAL_GADGET(SpanTwoParamConstructor) // Uses of `std::span(arg0, arg1)`
FIXABLE_GADGET(ULCArraySubscript) // `DRE[any]` in an Unspecified Lvalue Context
FIXABLE_GADGET(DerefSimplePtrArithFixable)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 7ef20726d0ab9..620da756c3a9c 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -825,9 +825,13 @@ struct LibcFunNamePrefixSuffixParser {
//
// `UnsafeArg` is the output argument that will be set only if this function
// returns true.
-static bool hasUnsafeFormatOrSArg(const CallExpr *Call, const Expr *&UnsafeArg,
- const unsigned FmtArgIdx, ASTContext &Ctx,
- bool isKprintf = false) {
+//
+// Format arguments start at `FmtIdx` + 1, if `FmtArgIdx` is insignificant.
+static bool
+hasUnsafeFormatOrSArg(ASTContext &Ctx, const CallExpr *Call,
+ const Expr *&UnsafeArg, const unsigned FmtIdx,
+ std::optional<const unsigned> FmtArgIdx = std::nullopt,
+ bool isKprintf = false) {
class StringFormatStringHandler
: public analyze_format_string::FormatStringHandler {
const CallExpr *Call;
@@ -847,18 +851,18 @@ static bool hasUnsafeFormatOrSArg(const CallExpr *Call, const Expr *&UnsafeArg,
const Expr *
getPrecisionAsExpr(const analyze_printf::OptionalAmount &Precision,
const CallExpr *Call) {
- unsigned PArgIdx = -1;
-
- if (Precision.hasDataArgument())
- PArgIdx = Precision.getPositionalArgIndex() + FmtArgIdx;
- if (0 < PArgIdx && PArgIdx < Call->getNumArgs()) {
- const Expr *PArg = Call->getArg(PArgIdx);
-
- // Strip the cast if `PArg` is a cast-to-int expression:
- if (auto *CE = dyn_cast<CastExpr>(PArg);
- CE && CE->getType()->isSignedIntegerType())
- PArg = CE->getSubExpr();
- return PArg;
+ if (Precision.hasDataArgument()) {
+ unsigned PArgIdx = Precision.getArgIndex() + FmtArgIdx;
+
+ if (PArgIdx < Call->getNumArgs()) {
+ const Expr *PArg = Call->getArg(PArgIdx);
+
+ // Strip the cast if `PArg` is a cast-to-int expression:
+ if (auto *CE = dyn_cast<CastExpr>(PArg);
+ CE && CE->getType()->isSignedIntegerType())
+ PArg = CE->getSubExpr();
+ return PArg;
+ }
}
if (Precision.getHowSpecified() ==
analyze_printf::OptionalAmount::HowSpecified::Constant) {
@@ -886,9 +890,9 @@ static bool hasUnsafeFormatOrSArg(const CallExpr *Call, const Expr *&UnsafeArg,
analyze_printf::PrintfConversionSpecifier::sArg)
return true; // continue parsing
- unsigned ArgIdx = FS.getPositionalArgIndex() + FmtArgIdx;
+ unsigned ArgIdx = FS.getArgIndex() + FmtArgIdx;
- if (!(0 < ArgIdx && ArgIdx < Call->getNumArgs()))
+ if (ArgIdx >= Call->getNumArgs())
// If the `ArgIdx` is invalid, give up.
return true; // continue parsing
@@ -921,12 +925,15 @@ static bool hasUnsafeFormatOrSArg(const CallExpr *Call, const Expr *&UnsafeArg,
bool isUnsafeArgSet() { return UnsafeArgSet; }
};
- const Expr *Fmt = Call->getArg(FmtArgIdx);
+ const Expr *Fmt = Call->getArg(FmtIdx);
+ unsigned FmtArgStartingIdx =
+ FmtArgIdx.has_value() ? static_cast<unsigned>(*FmtArgIdx) : FmtIdx + 1;
if (auto *SL = dyn_cast<clang::StringLiteral>(Fmt->IgnoreParenImpCasts())) {
if (SL->getCharByteWidth() == 1) {
StringRef FmtStr = SL->getString();
- StringFormatStringHandler Handler(Call, FmtArgIdx, UnsafeArg, Ctx);
+ StringFormatStringHandler Handler(Call, FmtArgStartingIdx, UnsafeArg,
+ Ctx);
return analyze_format_string::ParsePrintfString(
Handler, FmtStr.begin(), FmtStr.end(), Ctx.getLangOpts(),
@@ -935,7 +942,8 @@ static bool hasUnsafeFormatOrSArg(const CallExpr *Call, const Expr *&UnsafeArg,
}
if (auto FmtStr = SL->tryEvaluateString(Ctx)) {
- StringFormatStringHandler Handler(Call, FmtArgIdx, UnsafeArg, Ctx);
+ StringFormatStringHandler Handler(Call, FmtArgStartingIdx, UnsafeArg,
+ Ctx);
return analyze_format_string::ParsePrintfString(
Handler, FmtStr->data(), FmtStr->data() + FmtStr->size(),
Ctx.getLangOpts(), Ctx.getTargetInfo(), isKprintf) &&
@@ -946,7 +954,7 @@ static bool hasUnsafeFormatOrSArg(const CallExpr *Call, const Expr *&UnsafeArg,
// In this case, this call is considered unsafe if at least one argument
// (including the format argument) is unsafe pointer.
return llvm::any_of(
- llvm::make_range(Call->arg_begin() + FmtArgIdx, Call->arg_end()),
+ llvm::make_range(Call->arg_begin() + FmtArgStartingIdx, Call->arg_end()),
[&UnsafeArg, &Ctx](const Expr *Arg) -> bool {
if (Arg->getType()->isPointerType() && !isNullTermPointer(Arg, Ctx)) {
UnsafeArg = Arg;
@@ -1161,7 +1169,7 @@ static bool hasUnsafePrintfStringArg(const CallExpr &Node, ASTContext &Ctx,
// It is a fprintf:
const Expr *UnsafeArg;
- if (hasUnsafeFormatOrSArg(&Node, UnsafeArg, 1, Ctx, false)) {
+ if (hasUnsafeFormatOrSArg(Ctx, &Node, UnsafeArg, /* FmtIdx= */ 1)) {
Result.addNode(Tag, DynTypedNode::create(*UnsafeArg));
return true;
}
@@ -1175,7 +1183,8 @@ static bool hasUnsafePrintfStringArg(const CallExpr &Node, ASTContext &Ctx,
if (auto *II = FD->getIdentifier())
isKprintf = II->getName() == "kprintf";
- if (hasUnsafeFormatOrSArg(&Node, UnsafeArg, 0, Ctx, isKprintf)) {
+ if (hasUnsafeFormatOrSArg(Ctx, &Node, UnsafeArg, /* FmtIdx= */ 0,
+ /* FmtArgIdx= */ std::nullopt, isKprintf)) {
Result.addNode(Tag, DynTypedNode::create(*UnsafeArg));
return true;
}
@@ -1190,7 +1199,7 @@ static bool hasUnsafePrintfStringArg(const CallExpr &Node, ASTContext &Ctx,
// second is an integer, it is a snprintf:
const Expr *UnsafeArg;
- if (hasUnsafeFormatOrSArg(&Node, UnsafeArg, 2, Ctx, false)) {
+ if (hasUnsafeFormatOrSArg(Ctx, &Node, UnsafeArg, /* FmtIdx= */ 2)) {
Result.addNode(Tag, DynTypedNode::create(*UnsafeArg));
return true;
}
@@ -2068,6 +2077,7 @@ class UnsafeLibcFunctionCallGadget : public WarningGadget {
constexpr static const char *const UnsafeVaListTag =
"UnsafeLibcFunctionCall_va_list";
+public:
enum UnsafeKind {
OTHERS = 0, // no specific information, the callee function is unsafe
SPRINTF = 1, // never call `-sprintf`s, call `-snprintf`s instead.
@@ -2080,7 +2090,6 @@ class UnsafeLibcFunctionCallGadget : public WarningGadget {
// considered unsafe as it is not compile-time check
} WarnedFunKind = OTHERS;
-public:
UnsafeLibcFunctionCallGadget(const MatchResult &Result)
: WarningGadget(Kind::UnsafeLibcFunctionCall),
Call(Result.getNodeAs<CallExpr>(Tag)) {
@@ -2171,6 +2180,86 @@ class UnsafeLibcFunctionCallGadget : public WarningGadget {
SmallVector<const Expr *, 1> getUnsafePtrs() const override { return {}; }
};
+class UnsafeFormatAttributedFunctionCallGadget : public WarningGadget {
+ const CallExpr *const Call;
+ const Expr *UnsafeArg = nullptr;
+ constexpr static const char *const Tag = "UnsafeFormatAttributedFunctionCall";
+ constexpr static const char *const UnsafeStringTag =
+ "UnsafeFormatAttributedFunctionCall_string";
+
+public:
+ UnsafeFormatAttributedFunctionCallGadget(const MatchResult &Result)
+ : WarningGadget(Kind::UnsafeLibcFunctionCall),
+ Call(Result.getNodeAs<CallExpr>(Tag)),
+ UnsafeArg(Result.getNodeAs<Expr>(UnsafeStringTag)) {}
+
+ static bool matches(const Stmt *S, ASTContext &Ctx,
+ const UnsafeBufferUsageHandler *Handler,
+ MatchResult &Result) {
+ if (ignoreUnsafeLibcCall(Ctx, *S, Handler))
+ return false;
+ auto *CE = dyn_cast<CallExpr>(S);
+ if (!CE || !CE->getDirectCallee())
+ return false;
+ const auto *FD = dyn_cast<FunctionDecl>(CE->getDirectCallee());
+ if (!FD)
+ return false;
+
+ const FormatAttr *Attr = nullptr;
+ bool IsPrintf = false;
+ bool AnyAttr = llvm::any_of(
+ FD->specific_attrs<FormatAttr>(),
+ [&Attr, &IsPrintf](const FormatAttr *FA) -> bool {
+ if (const auto *II = FA->getType()) {
+ if (II->getName() == "printf" || II->getName() == "scanf") {
+ Attr = FA;
+ IsPrintf = II->getName() == "printf";
+ return true;
+ }
+ }
+ return false;
+ });
+ const Expr *UnsafeArg;
+
+ if (AnyAttr && !IsPrintf &&
+ (CE->getNumArgs() >= static_cast<unsigned>(Attr->getFirstArg()))) {
+ // for scanf-like functions, any format argument is considered unsafe:
+ Result.addNode(Tag, DynTypedNode::create(*CE));
+ return true;
+ }
+ if (AnyAttr && libc_func_matchers::hasUnsafeFormatOrSArg(
+ Ctx, CE, UnsafeArg,
+ // FormatAttribute indexes are 1-based:
+ /* FmtIdx= */ Attr->getFormatIdx() - 1,
+ /* FmtArgIdx= */ Attr->getFirstArg() - 1)) {
+ Result.addNode(Tag, DynTypedNode::create(*CE));
+ Result.addNode(UnsafeStringTag, DynTypedNode::create(*UnsafeArg));
+ return true;
+ }
+ return false;
+ }
+
+ const Stmt *getBaseStmt() const { return Call; }
+
+ SourceLocation getSourceLoc() const override { return Call->getBeginLoc(); }
+
+ void handleUnsafeOperation(UnsafeBufferUsageHandler &Handler,
+ bool IsRelatedToDecl,
+ ASTContext &Ctx) const override {
+ if (UnsafeArg)
+ Handler.handleUnsafeLibcCall(
+ Call, UnsafeLibcFunctionCallGadget::UnsafeKind::STRING, Ctx,
+ UnsafeArg);
+ else
+ Handler.handleUnsafeLibcCall(
+ Call, UnsafeLibcFunctionCallGadget::UnsafeKind::OTHERS, Ctx);
+ }
+
+ DeclUseList getClaimedVarUseSites() const override { return {}; }
+
+ SmallVector<const Expr *, 1> getUnsafePtrs() const override { return {}; }
+};
+
// Represents expressions of the form `DRE[*]` in the Unspecified Lvalue
// Context (see `findStmtsInUnspecifiedLvalueContext`).
// Note here `[]` is the built-in subscript operator.
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp
index 4f1af79609223..a68ecf128c3d9 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp
@@ -1,10 +1,10 @@
-// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage \
+// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage -Wno-gcc-compat\
// RUN: -verify %s
-// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage \
+// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage -Wno-gcc-compat\
// RUN: -verify %s -x objective-c++
-// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage-in-libc-call \
+// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage-in-libc-call -Wno-gcc-compat\
// RUN: -verify %s
-// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage-in-libc-call \
+// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage-in-libc-call -Wno-gcc-compat\
// RUN: -verify %s -DTEST_STD_NS
typedef struct {} FILE;
@@ -255,3 +255,34 @@ void dontCrashForInvalidFormatString() {
snprintf((char*)0, 0, "%");
snprintf((char*)0, 0, "\0");
}
+
+
+// Also warn about unsafe printf/scanf-like functions:
+void myprintf(const char *, ...) __attribute__((__format__ (__printf__, 1, 2)));
+void myprintf_2(const char *, int, const char *) __attribute__((__format__ (__printf__, 1, 3)));
+void myprintf_3(const char *, const char *, int, const char *) __attribute__((__format__ (__printf__, 2, 4)));
+void myscanf(const char *, ...) __attribute__((__format__ (__scanf__, 1, 2)));
+
+void test_myprintf(char * Str, std::string StdStr) {
+ myprintf("hello", Str);
+ myprintf("hello %s", StdStr.c_str());
+ myprintf("hello %s", Str); // expected-warning{{function 'myprintf' is unsafe}} \
+ expected-note{{string argument is not guaranteed to be null-terminated}}
+
+ myprintf_2("hello", 0, Str);
+ myprintf_2("hello %s", 0, StdStr.c_str());
+ myprintf_2("hello %s", 0, Str); // expected-warning{{function 'myprintf_2' is unsafe}} \
+ expected-note{{string argument is not guaranteed to be null-terminated}}
+
+ myprintf_3("irrelevant", "hello", 0, Str);
+ myprintf_3("irrelevant", "hello %s", 0, StdStr.c_str());
+ myprintf_3("irrelevant", "hello %s", 0, Str); // expected-warning{{function 'myprintf_3' is unsafe}} \
+ expected-note{{string argument is not guaranteed to be null-terminated}}
+
+ myscanf("hello %s");
+ myscanf("hello %s", Str); // expected-warning{{function 'myscanf' is unsafe}}
+
+ int X;
+
+ myscanf("hello %d", &X); // expected-warning{{function 'myscanf' is unsafe}}
+}
More information about the cfe-commits
mailing list