[clang] acc3cc6 - [-Wunsafe-buffer-usage] Introduce the unsafe_buffer_usage attribute
Rashmi Mudduluru via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 31 11:45:13 PST 2023
Author: Rashmi Mudduluru
Date: 2023-01-31T11:43:34-08:00
New Revision: acc3cc69e4d1c8e199fde51798a5a2a6edb35796
URL: https://github.com/llvm/llvm-project/commit/acc3cc69e4d1c8e199fde51798a5a2a6edb35796
DIFF: https://github.com/llvm/llvm-project/commit/acc3cc69e4d1c8e199fde51798a5a2a6edb35796.diff
LOG: [-Wunsafe-buffer-usage] Introduce the unsafe_buffer_usage attribute
Differential Revision: https://reviews.llvm.org/D138940
Added:
clang/test/SemaCXX/attr-unsafe-buffer-usage.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-function-attr.cpp
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/Misc/pragma-attribute-supported-attributes-list.test
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0a1197044a71a..c6139252e0c34 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -89,6 +89,11 @@ New Pragmas in Clang
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
---------------
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
index d10d95e5b1ba7..78889da32b3b4 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -29,6 +29,7 @@ WARNING_GADGET(Increment)
WARNING_GADGET(Decrement)
WARNING_GADGET(ArraySubscript)
WARNING_GADGET(PointerArithmetic)
+WARNING_GADGET(UnsafeBufferUsageAttr)
#undef FIXABLE_GADGET
#undef WARNING_GADGET
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index d449a2fe7f8f7..fc2c7f7e37f45 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -3966,6 +3966,12 @@ def ReleaseHandle : InheritableParamAttr {
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<Function, "Function">,
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 6d7a3ffd2d52c..b0b11e85ab6ad 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -6274,6 +6274,82 @@ attribute requires a string literal argument to identify the handle being releas
}];
}
+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<int> 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<int> 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
diff erent 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 = [{
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index b7052bb4b27f9..6d6f474f6dcda 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11786,7 +11786,7 @@ def warn_unsafe_buffer_variable : Warning<
InGroup<UnsafeBufferUsage>, 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<UnsafeBufferUsage>, DefaultIgnore;
def note_unsafe_buffer_operation : Note<
"used%select{| in pointer arithmetic| in buffer access}0 here">;
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 54e3fb17efe45..331479a80f6cc 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -366,6 +366,33 @@ class PointerArithmeticGadget : public WarningGadget {
// 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<CallExpr>(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 {
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 4530154ac9447..cf8f3d928b0b3 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2184,6 +2184,9 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
MsgParam = 1;
}
} else {
+ if (const auto *FC = dyn_cast<CallExpr>(Operation)) {
+ MsgParam = 3;
+ }
Loc = Operation->getBeginLoc();
Range = Operation->getSourceRange();
}
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index a303c7f572802..1a0bfb3d91bcc 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -8446,6 +8446,11 @@ static void handleHandleAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
D->addAttr(Attr::Create(S.Context, Argument, AL));
}
+template<typename Attr>
+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 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
handleHandleAttr<ReleaseHandleAttr>(S, D, AL);
break;
+ case ParsedAttr::AT_UnsafeBufferUsage:
+ handleUnsafeBufferUsage<UnsafeBufferUsageAttr>(S, D, AL);
+ break;
+
case ParsedAttr::AT_UseHandle:
handleHandleAttr<UseHandleAttr>(S, D, AL);
break;
diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
index 5d4a9fc01ef7f..98cdf82ba0c11 100644
--- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ b/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)
diff --git a/clang/test/SemaCXX/attr-unsafe-buffer-usage.cpp b/clang/test/SemaCXX/attr-unsafe-buffer-usage.cpp
new file mode 100644
index 0000000000000..91ba446663383
--- /dev/null
+++ b/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}}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-function-attr.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-function-attr.cpp
new file mode 100644
index 0000000000000..85043510c23aa
--- /dev/null
+++ b/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<typename T, typename... Args>
+[[clang::unsafe_buffer_usage]]
+T adder(T first, Args... args);
+
+template <typename T>
+void foo(T t) {}
+
+template<>
+[[clang::unsafe_buffer_usage]]
+void foo<int *>(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();
+}
More information about the cfe-commits
mailing list