From 0031494ff09f9eb166a1aed08b7976a744a35888 Mon Sep 17 00:00:00 2001 From: Ziqing Luo Date: Wed, 11 Dec 2024 10:57:30 -0800 Subject: [PATCH] [BoundsSafety][-Wunsafe-buffer-usage] Add a safe pattern of span construction from bounds-attributed pointers Cherry-picked from downstream. (rdar://141103910) --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 24 +++++++++++++++-- ...pan-construct-count-attributed-pointer.cpp | 26 +++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index a02a01076bf981e..cbd77a01c3f7c7b 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -847,6 +847,7 @@ bool isCountAttributedPointerArgumentSafe(ASTContext &Context, // `n` // 5. `std::span{any, 0}` // 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. AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct) { assert(Node.getNumArgs() == 2 && @@ -916,9 +917,28 @@ AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct) { } // Check form 6: + bool isArg0CastToBytePtrType = false; + + if (auto *CE = dyn_cast(Arg0)) { + if (auto DestTySize = Finder->getASTContext().getTypeSizeInCharsIfKnown( + Arg0Ty->getPointeeType())) { + if (!DestTySize->isOne()) + return false; // If the destination pointee type is NOT of one byte + // size, pattern match fails. + Arg0 = CE->getSubExpr()->IgnoreParenImpCasts(); + Arg0Ty = Arg0->getType(); + isArg0CastToBytePtrType = true; + } + } + // Check pointer and count/size with respect to the count-attribute: if (const auto *CAT = Arg0Ty->getAs()) { - // Accept __sized_by() if the size of the pointee type is 1. - if (CAT->isCountInBytes()) { + // For the pattern of `std::span{(char *) p, n}`, p must NOT be a + // __counted_by pointer. + if (!CAT->isCountInBytes() && isArg0CastToBytePtrType) + return false; + // If `Arg0` is not a cast and is a sized_by pointer, its pointee type size + // must be one byte: + if (CAT->isCountInBytes() && !isArg0CastToBytePtrType) { std::optional SizeOpt = Finder->getASTContext().getTypeSizeInCharsIfKnown( CAT->getPointeeType()); 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 39802a1200d14e3..e32223a69dcfd36 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 @@ -102,6 +102,32 @@ void span_from_sb_int(sb_int *i, sb_char *c) { std::span{c->p, c->size}; } +void span_from_sb_void(void * __sized_by(n) p, size_t n) { + std::span{(char *) p, n}; + std::span{(unsigned char *) p, n}; + std::span{(int *) p, n}; // expected-warning{{the two-parameter std::span construction is unsafe}} + std::span{(char *) p, n+1}; // expected-warning{{the two-parameter std::span construction is unsafe}} +} + +class SpanFromSbVoidMemberTest { + void * __sized_by(n) p; + size_t n; + + void test() { + std::span{(char *) p, n}; + std::span{(unsigned char *) p, n}; + std::span{(int *) p, n}; // expected-warning{{the two-parameter std::span construction is unsafe}} + std::span{(char *) p, n+1}; // expected-warning{{the two-parameter std::span construction is unsafe}} + } + + void test(SpanFromSbVoidMemberTest t) { + std::span{(char *) t.p, t.n}; + std::span{(unsigned char *) t.p, t.n}; + std::span{(int *) t.p, t.n}; // expected-warning{{the two-parameter std::span construction is unsafe}} + std::span{(char *) t.p, t.n+1}; // expected-warning{{the two-parameter std::span construction is unsafe}} + } +}; + void span_from_output_parm(int * __counted_by(n) *cb_p, size_t n, int * __counted_by(*m) *cb_q, size_t *m, int * __counted_by(*l) cb_z, size_t *l,