diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index e65c98e4765f6..3f12fdfc197e4 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -366,6 +366,7 @@ isInUnspecifiedUntypedContext(internal::Matcher InnerMatcher) { namespace { +/* TO_UPSTREAM(BoundsSafetyInterop) ON */ // Finds the argument that is passed as dependent count. const Expr *findCountArg(const Expr *Count, const CallExpr *Call) { const auto *DRE = dyn_cast(Count->IgnoreParenImpCasts()); @@ -747,7 +748,7 @@ const Expr *extractExtentFromSubviewDataCall(ASTContext &Context, static bool hasIntegeralConstant(const Expr *E, uint64_t Val, ASTContext &Ctx) { Expr::EvalResult ER; - if (E->EvaluateAsConstantExpr(ER, Ctx)) { + if (E->EvaluateAsInt(ER, Ctx)) { APInt Eval = ER.Val.getInt(); return APInt::isSameValue(Eval, APInt(Eval.getBitWidth(), Val)); @@ -758,7 +759,8 @@ static bool hasIntegeralConstant(const Expr *E, uint64_t Val, ASTContext &Ctx) { // Checks if the argument passed to count-attributed pointer is one of the // following forms: // 0. `NULL/nullptr`, if the argument to dependent count/size is `0`. -// 1. `&var`, if `var` is a variable identifier and the dependent count is `1`. +// 1. `&var`, if `var` is a variable identifier and (1.a.) the dependent count +// is `1`; or (1.b.) the counted_by expression is constant `1` // 2. `&var`, if `var` is a variable identifier and the dependent size is // equivalent to `sizeof(var)`. // 3. `sp.data()` if the argument to dependent count is `sp.size()`. @@ -769,17 +771,58 @@ static bool hasIntegeralConstant(const Expr *E, uint64_t Val, ASTContext &Ctx) { // `constant-array`. // 7. `(T*) constant-array` if the size of `T` equals to one byte and the // argument to dependent size is equivalent to `sizeof(constant-array)`. -bool isCountAttributedPointerArgumentSafe(ASTContext &Context, - const CountAttributedType *CAT, - const CallExpr *Call, - const Expr *Arg) { - const Expr *ArgNoImp = Arg->IgnoreParenImpCasts(); +// +// This function is generalized for two different cases: +// (a). `__counted_by(e)` attributes are annotated for the function being +// called; +// (b). The function being called has no annotations but this analysis +// hardcodes `__counted_by(n)/__sized_by(n)` relation for a pair of +// parameters--pointer `p` and count/size `n`. +// (E.g., the first two parameters of snprintf). +// +// \param PtrArg the argument to the parameter representing a counted_by +// pointer. +// \param CountArg the argument to the parameter representing count/size. Or +// NULL if the count/size cannot be represented by one argument. +// `CountArg` is never NULL for case (b). +// \param CountedByExpr +// the counted_by expression in the annotated bounds attribute +// for case (a); Or NULL for case (b). This parameter indicates +// whether it is case (a) or (b). +// \param CountedByPointerTy +// the pointer type of a counted_by parameter. Note +// this type is NOT necessarily a `CountAttributedType` in case +// (b) because the counted_by relation comes from hardcoded +// knowledge. +// \param isSizedBy +// true iff the counted_by relation is bytewise. +// \param isOrNull +// true iff the counted_by attribute is __counted_by_or_null +// \param DependentValueMap +// A map from variables in the counted_by expression to actual +// arguments in case (a); or NULL for case (b) or failure in +// obtatining the info. +static bool isCountAttributedPointerArgumentSafeImpl( + ASTContext &Context, const Expr *PtrArg, + const Expr * /* Nullable */ CountArg, const Type *CountedByPointerTy, + const Expr * /* Nullable */ CountedByExpr, bool isSizedBy, bool isOrNull, + const DependentValuesTy * /* Nullable */ DependentValueMap = nullptr) { + assert((CountedByExpr || CountArg) && + "Only one of them can be null. If the __counted_by information is " + "hardcoded, there must be an argument representing the actual " + "count."); + assert((CountedByExpr || !DependentValueMap) && + "If the __counted_by information is hardcoded, there is no " + "dependent value map."); + const Expr *PtrArgNoImp = PtrArg->IgnoreParenImpCasts(); // check form 0: - if (ArgNoImp->getType()->isNullPtrType()) { - if (const auto *CountArg = findCountArg(CAT->getCountExpr(), Call)) + if (PtrArgNoImp->getType()->isNullPtrType()) { + if (CountArg) return hasIntegeralConstant(CountArg, 0, Context); - return false; + // When there is no argument representing the count/size, it is safe iff + // the annotation is `__counted_by(0)`. + return hasIntegeralConstant(CountedByExpr, 0, Context); } // check form 1-2: @@ -788,14 +831,17 @@ bool isCountAttributedPointerArgumentSafe(ASTContext &Context, hasUnaryOperand(ignoringParenImpCasts(declRefExpr().bind("VarIdent"))))); if (auto *DRE = selectFirst( - "VarIdent", match(AddressofDRE, *ArgNoImp, Context))) { - if (const auto *CountArg = findCountArg(CAT->getCountExpr(), Call)) { - if (!CAT->isCountInBytes()) // form 1: + "VarIdent", match(AddressofDRE, *PtrArgNoImp, Context))) { + if (CountArg) { + if (!isSizedBy) // form 1.a.: return hasIntegeralConstant(CountArg, 1, Context); // form 2: if (auto TySize = Context.getTypeSizeInCharsIfKnown(DRE->getType())) return hasIntegeralConstant(CountArg, TySize->getQuantity(), Context); - } + } else + // form 1.b.: when there is no argument representing the count/size, it is + // safe iff the annotation is `__counted_by(1)`. + return !isSizedBy && hasIntegeralConstant(CountedByExpr, 1, Context); return false; } @@ -806,45 +852,52 @@ bool isCountAttributedPointerArgumentSafe(ASTContext &Context, }; std::optional ArgTypeSize = getTypeSize( - QualType(ArgNoImp->getType()->getPointeeOrArrayElementType(), 0)); - std::optional ParamTypeSize = getTypeSize(CAT->getPointeeType()); + QualType(PtrArgNoImp->getType()->getPointeeOrArrayElementType(), 0)); + std::optional ParamTypeSize = + getTypeSize(CountedByPointerTy->getPointeeType()); bool ArgInBytes = ArgTypeSize.has_value() && ArgTypeSize->isOne(); - bool ParamInBytes = CAT->isCountInBytes() || - (ParamTypeSize.has_value() && ParamTypeSize->isOne()); + bool ParamInBytes = + isSizedBy || (ParamTypeSize.has_value() && ParamTypeSize->isOne()); // If there is only one dependent count, check for the form 3. - if (const auto *CountArg = findCountArg(CAT->getCountExpr(), Call)) { - if (isValidContainerRange(Context, Arg, CountArg, ArgInBytes, ParamInBytes)) - return true; - } + if (CountArg && isValidContainerRange(Context, PtrArg, CountArg, ArgInBytes, + ParamInBytes)) + return true; // Check forms 4-7. if (ArgInBytes != ParamInBytes) return false; - auto ValuesOpt = getDependentValuesFromCall(CAT, Call); - if (!ValuesOpt.has_value()) + if (!DependentValueMap && CountedByExpr) + // Bail if the map is not available for case (a). Because + // we need the map to substitute parameters to arguments in case (a) but + // not case(b). return false; - const Expr *ArgCount = nullptr; + // the actual count of the pointer inferred through patterns below: + const Expr *ActualCount = nullptr; const Expr *MemberBase = nullptr; - if (const auto *ME = dyn_cast(ArgNoImp)) + if (const auto *ME = dyn_cast(PtrArgNoImp)) MemberBase = ME->getBase(); - if (const Expr *ExtentExpr = extractExtentFromSubviewDataCall(Context, Arg)) { + if (const Expr *ExtentExpr = + extractExtentFromSubviewDataCall(Context, PtrArg)) { // Form 4. - ArgCount = ExtentExpr; + ActualCount = ExtentExpr; } else if (const auto *ArgCAT = - ArgNoImp->getType()->getAs()) { + PtrArgNoImp->getType()->getAs()) { // Form 5. - if (ArgCAT->isOrNull() == CAT->isOrNull()) - ArgCount = ArgCAT->getCountExpr(); + bool areBoundsAttributesCompatible = + (ArgCAT->isCountInBytes() == isSizedBy || (ArgInBytes && ParamInBytes)); + + if (ArgCAT->isOrNull() == isOrNull && areBoundsAttributesCompatible) + ActualCount = ArgCAT->getCountExpr(); } else { // Form 6-7. - const Expr *ArrArg = ArgNoImp; + const Expr *ArrArg = PtrArgNoImp; if (ArgInBytes) if (auto *CE = dyn_cast(ArrArg)) @@ -854,18 +907,47 @@ bool isCountAttributedPointerArgumentSafe(ASTContext &Context, if (const auto *ATy = Context.getAsConstantArrayType(ArrArg->getType())) { APInt TySize = ATy->getSize(); - if (CAT->isCountInBytes()) + if (isSizedBy) if (auto TySizeInChar = Context.getTypeSizeInCharsIfKnown(ATy)) TySize = APInt(TySize.getBitWidth(), TySizeInChar->getQuantity()); - ArgCount = IntegerLiteral::Create(Context, TySize, Context.getSizeType(), - SourceLocation()); + ActualCount = IntegerLiteral::Create( + Context, TySize, Context.getSizeType(), SourceLocation()); } } - if (!ArgCount) + if (!ActualCount) return false; + if (!CountedByExpr) + // In case (b), the actual count of the pointer must match the argument + // passed to the parameter representing the count: + CountedByExpr = CountArg; + return isCompatibleWithCountExpr(ActualCount, CountedByExpr, MemberBase, + DependentValueMap, Context); +} + +// Checks if the argument passed to a count-attributed pointer is safe. This +// function is for the case where the pointer has bounds attributes. This is +// case (a) in `isCountAttributedPointerArgumentSafeImpl`. +static bool isCountAttributedPointerArgumentSafe(ASTContext &Context, + const CountAttributedType *CAT, + const CallExpr *Call, + const Expr *Arg) { + const Expr * /* Nullable */ CountArg = + findCountArg(CAT->getCountExpr(), Call); + auto ValuesOpt = getDependentValuesFromCall(CAT, Call); - return isCompatibleWithCountExpr(ArgCount, CAT->getCountExpr(), MemberBase, - &*ValuesOpt, Context); + return isCountAttributedPointerArgumentSafeImpl( + Context, Arg, CountArg, CAT, CAT->getCountExpr(), CAT->isCountInBytes(), + CAT->isOrNull(), ValuesOpt.has_value() ? &*ValuesOpt : nullptr); +} + +// Checks if arguments passed to a function, which has no annotation but +// hardcoded counted_by knowledge, is safe. This is case (b) in +// `isCountAttributedPointerArgumentSafeImpl`. +static bool isHardcodedCountedByPointerArgumentSafe( + ASTContext &Context, const Expr *PtrArg, const Expr *CountArg, + const Type *PtrParmType, bool isSizedBy, bool isOrNull) { + return isCountAttributedPointerArgumentSafeImpl( + Context, PtrArg, CountArg, PtrParmType, nullptr, isSizedBy, isOrNull); } // Checks if the argument passed to __single pointer is one of the following @@ -919,7 +1001,7 @@ bool isSinglePointerArgumentSafe(ASTContext &Context, const Expr *Arg) { return false; } - +/* TO_UPSTREAM(BoundsSafetyInterop) OFF */ } // namespace // Given a two-param std::span construct call, matches iff the call has the @@ -933,7 +1015,7 @@ bool isSinglePointerArgumentSafe(ASTContext &Context, const Expr *Arg) { // 6. `std::span{p, n}`, where `p` is a __counted_by(`n`)/__sized_by(`n`) // pointer OR `std::span{(char*)p, n}`, where `p` is a __sized_by(`n`) // pointer. - +// 7. `std::span{p, strlen(p)}` or `std::span{p, wcslen(p)}` AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct) { assert(Node.getNumArgs() == 2 && "expecting a two-parameter std::span constructor"); @@ -991,7 +1073,7 @@ AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct) { break; } - QualType Arg0Ty = Arg0->IgnoreImplicit()->getType(); + QualType Arg0Ty = Arg0->getType(); if (auto *ConstArrTy = Finder->getASTContext().getAsConstantArrayType(Arg0Ty)) { @@ -1047,6 +1129,20 @@ AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct) { /*DependentValues=*/nullptr, Finder->getASTContext()); } + // Check form 7: + if (const auto *Call = dyn_cast(Arg1); + Call && Call->getNumArgs() == 1) { + if (const FunctionDecl *FD = Call->getDirectCallee(); + FD && FD->getIdentifier()) { + StringRef FName = FD->getName(); + const Expr *CallArg = Call->getArg(0)->IgnoreParenImpCasts(); + + // TODO: we can re-use `LibcFunNamePrefixSuffixParser` to support more + // variants, e.g., `strlen_s` + return (FName == "strlen" || FName == "wcslen") && + AreSameDRE(Arg0, CallArg); + } + } return false; } @@ -1189,6 +1285,10 @@ static bool isNullTermPointer(const Expr *Ptr, ASTContext &Ctx) { return false; } +AST_MATCHER(Expr, isNullTermPointer) { + return isNullTermPointer(&Node, Finder->getASTContext()); +} + // Return true iff at least one of following cases holds: // 1. Format string is a literal and there is an unsafe pointer argument // corresponding to an `s` specifier; @@ -1546,55 +1646,15 @@ AST_MATCHER(CallExpr, hasUnsafeSnprintfBuffer) { !Size->getType()->isIntegerType()) return false; // not an snprintf call - // Pattern 1: - static StringRef SizedObjs[] = {"span", "array", "vector", - "basic_string_view", "basic_string"}; - Buf = Buf->IgnoreParenImpCasts(); - Size = Size->IgnoreParenImpCasts(); - if (auto *MCEPtr = dyn_cast(Buf)) - if (auto *MCESize = dyn_cast(Size)) { - auto *DREOfPtr = dyn_cast( - MCEPtr->getImplicitObjectArgument()->IgnoreParenImpCasts()); - auto *DREOfSize = dyn_cast( - MCESize->getImplicitObjectArgument()->IgnoreParenImpCasts()); - - if (!DREOfPtr || !DREOfSize) - return true; // not in safe pattern - if (DREOfPtr->getDecl() != DREOfSize->getDecl()) - return true; // not in safe pattern - if (MCEPtr->getMethodDecl()->getName() != "data") - return true; // not in safe pattern - - if (MCESize->getMethodDecl()->getName() == "size_bytes" || - // Note here the pointer must be a pointer-to-char type unless there - // is explicit casting. If there is explicit casting, this branch - // is unreachable. Thus, at this branch "size" and "size_bytes" are - // equivalent as the pointer is a char pointer: - MCESize->getMethodDecl()->getName() == "size") - for (StringRef SizedObj : SizedObjs) - if (MCEPtr->getRecordDecl()->isInStdNamespace() && - MCEPtr->getRecordDecl()->getCanonicalDecl()->getName() == - SizedObj) - return false; // It is in fact safe - } - - // Pattern 2: - if (auto *DRE = dyn_cast(Buf->IgnoreParenImpCasts())) { - ASTContext &Ctx = Finder->getASTContext(); - - if (auto *CAT = Ctx.getAsConstantArrayType(DRE->getType())) { - Expr::EvalResult ER; - // The array element type must be compatible with `char` otherwise an - // explicit cast will be needed, which will make this check unreachable. - // Therefore, the array extent is same as its' bytewise size. - if (Size->EvaluateAsInt(ER, Ctx)) { - APSInt EVal = ER.Val.getInt(); // Size must have integer type - - return APSInt::compareValues(EVal, APSInt(CAT->getSize(), true)) != 0; - } - } + if (std::optional NumChars = + Finder->getASTContext().getTypeSizeInCharsIfKnown(FirstPteTy)) { + Buf = Buf->IgnoreParenImpCasts(); + Size = Size->IgnoreParenImpCasts(); + return !isHardcodedCountedByPointerArgumentSafe( + Finder->getASTContext(), Buf, Size, FirstParmTy.getTypePtr(), + NumChars->isOne(), false); } - return true; // ptr and size are not in safe pattern + return false; } } // namespace libc_func_matchers @@ -2303,9 +2363,11 @@ class UnsafeLibcFunctionCallGadget : public WarningGadget { // safe: functionDecl(libc_func_matchers::isUnsafeSprintfFunc()) .bind(UnsafeSprintfTag)))), - // (unless the call has a sole string literal argument): + // (unless the call has a sole null-terminated argument, e.g., strlen, printf, atoi): unless( - allOf(hasArgument(0, expr(stringLiteral())), hasNumArgs(1)))), + allOf(hasArgument(0, + expr(libc_func_matchers::isNullTermPointer())), + hasNumArgs(1)))), // The following two cases require checking against actual // arguments of the call: diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct-count-attributed-pointer.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct-count-attributed-pointer.cpp index e32223a69dcfd..0e6d0c9941d3b 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct-count-attributed-pointer.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct-count-attributed-pointer.cpp @@ -1,4 +1,5 @@ -// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage-in-container -fexperimental-bounds-safety-attributes -verify %s +// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage-in-container\ +// RUN: -Wno-error=bounds-safety-strict-terminated-by-cast -fexperimental-bounds-safety-attributes -verify %s #include #include @@ -142,3 +143,22 @@ void span_from_output_parm(int * __counted_by(n) *cb_p, size_t n, std::span(sb_z, *l); // no warn std::span(*cb_q, n); // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}} } + +size_t strlen( const char* __null_terminated str ); +size_t wcslen( const wchar_t* __null_terminated str ); + +#pragma clang diagnostic push +#pragma clang diagnostic warning "-Wunsafe-buffer-usage" + +void test_span_ctor(const char * __null_terminated p, + const wchar_t * __null_terminated wp) { + std::span S{p, strlen(p)}; + std::span S2{wp, wcslen(wp)}; +} + +void test_span_ctor_warn(const char * p, + const wchar_t * wp) { + std::span S{p, strlen(p)}; // expected-warning{{passing 'const char *' to parameter of incompatible type 'const char * __terminated_by(0)' (aka 'const char *') is an unsafe operation}} + std::span S2{wp, wcslen(wp)}; // expected-warning{{passing 'const wchar_t *' to parameter of incompatible type 'const wchar_t * __terminated_by(0)' (aka 'const wchar_t *') is an unsafe operation}} +} +#pragma clang diagnostic pop diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-inline-namespace.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-inline-namespace.cpp index 2bd12db93fd52..9ca1e6261de57 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-inline-namespace.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-inline-namespace.cpp @@ -23,7 +23,7 @@ namespace std { T* p; T *c_str(); T *data(); - unsigned size_bytes(); + unsigned size(); }; typedef basic_string string; @@ -50,8 +50,8 @@ void f(char * p, char * q, std::span s) { } void v(std::string s1) { - std::snprintf(s1.data(), s1.size_bytes(), "%s%d", s1.c_str(), 0); // no warn - std::__1::snprintf(s1.data(), s1.size_bytes(), "%s%d", s1.c_str(), 0); // no warn + std::snprintf(s1.data(), s1.size(), "%s%d", s1.c_str(), 0); // no warn + std::__1::snprintf(s1.data(), s1.size(), "%s%d", s1.c_str(), 0); // no warn } void g(char *begin, char *end, char *p, std::span s) { diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp new file mode 100644 index 0000000000000..d600ae1d8ddb3 --- /dev/null +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp @@ -0,0 +1,145 @@ +// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage -Wno-error=bounds-safety-strict-terminated-by-cast\ +// RUN: -verify -fexperimental-bounds-safety-attributes %s +#include +#include + +typedef struct {} FILE; +typedef struct {} va_list; + +namespace std { + template + struct span { + T *data() const noexcept; + size_t size() const noexcept; + size_t size_bytes() const noexcept; + span first(size_t count) const noexcept; + span last(size_t count) const noexcept; + span subspan(size_t offset, size_t count) const noexcept; + const T &operator[](const size_t idx) const; + }; + + template + struct basic_string { + const CharT *data() const noexcept; + CharT *data() noexcept; + const CharT *c_str() const noexcept; + size_t size() const noexcept; + size_t length() const noexcept; + }; + + typedef basic_string string; + typedef basic_string wstring; +} + +namespace annotated_libc { + // For libc functions that have annotations, + // `-Wunsafe-buffer-usage-in-libc-call` yields to the interoperation + // warnings. + +// expected-note@+2{{consider using a safe container and passing '.data()' to the parameter 'dst' and '.size()' to its dependent parameter 'size' or 'std::span' and passing '.first(...).data()' to the parameter 'dst'}} +// expected-note@+1{{consider using a safe container and passing '.data()' to the parameter 'src' and '.size()' to its dependent parameter 'size' or 'std::span' and passing '.first(...).data()' to the parameter 'src'}} +void *memcpy(void * __sized_by(size) dst, const void * __sized_by(size) src, size_t size); +size_t strlen( const char* __null_terminated str ); +// expected-note@+1{{consider using a safe container and passing '.data()' to the parameter 'buffer' and '.size()' to its dependent parameter 'buf_size' or 'std::span' and passing '.first(...).data()' to the parameter 'buffer'}} +int snprintf( char* __counted_by(buf_size) buffer, size_t buf_size, const char* format, ... ); +// expected-note@+1 2{{consider using a safe container and passing '.data()' to the parameter 'buffer' and '.size()' to its dependent parameter 'buf_size' or 'std::span' and passing '.first(...).data()' to the parameter 'buffer'}} +int snwprintf( wchar_t* __counted_by(buf_size) buffer, size_t buf_size, const wchar_t* format, ... ); +int vsnprintf( char* __counted_by(buf_size) buffer, size_t buf_size, const char* format, va_list va_args); + +// The '__counted_by(10)' is not a correct bounds annotation for +// 'sprintf'. It is used to test that even if 'sprintf' has bounds +// annotations, the function will still be warned against as 'sprintf' +// can't be safe. +int sprintf( char* __counted_by(10) buffer, const char* format, ... ); + +void test(char * p, char * q, const char * str, + const char * __null_terminated safe_str, + char * __counted_by(n) safe_p, + size_t n, + char * __counted_by(10) safe_ten) { + memcpy(p, q, 10); // expected-warning2{{unsafe assignment to function parameter of count-attributed type}} + snprintf(p, 10, "%s", "hlo"); // expected-warning{{unsafe assignment to function parameter of count-attributed type}} + + // We still warn about unsafe string pointer arguments to printfs: + snprintf(safe_p, n, "%s", str); // expected-warning{{function 'snprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}} + + memcpy(safe_p, safe_p, n); // no warn + strlen(str); // expected-warning{{passing 'const char *' to parameter of incompatible type 'const char * __terminated_by(0)' (aka 'const char *') is an unsafe operation}} + snprintf(safe_p, n, "%s", "hlo"); // no warn + snprintf(safe_p, n, "%s", safe_str); // no warn + + // v-printf functions and sprintf are still warned about because + // they cannot be fully safe: + va_list vlist; + vsnprintf(safe_p, n, "%s", vlist); // expected-warning{{function 'vsnprintf' is unsafe}} expected-note{{'va_list' is unsafe}} + sprintf(safe_ten, "%s", safe_str); // expected-warning{{function 'sprintf' is unsafe}} expected-note{{change to 'snprintf' for explicit bounds checking}} + +} + +void test_wchar(wchar_t * p, wchar_t * q, const wchar_t * wstr, + const wchar_t * __null_terminated safe_wstr, + wchar_t * __null_terminated nt_wstr, + wchar_t * __counted_by(n) safe_p, + wchar_t * __sized_by(n) sizedby_p, + size_t n) { + std::wstring cxx_wstr; + std::span cxx_wspan; + + snwprintf(safe_p, n, L"%ls", safe_wstr); + snwprintf(cxx_wstr.data(), cxx_wstr.size(), cxx_wstr.c_str()); + snwprintf(cxx_wspan.data(), cxx_wspan.size(), cxx_wspan.data()); // expected-warning{{function 'snwprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}} + snwprintf(p, n, L"%ls", safe_wstr); // expected-warning{{unsafe assignment to function parameter of count-attributed type}} + snwprintf(sizedby_p, n, L"%ls", safe_wstr); // expected-warning{{unsafe assignment to function parameter of count-attributed type}} + snwprintf(safe_p, n, L"%ls", wstr); // expected-warning{{function 'snwprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}} +} +} // namespace annotated_libc + +namespace unannotated_libc { + // The -Wunsafe-buffer-usage analysis considers some printf + // functions safe, arguments are correctly annotated. Because these + // functions are harder to be changed to C++ equivalents. +int printf(const char*, ... ); +int fprintf (FILE*, const char*, ... ); +int snprintf( char*, size_t, const char*, ... ); +int snwprintf( wchar_t*, size_t, const wchar_t*, ... ); +int vsnprintf( char*, size_t, const char*, va_list va_args ); + // It is convenient to accept functions like `strlen` or `atoi` when + // they take a __null_termianted argument. +size_t strlen( const char* ); +int atoi( const char* ); + +void test(const char * __null_terminated safe_str, + char * __sized_by(n) safe_p, + size_t n) { + FILE *file; + printf("%s", safe_str); + fprintf(file, "%s", safe_str); + snprintf(safe_p, n, "%s", safe_str); + strlen(safe_str); + atoi(safe_str); + printf(safe_str); + strlen(safe_p); // safe_p is not null-terminated expected-warning{{function 'strlen' is unsafe}} + + // v-printf functions and sprintf are still warned about because + // they cannot be fully safe: + va_list vlist; + vsnprintf(safe_p, n, "%s", vlist); // expected-warning{{function 'vsnprintf' is unsafe}} expected-note{{'va_list' is unsafe}} +} + +void test_wchar(const wchar_t * unsafe_wstr, + const wchar_t * __null_terminated safe_wstr, + wchar_t * __null_terminated nt_wstr, + wchar_t * __sized_by(n) sizedby_wp, // a 'wchar_t' is larger than a `char` + wchar_t * __counted_by(n) safe_wp, + size_t n) { + std::wstring cxx_wstr; + std::span cxx_wspan; + + snwprintf(cxx_wstr.data(), cxx_wstr.size(), cxx_wstr.c_str()); + snwprintf(safe_wp, n, safe_wstr); + snwprintf(cxx_wspan.data(), cxx_wspan.size(), cxx_wspan.data()); // expected-warning{{function 'snwprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}} + snwprintf(sizedby_wp, n, safe_wstr); // expected-warning{{function 'snwprintf' is unsafe}} expected-note{{buffer pointer and size may not match}} + snwprintf(safe_wp, n, unsafe_wstr); // expected-warning{{function 'snwprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}} + +} +} // namespace unannotated_libc 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 98a4c7bc6943e..15a430b85885a 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp @@ -46,7 +46,7 @@ namespace std { T* p; T *c_str(); T *data(); - unsigned size_bytes(); + unsigned size(); }; typedef basic_string string; @@ -84,7 +84,6 @@ void f(char * p, char * q, std::span s, std::span s2) { 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}} - snprintf(nullptr, 0, ""); // expected-warning{{function 'snprintf' is unsafe}} expected-note{{buffer pointer and size may not match}} printf(nullptr); // expected-warning{{function 'printf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}} printf("%s", nullptr); // expected-warning{{function 'printf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}} snwprintf(s.data(), s2.size(), "%s%d", "hello", *p); // expected-warning{{function 'snwprintf' is unsafe}} expected-note{{buffer pointer and size may not match}} @@ -105,7 +104,8 @@ void f(char * p, char * q, std::span s, std::span s2) { 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}} + snprintf((char*)c, sizeof(c), "%s", __PRETTY_FUNCTION__); + snprintf(nullptr, 0, ""); // no warn 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 @@ -120,8 +120,8 @@ void f(char * p, char * q, std::span s, std::span s2) { } void safe_examples(std::string s1, int *p) { - snprintf(s1.data(), s1.size_bytes(), "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", p, s1.c_str()); // no warn - snprintf(s1.data(), s1.size_bytes(), s1.c_str(), __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn + snprintf(s1.data(), s1.size(), "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", p, s1.c_str()); // no warn + snprintf(s1.data(), s1.size(), s1.c_str(), __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn printf("%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", p, s1.c_str()); // no warn printf(s1.c_str(), __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn fprintf((FILE*)0, "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", p, s1.c_str()); // no warn