Index: clang/docs/ReleaseNotes.rst =================================================================== --- clang/docs/ReleaseNotes.rst +++ clang/docs/ReleaseNotes.rst @@ -86,6 +86,11 @@ Attribute Changes in Clang -------------------------- +Introduced a new function attribute ``__attribute__((unsafe_buffer_usage))`` +to be worn by functions containing buffer operations that could cause out of +bounds memory accesses. It emits warnings at call sites to such functions when +the flag ``-Wunsafe-buffer-usage`` is enabled. + Windows Support --------------- Index: clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def =================================================================== --- clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def +++ clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def @@ -29,6 +29,7 @@ WARNING_GADGET(Decrement) WARNING_GADGET(ArraySubscript) WARNING_GADGET(PointerArithmetic) +WARNING_GADGET(UnsafeBufferUsageAttr) #undef FIXABLE_GADGET #undef WARNING_GADGET Index: clang/include/clang/Basic/Attr.td =================================================================== --- clang/include/clang/Basic/Attr.td +++ clang/include/clang/Basic/Attr.td @@ -3966,6 +3966,12 @@ let Documentation = [ReleaseHandleDocs]; } +def UnsafeBufferUsage : InheritableAttr { + let Spellings = [Clang<"unsafe_buffer_usage">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [UnsafeBufferUsageDocs]; +} + def DiagnoseAsBuiltin : InheritableAttr { let Spellings = [Clang<"diagnose_as_builtin">]; let Args = [DeclArgument, Index: clang/include/clang/Basic/AttrDocs.td =================================================================== --- clang/include/clang/Basic/AttrDocs.td +++ clang/include/clang/Basic/AttrDocs.td @@ -6274,6 +6274,82 @@ }]; } +def UnsafeBufferUsageDocs : Documentation { + let Content = [{ +The attribute ``[[clang::unsafe_buffer_usage]]`` should be placed on functions +that need to be avoided as they are prone to buffer overflows. It is designed to +work together with the off-by-default compiler warning ``-Wunsafe-buffer-usage`` +to help codebases transition away from raw pointer based buffer management, +in favor of safer abstractions such as C++20 ``std::span``. The attribute causes +``-Wunsafe-buffer-usage`` to warn on every use of the function, and it may +enable ``-Wunsafe-buffer-usage`` to emit automatic fix-it hints +which would help the user replace such unsafe functions with safe +alternatives, though the attribute can be used even when the fix can't be automated. + +The attribute does not suppress ``-Wunsafe-buffer-usage`` inside the function +to which it is attached. These warnings still need to be addressed. + +The attribute is warranted even if the only way a function can overflow +the buffer is by violating the function's preconditions. For example, it +would make sense to put the attribute on function ``foo()`` below because +passing an incorrect size parameter would cause a buffer overflow: + +.. code-block:: c++ + [[clang::unsafe_buffer_usage]] + void foo(int *buf, size_t size) { + for (size_t i = 0; i < size; ++i) { + buf[i] = i; + } + } + +The attribute is NOT warranted when the function uses safe abstractions, +assuming that these abstractions weren't misused outside the function. +For example, function ``bar()`` below doesn't need the attribute, +because assuming that the container ``buf`` is well-formed (has size that +fits the original buffer it refers to), overflow cannot occur: + +.. code-block:: c++ + + void bar(std::span buf) { + for (size_t i = 0; i < buf.size(); ++i) { + buf[i] = i; + } + } + +In this case function ``bar()`` enables the user to keep the buffer +"containerized" in a span for as long as possible. On the other hand, +Function ``foo()`` in the previous example may have internal +consistency, but by accepting a raw buffer it requires the user to unwrap +their span, which is undesirable according to the programming model +behind ``-Wunsafe-buffer-usage``. + +The attribute is warranted when a function accepts a raw buffer only to +immediately put it into a span: + +.. code-block:: c++ + + [[clang::unsafe_buffer_usage]] + void baz(int *buf, size_t size) { + std::span sp{ buf, size }; + for (size_t i = 0; i < sp.size(); ++i) { + sp[i] = i; + } + } + +In this case ``baz()`` does not contain any unsafe operations, but the awkward +parameter type causes the caller to unwrap the span unnecessarily. +Note that regardless of the attribute, code inside ``baz()`` isn't flagged +by ``-Wunsafe-buffer-usage`` as unsafe. It is definitely undesirable, +but if ``baz()`` is on an API surface, there is no way to improve it +to make it as safe as ``bar()`` without breaking the source and binary +compatibility with existing users of the function. In such cases +the proper solution would be to create a different function (possibly +an overload of ``baz()``) that accepts a safe container like ``bar()``, +and then use the attribute on the original ``baz()`` to help the users +update their code to use the new function. + }]; +} + def DiagnoseAsBuiltinDocs : Documentation { let Category = DocCatFunction; let Content = [{ Index: clang/include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -11786,7 +11786,7 @@ InGroup, DefaultIgnore; def warn_unsafe_buffer_operation : Warning< "%select{unsafe pointer operation|unsafe pointer arithmetic|" - "unsafe buffer access}0">, + "unsafe buffer access|function introduces unsafe buffer manipulation}0">, InGroup, DefaultIgnore; def note_unsafe_buffer_operation : Note< "used%select{| in pointer arithmetic| in buffer access}0 here">; Index: clang/lib/Analysis/UnsafeBufferUsage.cpp =================================================================== --- clang/lib/Analysis/UnsafeBufferUsage.cpp +++ clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -366,6 +366,33 @@ // FIXME: pointer adding zero should be fine //FIXME: this gadge will need a fix-it }; + + +/// A call of a function or method that performs unchecked buffer operations +/// over one of its pointer parameters. +class UnsafeBufferUsageAttrGadget : public WarningGadget { + constexpr static const char *const OpTag = "call_expr"; + const CallExpr *Op; + +public: + UnsafeBufferUsageAttrGadget(const MatchFinder::MatchResult &Result) + : WarningGadget(Kind::UnsafeBufferUsageAttr), + Op(Result.Nodes.getNodeAs(OpTag)) {} + + static bool classof(const Gadget *G) { + return G->getKind() == Kind::UnsafeBufferUsageAttr; + } + + static Matcher matcher() { + return stmt(callExpr(callee(functionDecl(hasAttr(attr::UnsafeBufferUsage)))) + .bind(OpTag)); + } + const Stmt *getBaseStmt() const override { return Op; } + + DeclUseList getClaimedVarUseSites() const override { + return {}; + } +}; } // namespace namespace { Index: clang/lib/Sema/AnalysisBasedWarnings.cpp =================================================================== --- clang/lib/Sema/AnalysisBasedWarnings.cpp +++ clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2184,6 +2184,9 @@ MsgParam = 1; } } else { + if (const auto *FC = dyn_cast(Operation)) { + MsgParam = 3; + } Loc = Operation->getBeginLoc(); Range = Operation->getSourceRange(); } Index: clang/lib/Sema/SemaDeclAttr.cpp =================================================================== --- clang/lib/Sema/SemaDeclAttr.cpp +++ clang/lib/Sema/SemaDeclAttr.cpp @@ -8446,6 +8446,11 @@ D->addAttr(Attr::Create(S.Context, Argument, AL)); } +template +static void handleUnsafeBufferUsage(Sema &S, Decl *D, const ParsedAttr &AL) { + D->addAttr(Attr::Create(S.Context, AL)); +} + static void handleCFGuardAttr(Sema &S, Decl *D, const ParsedAttr &AL) { // The guard attribute takes a single identifier argument. @@ -9328,6 +9333,10 @@ handleHandleAttr(S, D, AL); break; + case ParsedAttr::AT_UnsafeBufferUsage: + handleUnsafeBufferUsage(S, D, AL); + break; + case ParsedAttr::AT_UseHandle: handleHandleAttr(S, D, AL); break; Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test =================================================================== --- clang/test/Misc/pragma-attribute-supported-attributes-list.test +++ clang/test/Misc/pragma-attribute-supported-attributes-list.test @@ -187,6 +187,7 @@ // CHECK-NEXT: TestTypestate (SubjectMatchRule_function_is_member) // CHECK-NEXT: TrivialABI (SubjectMatchRule_record) // CHECK-NEXT: Uninitialized (SubjectMatchRule_variable_is_local) +// CHECK-NEXT: UnsafeBufferUsage (SubjectMatchRule_function) // CHECK-NEXT: UseHandle (SubjectMatchRule_variable_is_parameter) // CHECK-NEXT: VecReturn (SubjectMatchRule_record) // CHECK-NEXT: VecTypeHint (SubjectMatchRule_function) Index: clang/test/SemaCXX/attr-unsafe-buffer-usage.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/attr-unsafe-buffer-usage.cpp @@ -0,0 +1,6 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +// Function annotations. +[[clang::unsafe_buffer_usage]] +void f(int *buf, int size); +void g(int *buffer [[clang::unsafe_buffer_usage("buffer")]], int size); // expected-warning {{'unsafe_buffer_usage' attribute only applies to functions}} Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-function-attr.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-function-attr.cpp @@ -0,0 +1,86 @@ +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -verify %s + +[[clang::unsafe_buffer_usage]] +void deprecatedFunction3(); + +void deprecatedFunction4(int z); + +void someFunction(); + +[[clang::unsafe_buffer_usage]] +void overloading(int* x); + +void overloading(char c[]); + +void overloading(int* x, int size); + +[[clang::unsafe_buffer_usage]] +void deprecatedFunction4(int z); + +void caller(int z, int* x, int size, char c[]) { + deprecatedFunction3(); // expected-warning{{function introduces unsafe buffer manipulation}} + deprecatedFunction4(z); // expected-warning{{function introduces unsafe buffer manipulation}} + someFunction(); + + overloading(x); // expected-warning{{function introduces unsafe buffer manipulation}} + overloading(x, size); + overloading(c); +} + +[[clang::unsafe_buffer_usage]] +void overloading(char c[]); + +// Test variadics +[[clang::unsafe_buffer_usage]] +void testVariadics(int *ptr, ...); + +template +[[clang::unsafe_buffer_usage]] +T adder(T first, Args... args); + +template +void foo(T t) {} + +template<> +[[clang::unsafe_buffer_usage]] +void foo(int *t) {} + +void caller1(int *p, int *q) { + testVariadics(p, q); // expected-warning{{function introduces unsafe buffer manipulation}} + adder(p, q); // expected-warning{{function introduces unsafe buffer manipulation}} + + int x; + foo(x); + foo(&x); // expected-warning{{function introduces unsafe buffer manipulation}} +} + +// Test virtual functions +class BaseClass { +public: + [[clang::unsafe_buffer_usage]] + virtual void func() {} + + virtual void func1() {} +}; + +class DerivedClass : public BaseClass { +public: + void func() {} + + [[clang::unsafe_buffer_usage]] + void func1() {} +}; + +void testInheritance() { + DerivedClass DC; + DC.func(); + DC.func1(); // expected-warning{{function introduces unsafe buffer manipulation}} + + BaseClass *BC; + BC->func(); // expected-warning{{function introduces unsafe buffer manipulation}} + BC->func1(); + + BC = &DC; + BC->func(); // expected-warning{{function introduces unsafe buffer manipulation}} + BC->func1(); +}