diff --git a/clang/docs/AutomaticReferenceCounting.rst b/clang/docs/AutomaticReferenceCounting.rst --- a/clang/docs/AutomaticReferenceCounting.rst +++ b/clang/docs/AutomaticReferenceCounting.rst @@ -635,7 +635,7 @@ For loads from ``const`` global variables of :ref:`C retainable pointer type `, it is reasonable to assume that global system - constants were initialitzed with true constants (e.g. string literals), but + constants were initialized with true constants (e.g. string literals), but user constants might have been initialized with something dynamically allocated, using a global initializer. diff --git a/clang/docs/ConstantInterpreter.rst b/clang/docs/ConstantInterpreter.rst --- a/clang/docs/ConstantInterpreter.rst +++ b/clang/docs/ConstantInterpreter.rst @@ -81,7 +81,7 @@ * ``PT_VoidPtr`` - Void pointer type, can be used for rount-trip casts. Represented as + Void pointer type, can be used for round-trip casts. Represented as the union of all pointers which can be cast to void. Defined in ``"VoidPointer.h"``. diff --git a/clang/docs/CrossCompilation.rst b/clang/docs/CrossCompilation.rst --- a/clang/docs/CrossCompilation.rst +++ b/clang/docs/CrossCompilation.rst @@ -97,7 +97,7 @@ that Clang doesn't know, like ``blerg``, it'll ignore and assume ``unknown``, which is not always desired, so be careful. -Finally, the env (enviornment) option is something that will pick default +Finally, the env (environment) option is something that will pick default CPU/FPU, define the specific behaviour of your code (PCS, extensions), and also choose the correct library calls, etc. diff --git a/clang/docs/DataFlowAnalysisIntro.md b/clang/docs/DataFlowAnalysisIntro.md --- a/clang/docs/DataFlowAnalysisIntro.md +++ b/clang/docs/DataFlowAnalysisIntro.md @@ -219,7 +219,7 @@ this fact as `⊤`. * When two control flow paths join, we compute the set union of incoming - values (limiting the number of elements to 3, representig larger sets as + values (limiting the number of elements to 3, representing larger sets as `⊤`). The sets of possible values are influenced by: @@ -332,7 +332,7 @@ We can't say what specific value gets printed, but we know that it is either `x` or `-x`. -Dataflow analysis is an istance of abstract interpretation, and does not dictate +Dataflow analysis is an instance of abstract interpretation, and does not dictate how exactly the lattice and transfer functions should be designed, beyond the necessary conditions for the analysis to converge. Nevertheless, we can use symbolic execution ideas to guide our design of the lattice and transfer @@ -634,7 +634,7 @@ For this purpose we can use lattice in a form of a mapping from variable declarations to initialization states; each initialization state is represented -by the followingn lattice: +by the following lattice: ![Lattice for definitive initialization analysis](DataFlowAnalysisIntroImages/DefinitiveInitializationLattice.svg) diff --git a/clang/docs/DebuggingCoroutines.rst b/clang/docs/DebuggingCoroutines.rst --- a/clang/docs/DebuggingCoroutines.rst +++ b/clang/docs/DebuggingCoroutines.rst @@ -620,7 +620,7 @@ $ clang++ -std=c++20 -g debugging-example.cpp -o debugging-example $ gdb ./debugging-example - (gdb) # We've alreay set the breakpoint. + (gdb) # We've already set the breakpoint. (gdb) r Program received signal SIGTRAP, Trace/breakpoint trap. detail::chain_fn<0> () at debugging-example2.cpp:73 diff --git a/clang/docs/analyzer/developer-docs/nullability.rst b/clang/docs/analyzer/developer-docs/nullability.rst --- a/clang/docs/analyzer/developer-docs/nullability.rst +++ b/clang/docs/analyzer/developer-docs/nullability.rst @@ -18,7 +18,7 @@ Taking a branch on nullable pointers are the same like taking branch on null unspecified pointers. -Explicit cast from nullable to nonnul: +Explicit cast from nullable to nonnull: .. code-block:: cpp diff --git a/clang/include/clang/AST/CXXInheritance.h b/clang/include/clang/AST/CXXInheritance.h --- a/clang/include/clang/AST/CXXInheritance.h +++ b/clang/include/clang/AST/CXXInheritance.h @@ -315,7 +315,7 @@ /// virtual function; in abstract classes, the final overrider for at /// least one virtual function is a pure virtual function. Due to /// multiple, virtual inheritance, it is possible for a class to have -/// more than one final overrider. Athough this is an error (per C++ +/// more than one final overrider. Although this is an error (per C++ /// [class.virtual]p2), it is not considered an error here: the final /// overrider map can represent multiple final overriders for a /// method, and it is up to the client to determine whether they are diff --git a/clang/include/clang/AST/CommentSema.h b/clang/include/clang/AST/CommentSema.h --- a/clang/include/clang/AST/CommentSema.h +++ b/clang/include/clang/AST/CommentSema.h @@ -193,7 +193,7 @@ void checkContainerDecl(const BlockCommandComment *Comment); /// Resolve parameter names to parameter indexes in function declaration. - /// Emit diagnostics about unknown parametrs. + /// Emit diagnostics about unknown parameters. void resolveParamCommandIndexes(const FullComment *FC); /// \returns \c true if the declaration that this comment is attached to diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -2168,7 +2168,7 @@ /// declaration to the declaration that is a definition (if there is one). /// /// \param CheckForPendingFriendDefinition If \c true, also check for friend - /// declarations that were instantiataed from function definitions. + /// declarations that were instantiated from function definitions. /// Such a declaration behaves as if it is a definition for the /// purpose of redefinition checking, but isn't actually a "real" /// definition until its body is instantiated. diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h --- a/clang/include/clang/AST/DeclBase.h +++ b/clang/include/clang/AST/DeclBase.h @@ -1601,7 +1601,7 @@ uint64_t : NumDeclContextBits; /// Kind of initializer, - /// function call or omp_priv initializtion. + /// function call or omp_priv initialization. uint64_t InitializerKind : 2; }; diff --git a/clang/include/clang/AST/DeclTemplate.h b/clang/include/clang/AST/DeclTemplate.h --- a/clang/include/clang/AST/DeclTemplate.h +++ b/clang/include/clang/AST/DeclTemplate.h @@ -850,7 +850,7 @@ /// template. /// /// This pointer refers to the template arguments (there are as - /// many template arguments as template parameaters) for the + /// many template arguments as template parameters) for the /// template, and is allocated lazily, since most templates do not /// require the use of this information. TemplateArgument *InjectedArgs = nullptr; diff --git a/clang/include/clang/AST/DeclarationName.h b/clang/include/clang/AST/DeclarationName.h --- a/clang/include/clang/AST/DeclarationName.h +++ b/clang/include/clang/AST/DeclarationName.h @@ -763,7 +763,7 @@ }; /// DeclarationNameInfo - A collector data type for bundling together -/// a DeclarationName and the correspnding source/type location info. +/// a DeclarationName and the corresponding source/type location info. struct DeclarationNameInfo { private: /// Name - The declaration name, also encoding name kind. diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -135,8 +135,8 @@ void setDependence(ExprDependence Deps) { ExprBits.Dependent = static_cast(Deps); } - friend class ASTImporter; // Sets dependence dircetly. - friend class ASTStmtReader; // Sets dependence dircetly. + friend class ASTImporter; // Sets dependence directly. + friend class ASTStmtReader; // Sets dependence directly. public: QualType getType() const { return TR; } @@ -171,7 +171,7 @@ } /// Determines whether the type of this expression depends on - /// - a template paramter (C++ [temp.dep.expr], which means that its type + /// - a template parameter (C++ [temp.dep.expr], which means that its type /// could change from one template instantiation to the next) /// - or an error /// @@ -820,7 +820,7 @@ /// member expression. static QualType findBoundMemberType(const Expr *expr); - /// Skip past any invisble AST nodes which might surround this + /// Skip past any invisible AST nodes which might surround this /// statement, such as ExprWithCleanups or ImplicitCastExpr nodes, /// but also injected CXXMemberExpr and CXXConstructExpr which represent /// implicit conversions. @@ -924,7 +924,7 @@ return const_cast(this)->IgnoreParenLValueCasts(); } - /// Skip past any parenthese and casts which do not change the value + /// Skip past any parentheses and casts which do not change the value /// (including ptr->int casts of the same size) until reaching a fixed point. /// Skips: /// * What IgnoreParens() skips @@ -2815,7 +2815,7 @@ /// The number of arguments in the call expression. unsigned NumArgs; - /// The location of the right parenthese. This has a different meaning for + /// The location of the right parentheses. This has a different meaning for /// the derived classes of CallExpr. SourceLocation RParenLoc; diff --git a/clang/include/clang/Analysis/Analyses/CalledOnceCheck.h b/clang/include/clang/Analysis/Analyses/CalledOnceCheck.h --- a/clang/include/clang/Analysis/Analyses/CalledOnceCheck.h +++ b/clang/include/clang/Analysis/Analyses/CalledOnceCheck.h @@ -28,7 +28,7 @@ /// \enum IfThen -- then branch of the if statement has no call. /// \enum IfElse -- else branch of the if statement has no call. /// \enum Switch -- one of the switch cases doesn't have a call. -/// \enum SwitchSkipped -- there is no call if none of the cases appies. +/// \enum SwitchSkipped -- there is no call if none of the cases applies. /// \enum LoopEntered -- no call when the loop is entered. /// \enum LoopSkipped -- no call when the loop is not entered. /// \enum FallbackReason -- fallback case when we were not able to figure out diff --git a/clang/include/clang/Analysis/Analyses/Consumed.h b/clang/include/clang/Analysis/Analyses/Consumed.h --- a/clang/include/clang/Analysis/Analyses/Consumed.h +++ b/clang/include/clang/Analysis/Analyses/Consumed.h @@ -258,7 +258,7 @@ /// Check a function's CFG for consumed violations. /// /// We traverse the blocks in the CFG, keeping track of the state of each - /// value who's type has uniquness annotations. If methods are invoked in + /// value who's type has uniqueness annotations. If methods are invoked in /// the wrong state a warning is issued. Each block in the CFG is traversed /// exactly once. void run(AnalysisDeclContext &AC); diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h b/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h --- a/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h +++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h @@ -1806,7 +1806,7 @@ }; /// An if-then-else expression. -/// This is a pseduo-term; it will be lowered to a branch in a CFG. +/// This is a pseudo-term; it will be lowered to a branch in a CFG. class IfThenElse : public SExpr { public: IfThenElse(SExpr *C, SExpr *T, SExpr *E) @@ -1851,7 +1851,7 @@ }; /// A let-expression, e.g. let x=t; u. -/// This is a pseduo-term; it will be lowered to instructions in a CFG. +/// This is a pseudo-term; it will be lowered to instructions in a CFG. class Let : public SExpr { public: Let(Variable *Vd, SExpr *Bd) : SExpr(COP_Let), VarDecl(Vd), Body(Bd) { diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def --- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def +++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def @@ -32,6 +32,7 @@ WARNING_GADGET(UnsafeBufferUsageAttr) FIXABLE_GADGET(ULCArraySubscript) FIXABLE_GADGET(DerefSimplePtrArithFixable) +FIXABLE_GADGET(PointerDereference) #undef FIXABLE_GADGET #undef WARNING_GADGET diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -463,6 +463,45 @@ return {}; } }; + +class PointerDereferenceGadget : public FixableGadget { + static constexpr const char *const BaseDeclRefExprTag = "BaseDRE"; + static constexpr const char *const OperatorTag = "op"; + + const DeclRefExpr *BaseDeclRefExpr = nullptr; + const UnaryOperator *Op = nullptr; + +public: + PointerDereferenceGadget(const MatchFinder::MatchResult &Result) + : FixableGadget(Kind::PointerDereference), + BaseDeclRefExpr( + Result.Nodes.getNodeAs(BaseDeclRefExprTag)), + Op(Result.Nodes.getNodeAs(OperatorTag)) {} + + static bool classof(const Gadget *G) { + return G->getKind() == Kind::PointerDereference; + } + + static Matcher matcher() { + auto Target = + unaryOperator( + hasOperatorName("*"), + has(expr(ignoringParenImpCasts( + declRefExpr(to(varDecl())).bind(BaseDeclRefExprTag))))) + .bind(OperatorTag); + + return expr(isInUnspecifiedLvalueContext(Target)); + } + + DeclUseList getClaimedVarUseSites() const override { + return {BaseDeclRefExpr}; + } + + virtual const Stmt *getBaseStmt() const final { return Op; } + + virtual std::optional getFixits(const Strategy &S) const override; +}; + } // namespace namespace { @@ -914,6 +953,37 @@ return std::nullopt; // something wrong or unsupported, give up } +std::optional +PointerDereferenceGadget::getFixits(const Strategy &S) const { + const VarDecl *VD = cast(BaseDeclRefExpr->getDecl()); + switch (S.lookup(VD)) { + case Strategy::Kind::Span: { + ASTContext &Ctx = VD->getASTContext(); + SourceManager &SM = Ctx.getSourceManager(); + // Required changes: *(ptr); => (ptr[0]); and *ptr; => ptr[0] + // Deletes the *operand + CharSourceRange derefRange = clang::CharSourceRange::getCharRange( + Op->getBeginLoc(), Op->getBeginLoc().getLocWithOffset(1)); + // Inserts the [0] + std::optional endOfOperand = + getEndCharLoc(BaseDeclRefExpr, SM, Ctx.getLangOpts()); + if (endOfOperand) { + return FixItList{{FixItHint::CreateRemoval(derefRange), + FixItHint::CreateInsertion( + endOfOperand.value().getLocWithOffset(1), "[0]")}}; + } + } + case Strategy::Kind::Iterator: + case Strategy::Kind::Array: + case Strategy::Kind::Vector: + llvm_unreachable("Strategy not implemented yet!"); + case Strategy::Kind::Wontfix: + llvm_unreachable("Invalid strategy!"); + } + + return std::nullopt; +} + // For a non-null initializer `Init` of `T *` type, this function returns // `FixItHint`s producing a list initializer `{Init, S}` as a part of a fix-it // to output stream. diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-deref.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-deref.cpp new file mode 100644 --- /dev/null +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-deref.cpp @@ -0,0 +1,55 @@ +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s + +void basic_dereference() { + int tmp; + auto p = new int[10]; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span p" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}" + tmp = p[5]; + int val = *p; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:14}:"" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:15-[[@LINE-2]]:15}:"[0]" +} + +int return_method() { + auto p = new int[10]; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span p" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}" + int tmp = p[5]; + return *p; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:11}:"" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"[0]" +} + +void foo(int v) { +} + +void method_invocation() { + auto p = new int[10]; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span p" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}" + + int tmp = p[5]; + + foo(*p); + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:"" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:9-[[@LINE-2]]:9}:"[0]" +} + +void binary_operation() { + auto p = new int[10]; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span p" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}" + + int tmp = p[5]; + + int k = *p + 20; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:12}:"" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:"[0]" + +} +