Skip to content

Commit ebf25d9

Browse files
authored
[-Wunsafe-buffer-usage] Reduce false positives with constant arrays in libc warnings (#108308)
For `snprintf(a, sizeof a, ...)`, the first two arguments form a safe pattern if `a` is a constant array. In such a case, this commit will suppress the warning. (rdar://117182250)
1 parent 90f077c commit ebf25d9

File tree

2 files changed

+38
-1
lines changed

2 files changed

+38
-1
lines changed

clang/lib/Analysis/UnsafeBufferUsage.cpp

+25
Original file line numberDiff line numberDiff line change
@@ -833,9 +833,16 @@ AST_MATCHER_P(CallExpr, hasUnsafePrintfStringArg,
833833
//
834834
// For the first two arguments: `ptr` and `size`, they are safe if in the
835835
// following patterns:
836+
//
837+
// Pattern 1:
836838
// ptr := DRE.data();
837839
// size:= DRE.size()/DRE.size_bytes()
838840
// And DRE is a hardened container or view.
841+
//
842+
// Pattern 2:
843+
// ptr := Constant-Array-DRE;
844+
// size:= any expression that has compile-time constant value equivalent to
845+
// sizeof (Constant-Array-DRE)
839846
AST_MATCHER(CallExpr, hasUnsafeSnprintfBuffer) {
840847
const FunctionDecl *FD = Node.getDirectCallee();
841848

@@ -856,6 +863,7 @@ AST_MATCHER(CallExpr, hasUnsafeSnprintfBuffer) {
856863
!Size->getType()->isIntegerType())
857864
return false; // not an snprintf call
858865

866+
// Pattern 1:
859867
static StringRef SizedObjs[] = {"span", "array", "vector",
860868
"basic_string_view", "basic_string"};
861869
Buf = Buf->IgnoreParenImpCasts();
@@ -886,6 +894,23 @@ AST_MATCHER(CallExpr, hasUnsafeSnprintfBuffer) {
886894
SizedObj)
887895
return false; // It is in fact safe
888896
}
897+
898+
// Pattern 2:
899+
if (auto *DRE = dyn_cast<DeclRefExpr>(Buf->IgnoreParenImpCasts())) {
900+
ASTContext &Ctx = Finder->getASTContext();
901+
902+
if (auto *CAT = Ctx.getAsConstantArrayType(DRE->getType())) {
903+
Expr::EvalResult ER;
904+
// The array element type must be compatible with `char` otherwise an
905+
// explicit cast will be needed, which will make this check unreachable.
906+
// Therefore, the array extent is same as its' bytewise size.
907+
if (Size->EvaluateAsConstantExpr(ER, Ctx)) {
908+
APSInt EVal = ER.Val.getInt(); // Size must have integer type
909+
910+
return APSInt::compareValues(EVal, APSInt(CAT->getSize(), true)) != 0;
911+
}
912+
}
913+
}
889914
return true; // ptr and size are not in safe pattern
890915
}
891916
} // namespace libc_func_matchers

clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp

+13-1
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,12 @@ void f(char * p, char * q, std::span<char> s, std::span<char> s2) {
8383
sscanf(p, "%s%d", "hello", *p); // expected-warning{{function 'sscanf' is unsafe}}
8484
sscanf_s(p, "%s%d", "hello", *p); // expected-warning{{function 'sscanf_s' is unsafe}}
8585
fprintf((FILE*)p, "%P%d%p%i hello world %32s", *p, *p, p, *p, p); // expected-warning{{function 'fprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}}
86+
87+
char a[10], b[11];
88+
int c[10];
89+
90+
snprintf(a, sizeof(b), "%s", __PRETTY_FUNCTION__); // expected-warning{{function 'snprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
91+
snprintf((char*)c, sizeof(c), "%s", __PRETTY_FUNCTION__); // expected-warning{{function 'snprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
8692
fprintf((FILE*)p, "%P%d%p%i hello world %32s", *p, *p, p, *p, "hello"); // no warn
8793
printf("%s%d", "hello", *p); // no warn
8894
snprintf(s.data(), s.size_bytes(), "%s%d", "hello", *p); // no warn
@@ -93,13 +99,19 @@ void f(char * p, char * q, std::span<char> s, std::span<char> s2) {
9399
__asan_printf();// a printf but no argument, so no warn
94100
}
95101

96-
void v(std::string s1, int *p) {
102+
void safe_examples(std::string s1, int *p) {
97103
snprintf(s1.data(), s1.size_bytes(), "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", p, s1.c_str()); // no warn
98104
snprintf(s1.data(), s1.size_bytes(), s1.c_str(), __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn
99105
printf("%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", p, s1.c_str()); // no warn
100106
printf(s1.c_str(), __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn
101107
fprintf((FILE*)0, "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", p, s1.c_str()); // no warn
102108
fprintf((FILE*)0, s1.c_str(), __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn
109+
110+
char a[10];
111+
112+
snprintf(a, sizeof a, "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn
113+
snprintf(a, sizeof(decltype(a)), "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn
114+
snprintf(a, 10, "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn
103115
}
104116

105117

0 commit comments

Comments
 (0)