Skip to content

Commit

Permalink
[-Wunsafe-buffer-usage] Introduce std::array fixits (#80084)
Browse files Browse the repository at this point in the history
Array subscript on a const size array is not bounds-checked. The idiomatic
replacement is std::array which is bounds-safe in hardened mode of libc++.

This commit extends the fixit-producing machine to consider std::array as a
transformation target type and teaches it to handle the array subscript on const
size arrays with a trivial (empty) fixit.
  • Loading branch information
jkorous-apple committed Feb 12, 2024
1 parent fac6d3d commit 644ac2a
Show file tree
Hide file tree
Showing 8 changed files with 536 additions and 157 deletions.
45 changes: 42 additions & 3 deletions clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,43 @@ class VariableGroupsManager {
virtual VarGrpRef getGroupOfParms() const =0;
};

// FixitStrategy is a map from variables to the way we plan to emit fixes for
// these variables. It is figured out gradually by trying different fixes
// for different variables depending on gadgets in which these variables
// participate.
class FixitStrategy {
public:
enum class Kind {
Wontfix, // We don't plan to emit a fixit for this variable.
Span, // We recommend replacing the variable with std::span.
Iterator, // We recommend replacing the variable with std::span::iterator.
Array, // We recommend replacing the variable with std::array.
Vector // We recommend replacing the variable with std::vector.
};

private:
using MapTy = llvm::DenseMap<const VarDecl *, Kind>;

MapTy Map;

public:
FixitStrategy() = default;
FixitStrategy(const FixitStrategy &) = delete; // Let's avoid copies.
FixitStrategy &operator=(const FixitStrategy &) = delete;
FixitStrategy(FixitStrategy &&) = default;
FixitStrategy &operator=(FixitStrategy &&) = default;

void set(const VarDecl *VD, Kind K) { Map[VD] = K; }

Kind lookup(const VarDecl *VD) const {
auto I = Map.find(VD);
if (I == Map.end())
return Kind::Wontfix;

return I->second;
}
};

/// The interface that lets the caller handle unsafe buffer usage analysis
/// results by overriding this class's handle... methods.
class UnsafeBufferUsageHandler {
Expand Down Expand Up @@ -75,9 +112,11 @@ class UnsafeBufferUsageHandler {
///
/// `D` is the declaration of the callable under analysis that owns `Variable`
/// and all of its group mates.
virtual void handleUnsafeVariableGroup(const VarDecl *Variable,
const VariableGroupsManager &VarGrpMgr,
FixItList &&Fixes, const Decl *D) = 0;
virtual void
handleUnsafeVariableGroup(const VarDecl *Variable,
const VariableGroupsManager &VarGrpMgr,
FixItList &&Fixes, const Decl *D,
const FixitStrategy &VarTargetTypes) = 0;

#ifndef NDEBUG
public:
Expand Down
4 changes: 2 additions & 2 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -12128,9 +12128,9 @@ def warn_unsafe_buffer_operation : Warning<
def note_unsafe_buffer_operation : Note<
"used%select{| in pointer arithmetic| in buffer access}0 here">;
def note_unsafe_buffer_variable_fixit_group : Note<
"change type of %0 to '%select{std::span|std::array|std::span::iterator}1' to preserve bounds information%select{|, and change %2 to '%select{std::span|std::array|std::span::iterator}1' to propagate bounds information between them}3">;
"change type of %0 to '%select{std::span' to preserve bounds information|std::array' to label it for hardening|std::span::iterator' to preserve bounds information}1%select{|, and change %2 to '%select{std::span|std::array|std::span::iterator}1' to propagate bounds information between them}3">;
def note_unsafe_buffer_variable_fixit_together : Note<
"change type of %0 to '%select{std::span|std::array|std::span::iterator}1' to preserve bounds information"
"change type of %0 to '%select{std::span' to preserve bounds information|std::array' to label it for hardening|std::span::iterator' to preserve bounds information}1"
"%select{|, and change %2 to safe types to make function %4 bounds-safe}3">;
def note_safe_buffer_usage_suggestions_disabled : Note<
"pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions">;
Expand Down

0 comments on commit 644ac2a

Please sign in to comment.