NoQ created this revision.
NoQ added reviewers: aaron.ballman, gribozavr2, xazax.hun, jkorous, t-rasmud,
ziqingluo-90, malavikasamak.
Herald added subscribers: steakhal, martong, rnkovacs.
Herald added a project: All.
NoQ requested review of this revision.
This is the initial commit for `-Wunsafe-buffer-usage`, a warning that helps
codebases (especially modern C++ codebases) transition away from buffer
accesses. It's a minimal commit that barely implements anything, mostly adds
skeleton for future work; we have a long road ahead of us.
Backstory in https://discourse.llvm.org/t/rfc-c-buffer-hardening/65734, more
documentation for the proposed programming model in D136811
<https://reviews.llvm.org/D136811>.
I'm putting the actual implementation into `libAnalysis` as it's going to be a
non-trivial analysis - mostly the fixit part where we try to figure out if we
understand a variable's use pattern well enough to suggest a safe
container/view replacement. Some parts of it may eventually prove useful for
any similar fixit machine that tries to change types of variables. More on that
in the next patch.
The interface for the analysis is currently very primitive, the analysis emits
operations on raw buffers it thinks are unsafe. The plan is that it'll also
emit fixit objects, but then the consuming class will figure out how to present
them.
Warning text is currently somewhat lame, it going to improve a lot once we
specialize it for different operations, and once we start emitting fixits we'll
have to rethink it anyway because fixits can't be attached to a specific
operation (but to an entire variable or even group of variables).
The warning is disabled by default.
Repository:
rC Clang
https://reviews.llvm.org/D137346
Files:
clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Analysis/CMakeLists.txt
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -verify %s
+
+void testIncrement(char *p) {
+ ++p; // expected-warning{{unchecked operation on raw buffer in expression}}
+ p++; // expected-warning{{unchecked operation on raw buffer in expression}}
+ --p; // expected-warning{{unchecked operation on raw buffer in expression}}
+ p--; // expected-warning{{unchecked operation on raw buffer in expression}}
+}
Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===================================================================
--- clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -29,6 +29,7 @@
#include "clang/Analysis/Analyses/ReachableCode.h"
#include "clang/Analysis/Analyses/ThreadSafety.h"
#include "clang/Analysis/Analyses/UninitializedValues.h"
+#include "clang/Analysis/Analyses/UnsafeBufferUsage.h"
#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Analysis/CFG.h"
#include "clang/Analysis/CFGStmtMap.h"
@@ -2138,6 +2139,22 @@
} // namespace consumed
} // namespace clang
+//===----------------------------------------------------------------------===//
+// Unsafe buffer usage analysis.
+//===----------------------------------------------------------------------===//
+
+class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
+ Sema &S;
+
+public:
+ UnsafeBufferUsageReporter(Sema &S) : S(S) {}
+
+ void handleUnsafeOperation(const Stmt *Operation) override {
+ S.Diag(Operation->getBeginLoc(), diag::warn_unsafe_buffer_usage);
+ }
+};
+
+
//===----------------------------------------------------------------------===//
// AnalysisBasedWarnings - Worker object used by Sema to execute analysis-based
// warnings on a function, method, or block.
@@ -2430,6 +2447,12 @@
if (S.getLangOpts().CPlusPlus && isNoexcept(FD))
checkThrowInNonThrowingFunc(S, FD, AC);
+ // Emit unsafe buffer usage warnings and fixits.
+ if (!Diags.isIgnored(diag::warn_unsafe_buffer_usage, D->getBeginLoc())) {
+ UnsafeBufferUsageReporter R(S);
+ checkUnsafeBufferUsage(D, R);
+ }
+
// If none of the previous checks caused a CFG build, trigger one here
// for the logical error handler.
if (LogicalErrorHandler::hasActiveDiagnostics(Diags, D->getBeginLoc())) {
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===================================================================
--- /dev/null
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -0,0 +1,79 @@
+//===- UnsafeBufferUsage.cpp - Replace pointers with modern C++ -----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Analysis/Analyses/UnsafeBufferUsage.h"
+#include "llvm/ADT/SmallVector.h"
+
+using namespace llvm;
+using namespace clang;
+using namespace ast_matchers;
+
+namespace {
+// TODO: Better abstractions over gadgets.
+using GadgetList = std::vector<const Stmt *>;
+}
+
+// Scan the function and return a list of gadgets found with provided kits.
+static GadgetList findGadgets(const Decl *D) {
+
+ class GadgetFinderCallback : public MatchFinder::MatchCallback {
+ GadgetList &Output;
+
+ public:
+ GadgetFinderCallback(GadgetList &Output) : Output(Output) {}
+
+ void run(const MatchFinder::MatchResult &Result) override {
+ Output.push_back(Result.Nodes.getNodeAs<Stmt>("root_node"));
+ }
+ };
+
+ GadgetList G;
+ MatchFinder M;
+
+ auto IncrementMatcher = unaryOperator(
+ hasOperatorName("++"),
+ hasUnaryOperand(hasType(pointerType()))
+ );
+ auto DecrementMatcher = unaryOperator(
+ hasOperatorName("--"),
+ hasUnaryOperand(hasType(pointerType()))
+ );
+
+ GadgetFinderCallback CB(G);
+
+ M.addMatcher(
+ stmt(forEachDescendant(
+ stmt(
+ anyOf(
+ IncrementMatcher,
+ DecrementMatcher
+ /* Fill me in! */
+ )
+ // FIXME: Idiomatically there should be a forCallable(equalsNode(D))
+ // here, to make sure that the statement actually belongs to the
+ // function and not to a nested function. However, forCallable uses
+ // ParentMap which can't be used before the AST is fully constructed.
+ // The original problem doesn't sound like it needs ParentMap though,
+ // maybe there's a more direct solution?
+ ).bind("root_node")
+ )), &CB);
+
+ M.match(*D->getBody(), D->getASTContext());
+
+ return G; // NRVO!
+}
+
+void clang::checkUnsafeBufferUsage(const Decl *D,
+ UnsafeBufferUsageHandler &Handler) {
+ assert(D && D->getBody());
+
+ GadgetList G = findGadgets(D);
+ for (const Stmt *S : G) {
+ Handler.handleUnsafeOperation(S);
+ }
+}
Index: clang/lib/Analysis/CMakeLists.txt
===================================================================
--- clang/lib/Analysis/CMakeLists.txt
+++ clang/lib/Analysis/CMakeLists.txt
@@ -32,6 +32,7 @@
ThreadSafetyLogical.cpp
ThreadSafetyTIL.cpp
UninitializedValues.cpp
+ UnsafeBufferUsage.cpp
LINK_LIBS
clangAST
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11726,4 +11726,8 @@
"a randomized struct can only be initialized with a designated initializer">;
def err_cast_from_randomized_struct : Error<
"casting from randomized structure pointer type %0 to %1">;
+
+// Unsafe buffer usage diagnostics.
+def warn_unsafe_buffer_usage : Warning<"unchecked operation on raw buffer in expression">,
+ InGroup<DiagGroup<"unsafe-buffer-usage">>, DefaultIgnore;
} // end of sema component.
Index: clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
===================================================================
--- /dev/null
+++ clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
@@ -0,0 +1,38 @@
+//===- UnsafeBufferUsage.h - Replace pointers with modern C++ ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines an analysis that aids replacing buffer accesses through
+// raw pointers with safer C++ abstractions such as containers and views/spans.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_ANALYSIS_ANALYSES_UNSAFEBUFFERUSAGE_H
+#define LLVM_CLANG_ANALYSIS_ANALYSES_UNSAFEBUFFERUSAGE_H
+
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+namespace clang {
+
+/// The interface that lets the caller handle unsafe buffer usage analysis
+/// results by overriding this class's handle... methods.
+class UnsafeBufferUsageHandler {
+public:
+ UnsafeBufferUsageHandler() = default;
+ virtual ~UnsafeBufferUsageHandler() = default;
+
+ /// Invoked when an unsafe operation over raw pointers is found.
+ virtual void handleUnsafeOperation(const Stmt *Operation) = 0;
+};
+
+// This function invokes the analysis and allows the caller to react to it
+// through the handler class.
+void checkUnsafeBufferUsage(const Decl *D, UnsafeBufferUsageHandler &Handler);
+
+} // end namespace clang
+
+#endif /* LLVM_CLANG_ANALYSIS_ANALYSES_UNSAFEBUFFERUSAGE_H */
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
