Skip to content

Commit

Permalink
[-Wunsafe-buffer-usage] Fix a bug and suppress libc warnings for C fi…
Browse files Browse the repository at this point in the history
…les (#109496)

- Fix a bug in UnsafeBufferUsage.cpp related to casting to PointerType
- Suppress -Wunsafe-buffer-usage-in-libc-call for C files

(rdar://117182250)
ziqingluo-90 authored Sep 22, 2024
1 parent 19ecded commit 090dc77
Showing 4 changed files with 31 additions and 6 deletions.
12 changes: 7 additions & 5 deletions clang/lib/Analysis/UnsafeBufferUsage.cpp
Original file line number Diff line number Diff line change
@@ -250,7 +250,9 @@ AST_MATCHER_P(Stmt, ignoreUnsafeBufferInContainer,

AST_MATCHER_P(Stmt, ignoreUnsafeLibcCall, const UnsafeBufferUsageHandler *,
Handler) {
return Handler->ignoreUnsafeBufferInLibcCall(Node.getBeginLoc());
if (Finder->getASTContext().getLangOpts().CPlusPlus)
return Handler->ignoreUnsafeBufferInLibcCall(Node.getBeginLoc());
return true; /* Only warn about libc calls for C++ */
}

AST_MATCHER_P(CastExpr, castSubExpr, internal::Matcher<Expr>, innerMatcher) {
@@ -784,12 +786,12 @@ AST_MATCHER_P(CallExpr, hasUnsafePrintfStringArg,
return false; // possibly some user-defined printf function

ASTContext &Ctx = Finder->getASTContext();
QualType FristParmTy = FD->getParamDecl(0)->getType();
QualType FirstParmTy = FD->getParamDecl(0)->getType();

if (!FristParmTy->isPointerType())
if (!FirstParmTy->isPointerType())
return false; // possibly some user-defined printf function

QualType FirstPteTy = (cast<PointerType>(FristParmTy))->getPointeeType();
QualType FirstPteTy = FirstParmTy->castAs<PointerType>()->getPointeeType();

if (!Ctx.getFILEType()
.isNull() && //`FILE *` must be in the context if it is fprintf
@@ -865,7 +867,7 @@ AST_MATCHER(CallExpr, hasUnsafeSnprintfBuffer) {
if (!FirstParmTy->isPointerType())
return false; // Not an snprint

QualType FirstPteTy = cast<PointerType>(FirstParmTy)->getPointeeType();
QualType FirstPteTy = FirstParmTy->castAs<PointerType>()->getPointeeType();
const Expr *Buf = Node.getArg(0), *Size = Node.getArg(1);

if (FirstPteTy.isConstQualified() || !Buf->getType()->isPointerType() ||
3 changes: 2 additions & 1 deletion clang/lib/Sema/AnalysisBasedWarnings.cpp
Original file line number Diff line number Diff line change
@@ -2581,7 +2581,8 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
!Diags.isIgnored(diag::warn_unsafe_buffer_variable, SourceLocation()) ||
!Diags.isIgnored(diag::warn_unsafe_buffer_usage_in_container,
SourceLocation()) ||
!Diags.isIgnored(diag::warn_unsafe_buffer_libc_call, SourceLocation())) {
(!Diags.isIgnored(diag::warn_unsafe_buffer_libc_call, SourceLocation()) &&
S.getLangOpts().CPlusPlus /* only warn about libc calls in C++ */)) {
CallableVisitor(CallAnalyzers).TraverseTranslationUnitDecl(TU);
}
}
12 changes: 12 additions & 0 deletions clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage \
// RUN: -verify %s
// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage \
// RUN: -verify %s -x objective-c++
// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage-in-libc-call \
// RUN: -verify %s

@@ -56,6 +58,11 @@ namespace std {
}

void f(char * p, char * q, std::span<char> s, std::span<char> s2) {
typedef FILE * _Nullable aligned_file_ptr_t __attribute__((align_value(64)));
typedef char * _Nullable aligned_char_ptr_t __attribute__((align_value(64)));
aligned_file_ptr_t fp;
aligned_char_ptr_t cp;

memcpy(); // expected-warning{{function 'memcpy' is unsafe}}
std::memcpy(); // expected-warning{{function 'memcpy' is unsafe}}
__builtin_memcpy(p, q, 64); // expected-warning{{function '__builtin_memcpy' is unsafe}}
@@ -71,9 +78,11 @@ void f(char * p, char * q, std::span<char> s, std::span<char> s2) {
printf("%s%d", // expected-warning{{function 'printf' is unsafe}}
p, // expected-note{{string argument is not guaranteed to be null-terminated}} note attached to the unsafe argument
*p);
printf(cp, p, *p); // expected-warning{{function 'printf' is unsafe}} // expected-note{{string argument is not guaranteed to be null-terminated}}
sprintf(q, "%s%d", "hello", *p); // expected-warning{{function 'sprintf' is unsafe}} expected-note{{change to 'snprintf' for explicit bounds checking}}
swprintf(q, "%s%d", "hello", *p); // expected-warning{{function 'swprintf' is unsafe}} expected-note{{change to 'snprintf' for explicit bounds checking}}
snprintf(q, 10, "%s%d", "hello", *p); // expected-warning{{function 'snprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
snprintf(cp, 10, "%s%d", "hello", *p); // expected-warning{{function 'snprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
snprintf(s.data(), s2.size(), "%s%d", "hello", *p); // expected-warning{{function 'snprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
snwprintf(s.data(), s2.size(), "%s%d", "hello", *p); // expected-warning{{function 'snwprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
snwprintf_s( // expected-warning{{function 'snwprintf_s' is unsafe}}
@@ -84,15 +93,18 @@ void f(char * p, char * q, std::span<char> s, std::span<char> s2) {
sscanf(p, "%s%d", "hello", *p); // expected-warning{{function 'sscanf' is unsafe}}
sscanf_s(p, "%s%d", "hello", *p); // expected-warning{{function 'sscanf_s' is unsafe}}
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}}
fprintf(fp, "%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}}
wprintf(L"hello %s", p); // expected-warning{{function 'wprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}}


char a[10], b[11];
int c[10];
std::wstring WS;

snprintf(a, sizeof(b), "%s", __PRETTY_FUNCTION__); // expected-warning{{function 'snprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
snprintf((char*)c, sizeof(c), "%s", __PRETTY_FUNCTION__); // expected-warning{{function 'snprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
fprintf((FILE*)p, "%P%d%p%i hello world %32s", *p, *p, p, *p, "hello"); // no warn
fprintf(fp, "%P%d%p%i hello world %32s", *p, *p, p, *p, "hello"); // no warn
printf("%s%d", "hello", *p); // no warn
snprintf(s.data(), s.size_bytes(), "%s%d", "hello", *p); // no warn
snprintf(s.data(), s.size_bytes(), "%s%d", __PRETTY_FUNCTION__, *p); // no warn
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// RUN: %clang_cc1 -Wunsafe-buffer-usage %s -verify %s -x c
// RUN: %clang_cc1 -Wunsafe-buffer-usage %s -verify %s -x objective-c

void* __asan_memcpy(void *dst,const void *src, unsigned long size);

void f(int *p, int *q) {

__asan_memcpy(p, q, 10); // no libc warn in C
++p[5]; // expected-warning{{unsafe buffer access}}
}

0 comments on commit 090dc77

Please sign in to comment.