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 may cause buffer overflows. It is designed to +aid automatic fixits which would replace such unsafe functions with safe +alternatives, though it can be used even when the fix can't be automated. + +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 has runtime protection against +overflows, assuming hardened libc++, assuming that containers constructed +outside the function are well-formed. For example, function ``bar()`` below +doesn't need an attribute, because assuming buf is well-formed (has size that +fits the original buffer it refers to), hardened libc++ protects this function +from overflowing the buffer: + +.. code-block:: c++ + + void bar(std::span buf) { + for (size_t i = 0; i < buf.size(); ++i) { + buf[i] = i; + } + } + +This corresponds to our safety precaution about keeping buffers "containerized" +in spans for as long as possible. Function ``foo()`` may have internal +consistency, but by accepting a raw buffer it requires the user to unwrap +the span, which is undesirable. + +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. +In such cases the attribute may never be removed. + +In particular, the attribute is NOT an "escape hatch". It does not suppress +the warnings about unsafe operations in the function. Addressing warnings +inside the function is still valuable for internal consistency. + +Attribute ``[[clang::unsafe_buffer_usage]]`` is similar to attribute +[[deprecated]] but it has important differences: + +* Use of a function annotated by such attribute causes ``-Wunsafe-buffer-usage`` + warning to appear, instead of ``-Wdeprecated``, so they can be + enabled/disabled independently as all four combinations make sense; +* The "replacement" parameter of ``[[deprecated]]``, which allows for automatic + fixits when the function has a drop-in replacement, becomes significantly more + powerful and flexible in ``[[clang::unsafe_buffer_usage]]`` where it will allow + non-trivial automatic fixes. + +(TODO: Explain parameters of the attribute, how they aid automatic fixits) + }]; +} + def DiagnoseAsBuiltinDocs : Documentation { let Category = DocCatFunction; let Content = [{ Index: clang/lib/Analysis/UnsafeBufferUsage.cpp =================================================================== --- clang/lib/Analysis/UnsafeBufferUsage.cpp +++ clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -366,6 +366,36 @@ // 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 { + // FIXME: Not implemented yet. Returning {} is safe as it causes the gadget + // to block any attempts to fix variables that it could have otherwise + // claimed as known. + return {}; + } +}; } // namespace namespace { 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,31 @@ +// 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{{unchecked operation on raw buffer in expression}} + deprecatedFunction4(z); // expected-warning{{unchecked operation on raw buffer in expression}} + someFunction(); + + overloading(x); // expected-warning{{unchecked operation on raw buffer in expression}} + overloading(x, size); + overloading(c); +} + +[[clang::unsafe_buffer_usage]] +void overloading(char c[]);