Merge pull request #5840 from ziqingluo-90/stable/20221013-cherrypick-2

static analysis related cherry-picks to stable/20221013 #2

Author: ziqingluo-90

clang-tools-extra/clang-doc/Generators.cpp

@@ -65,7 +65,7 @@ void Generator::addInfoToIndex(Index &Idx, const doc::Info *Info) {
   for (const auto &R : llvm::reverse(Info->Namespace)) {
     // Look for the current namespace in the children of the index I is
     // pointing.
-    auto It = std::find(I->Children.begin(), I->Children.end(), R.USR);
+    auto It = llvm::find(I->Children, R.USR);
     if (It != I->Children.end()) {
       // If it is found, just change I to point the namespace reference found.
       I = &*It;
@@ -79,7 +79,7 @@ void Generator::addInfoToIndex(Index &Idx, const doc::Info *Info) {
   // Look for Info in the vector where it is supposed to be; it could already
   // exist if it is a parent namespace of an Info already passed to this
   // function.
-  auto It = std::find(I->Children.begin(), I->Children.end(), Info->USR);
+  auto It = llvm::find(I->Children, Info->USR);
   if (It == I->Children.end()) {
     // If it is not in the vector it is inserted
     I->Children.emplace_back(Info->USR, Info->extractName(), Info->IT,

clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp

@@ -78,8 +78,7 @@ std::string ConfusableIdentifierCheck::skeleton(StringRef Name) {
       UTF8 *BufferStart = std::begin(Buffer);
       UTF8 *IBuffer = BufferStart;
       const UTF32 *ValuesStart = std::begin(Where->values);
-      const UTF32 *ValuesEnd =
-          std::find(std::begin(Where->values), std::end(Where->values), '\0');
+      const UTF32 *ValuesEnd = llvm::find(Where->values, '\0');
       if (ConvertUTF32toUTF8(&ValuesStart, ValuesEnd, &IBuffer,
                              std::end(Buffer),
                              strictConversion) != conversionOK) {

clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp

@@ -75,7 +75,7 @@ static bool containsMisleadingBidi(StringRef Buffer,
       BidiContexts.push_back(PDI);
     // Close a PDI Context.
     else if (CodePoint == PDI) {
-      auto R = std::find(BidiContexts.rbegin(), BidiContexts.rend(), PDI);
+      auto R = llvm::find(llvm::reverse(BidiContexts), PDI);
       if (R != BidiContexts.rend())
         BidiContexts.resize(BidiContexts.rend() - R - 1);
     }

clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp

@@ -120,8 +120,7 @@ void UnnecessaryValueParamCheck::check(const MatchFinder::MatchResult &Result) {
     }
   }
 
-  const size_t Index = std::find(Function->parameters().begin(),
-                                 Function->parameters().end(), Param) -
+  const size_t Index = llvm::find(Function->parameters(), Param) -
                        Function->parameters().begin();
 
   auto Diag =

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

@@ -519,9 +519,9 @@ int clangTidyMain(int argc, const char **argv) {
     std::vector<clang::tidy::ClangTidyOptionsProvider::OptionsSource>
         RawOptions = OptionsProvider->getRawOptions(FilePath);
     for (const std::string &Check : EnabledChecks) {
-      for (auto It = RawOptions.rbegin(); It != RawOptions.rend(); ++It) {
-        if (It->first.Checks && GlobList(*It->first.Checks).contains(Check)) {
-          llvm::outs() << "'" << Check << "' is enabled in the " << It->second
+      for (const auto &[Opts, Source] : llvm::reverse(RawOptions)) {
+        if (Opts.Checks && GlobList(*Opts.Checks).contains(Check)) {
+          llvm::outs() << "'" << Check << "' is enabled in the " << Source
                        << ".\n";
           break;
         }
@@ -557,20 +557,16 @@ int clangTidyMain(int argc, const char **argv) {
     NamesAndOptions Valid =
         getAllChecksAndOptions(AllowEnablingAnalyzerAlphaCheckers);
     bool AnyInvalid = false;
-    for (const std::pair<ClangTidyOptions, std::string> &OptionWithSource :
-         RawOptions) {
-      const ClangTidyOptions &Opts = OptionWithSource.first;
+    for (const auto &[Opts, Source] : RawOptions) {
       if (Opts.Checks)
-        AnyInvalid |=
-            verifyChecks(Valid.Names, *Opts.Checks, OptionWithSource.second);
+        AnyInvalid |= verifyChecks(Valid.Names, *Opts.Checks, Source);
 
       for (auto Key : Opts.CheckOptions.keys()) {
         if (Valid.Options.contains(Key))
           continue;
         AnyInvalid = true;
-        auto &Output =
-            llvm::WithColor::warning(llvm::errs(), OptionWithSource.second)
-            << "unknown check option '" << Key << '\'';
+        auto &Output = llvm::WithColor::warning(llvm::errs(), Source)
+                       << "unknown check option '" << Key << '\'';
         llvm::StringRef Closest = closest(Key, Valid.Options);
         if (!Closest.empty())
           Output << "; did you mean '" << Closest << '\'';

clang-tools-extra/clangd/InlayHints.cpp

@@ -581,9 +581,8 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
   static const ParmVarDecl *getParamDefinition(const ParmVarDecl *P) {
     if (auto *Callee = dyn_cast<FunctionDecl>(P->getDeclContext())) {
       if (auto *Def = Callee->getDefinition()) {
-        auto I = std::distance(
-            Callee->param_begin(),
-            std::find(Callee->param_begin(), Callee->param_end(), P));
+        auto I = std::distance(Callee->param_begin(),
+                               llvm::find(Callee->parameters(), P));
         if (I < Callee->getNumParams()) {
           return Def->getParamDecl(I);
         }

clang/docs/ReleaseNotes.rst

@@ -418,6 +418,11 @@ Attribute Changes in Clang
   used ``201904L`` (the date the proposal was seen by the committee) by mistake.
   There were no other changes to the attribute behavior.
 
+- Introduced a new record declaration attribute ``__attribute__((enforce_read_only_placement))``
+  to support analysis of instances of a given type focused on read-only program
+  memory placement. It emits a warning if something in the code provably prevents
+  an instance from a read-only memory placement.
+
 Windows Support
 ---------------
 - For the MinGW driver, added the options ``-mguard=none``, ``-mguard=cf`` and

clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h

@@ -32,6 +32,16 @@
 namespace clang {
 namespace dataflow {
 
+template <typename AnalysisT, typename LatticeT, typename = std::void_t<>>
+struct HasTransferBranchFor : std::false_type {};
+
+template <typename AnalysisT, typename LatticeT>
+struct HasTransferBranchFor<
+    AnalysisT, LatticeT,
+    std::void_t<decltype(std::declval<AnalysisT>().transferBranch(
+        std::declval<bool>(), std::declval<const Stmt *>(),
+        std::declval<LatticeT &>(), std::declval<Environment &>()))>>
+    : std::true_type {};
 /// Base class template for dataflow analyses built on a single lattice type.
 ///
 /// Requirements:
@@ -101,6 +111,17 @@ class DataflowAnalysis : public TypeErasedDataflowAnalysis {
     static_cast<Derived *>(this)->transfer(Element, L, Env);
   }
 
+  void transferBranchTypeErased(bool Branch, const Stmt *Stmt,
+                                TypeErasedLattice &E, Environment &Env) final {
+    if constexpr (HasTransferBranchFor<Derived, LatticeT>::value) {
+      Lattice &L = llvm::any_cast<Lattice &>(E.Value);
+      static_cast<Derived *>(this)->transferBranch(Branch, Stmt, L, Env);
+    }
+    // Silence unused parameter warnings.
+    (void)Branch;
+    (void)Stmt;
+  }
+
 private:
   ASTContext &Context;
 };

clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h

@@ -84,6 +84,14 @@ class TypeErasedDataflowAnalysis : public Environment::ValueModel {
   virtual void transferTypeErased(const CFGElement *, TypeErasedLattice &,
                                   Environment &) = 0;
 
+  /// Applies the analysis transfer function for a given edge from a CFG block
+  /// of a conditional statement.
+  /// @param Stmt The condition which is responsible for the split in the CFG.
+  /// @param Branch True if the edge goes to the basic block where the
+  /// condition is true.
+  virtual void transferBranchTypeErased(bool Branch, const Stmt *,
+                                        TypeErasedLattice &, Environment &) = 0;
+
   /// If the built-in transfer functions (which model the heap and stack in the
   /// `Environment`) are to be applied, returns the options to be passed to
   /// them. Otherwise returns empty.

clang/include/clang/Basic/Attr.td

@@ -4145,3 +4145,8 @@ def FunctionReturnThunks : InheritableAttr,
   let Subjects = SubjectList<[Function]>;
   let Documentation = [FunctionReturnThunksDocs];
 }
+def ReadOnlyPlacement : InheritableAttr {
+  let Spellings = [Clang<"enforce_read_only_placement">];
+  let Subjects = SubjectList<[Record]>;
+  let Documentation = [ReadOnlyPlacementDocs];
+}

clang/include/clang/Basic/AttrDocs.td

@@ -6910,3 +6910,42 @@ The symbol used for ``thunk-extern`` is target specific:
 As such, this function attribute is currently only supported on X86 targets.
   }];
 }
+
+def ReadOnlyPlacementDocs : Documentation {
+  let Category = DocCatType;
+  let Content = [{This attribute is attached to a structure, class or union declaration.
+  When attached to a record declaration/definition, it checks if all instances
+  of this type can be placed in the read-only data segment of the program. If it
+  finds an instance that can not be placed in a read-only segment, the compiler
+  emits a warning at the source location where the type was used.
+
+  Examples:
+  * ``struct __attribute__((enforce_read_only_placement)) Foo;``
+  * ``struct __attribute__((enforce_read_only_placement)) Bar { ... };``
+
+  Both ``Foo`` and ``Bar`` types have the ``enforce_read_only_placement`` attribute.
+
+  The goal of introducing this attribute is to assist developers with writing secure
+  code. A ``const``-qualified global is generally placed in the read-only section
+  of the memory that has additional run time protection from malicious writes. By
+  attaching this attribute to a declaration, the developer can express the intent
+  to place all instances of the annotated type in the read-only program memory.
+
+  Note 1: The attribute doesn't guarantee that the object will be placed in the
+  read-only data segment as it does not instruct the compiler to ensure such
+  a placement. It emits a warning if something in the code can be proven to prevent
+  an instance from being placed in the read-only data segment.
+
+  Note 2: Currently, clang only checks if all global declarations of a given type 'T'
+  are ``const``-qualified. The following conditions would also prevent the data to be
+  put into read only segment, but the corresponding warnings are not yet implemented.
+
+  1. An instance of type ``T`` is allocated on the heap/stack.
+  2. Type ``T`` defines/inherits a mutable field.
+  3. Type ``T`` defines/inherits non-constexpr constructor(s) for initialization.
+  4. A field of type ``T`` is defined by type ``Q``, which does not bear the
+     ``enforce_read_only_placement`` attribute.
+  5. A type ``Q`` inherits from type ``T`` and it does not have the
+     ``enforce_read_only_placement`` attribute.
+  }];
+}

clang/include/clang/Basic/DiagnosticGroups.td

@@ -1396,3 +1396,6 @@ def BranchProtection : DiagGroup<"branch-protection">;
 // HLSL diagnostic groups
 // Warnings for HLSL Clang extensions
 def HLSLExtension : DiagGroup<"hlsl-extensions">;
+
+// Warnings and notes related to const_var_decl_type attribute checks
+def ReadOnlyPlacementChecks : DiagGroup<"read-only-types">;

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -5744,6 +5744,12 @@ def err_new_abi_tag_on_redeclaration : Error<
 def note_use_ifdef_guards : Note<
   "unguarded header; consider using #ifdef guards or #pragma once">;
 
+def warn_var_decl_not_read_only : Warning<
+  "object of type %0 cannot be placed in read-only memory">,
+  InGroup<ReadOnlyPlacementChecks>;
+def note_enforce_read_only_placement : Note<"type was declared read-only here">;
+
+
 def note_deleted_dtor_no_operator_delete : Note<
   "virtual destructor requires an unambiguous, accessible 'operator delete'">;
 def note_deleted_special_member_class_subobject : Note<

clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp

@@ -69,54 +69,63 @@ static int blockIndexInPredecessor(const CFGBlock &Pred,
   return BlockPos - Pred.succ_begin();
 }
 
+// The return type of the visit functions in TerminatorVisitor. The first
+// element represents the terminator expression (that is the conditional
+// expression in case of a path split in the CFG). The second element
+// represents whether the condition was true or false.
+using TerminatorVisitorRetTy = std::pair<const Expr *, bool>;
+
 /// Extends the flow condition of an environment based on a terminator
 /// statement.
-class TerminatorVisitor : public ConstStmtVisitor<TerminatorVisitor> {
+class TerminatorVisitor
+    : public ConstStmtVisitor<TerminatorVisitor, TerminatorVisitorRetTy> {
 public:
   TerminatorVisitor(const StmtToEnvMap &StmtToEnv, Environment &Env,
                     int BlockSuccIdx, TransferOptions TransferOpts)
-      : StmtToEnv(StmtToEnv), Env(Env), BlockSuccIdx(BlockSuccIdx),
-        TransferOpts(TransferOpts) {}
+      : StmtToEnv(StmtToEnv), Env(Env),
+        BlockSuccIdx(BlockSuccIdx), TransferOpts(TransferOpts) {}
 
-  void VisitIfStmt(const IfStmt *S) {
+  TerminatorVisitorRetTy VisitIfStmt(const IfStmt *S) {
     auto *Cond = S->getCond();
     assert(Cond != nullptr);
-    extendFlowCondition(*Cond);
+    return extendFlowCondition(*Cond);
   }
 
-  void VisitWhileStmt(const WhileStmt *S) {
+  TerminatorVisitorRetTy VisitWhileStmt(const WhileStmt *S) {
     auto *Cond = S->getCond();
     assert(Cond != nullptr);
-    extendFlowCondition(*Cond);
+    return extendFlowCondition(*Cond);
   }
 
-  void VisitDoStmt(const DoStmt *S) {
+  TerminatorVisitorRetTy VisitDoStmt(const DoStmt *S) {
     auto *Cond = S->getCond();
     assert(Cond != nullptr);
-    extendFlowCondition(*Cond);
+    return extendFlowCondition(*Cond);
   }
 
-  void VisitForStmt(const ForStmt *S) {
+  TerminatorVisitorRetTy VisitForStmt(const ForStmt *S) {
     auto *Cond = S->getCond();
     if (Cond != nullptr)
-      extendFlowCondition(*Cond);
+      return extendFlowCondition(*Cond);
+    return {nullptr, false};
   }
 
-  void VisitBinaryOperator(const BinaryOperator *S) {
+  TerminatorVisitorRetTy VisitBinaryOperator(const BinaryOperator *S) {
     assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
     auto *LHS = S->getLHS();
     assert(LHS != nullptr);
-    extendFlowCondition(*LHS);
+    return extendFlowCondition(*LHS);
   }
 
-  void VisitConditionalOperator(const ConditionalOperator *S) {
+  TerminatorVisitorRetTy
+  VisitConditionalOperator(const ConditionalOperator *S) {
     auto *Cond = S->getCond();
     assert(Cond != nullptr);
-    extendFlowCondition(*Cond);
+    return extendFlowCondition(*Cond);
   }
 
 private:
-  void extendFlowCondition(const Expr &Cond) {
+  TerminatorVisitorRetTy extendFlowCondition(const Expr &Cond) {
     // The terminator sub-expression might not be evaluated.
     if (Env.getStorageLocation(Cond, SkipPast::None) == nullptr)
       transfer(StmtToEnv, Cond, Env, TransferOpts);
@@ -140,12 +149,16 @@ class TerminatorVisitor : public ConstStmtVisitor<TerminatorVisitor> {
       Env.setValue(*Loc, *Val);
     }
 
+    bool ConditionValue = true;
     // The condition must be inverted for the successor that encompasses the
     // "else" branch, if such exists.
-    if (BlockSuccIdx == 1)
+    if (BlockSuccIdx == 1) {
       Val = &Env.makeNot(*Val);
+      ConditionValue = false;
+    }
 
     Env.addToFlowCondition(*Val);
+    return {&Cond, ConditionValue};
   }
 
   const StmtToEnvMap &StmtToEnv;
@@ -239,10 +252,16 @@ computeBlockInputState(const CFGBlock &Block, AnalysisContext &AC) {
     if (BuiltinTransferOpts) {
       if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
         const StmtToEnvMapImpl StmtToEnv(AC.CFCtx, AC.BlockStates);
-        TerminatorVisitor(StmtToEnv, PredState.Env,
-                          blockIndexInPredecessor(*Pred, Block),
-                          *BuiltinTransferOpts)
-            .Visit(PredTerminatorStmt);
+        auto [Cond, CondValue] =
+            TerminatorVisitor(StmtToEnv, PredState.Env,
+                              blockIndexInPredecessor(*Pred, Block),
+                              *BuiltinTransferOpts)
+                .Visit(PredTerminatorStmt);
+        if (Cond != nullptr)
+          // FIXME: Call transferBranchTypeErased even if BuiltinTransferOpts
+          // are not set.
+          Analysis.transferBranchTypeErased(CondValue, Cond, PredState.Lattice,
+                                            PredState.Env);
       }
     }
 

clang/lib/Sema/SemaDecl.cpp

@@ -7380,6 +7380,36 @@ static void copyAttrFromTypedefToDecl(Sema &S, Decl *D, const TypedefType *TT) {
   }
 }
 
+// This function emits warning and a corresponding note based on the
+// ReadOnlyPlacementAttr attribute. The warning checks that all global variable
+// declarations of an annotated type must be const qualified.
+void emitReadOnlyPlacementAttrWarning(Sema &S, const VarDecl *VD) {
+  QualType VarType = VD->getType().getCanonicalType();
+
+  // Ignore local declarations (for now) and those with const qualification.
+  // TODO: Local variables should not be allowed if their type declaration has
+  // ReadOnlyPlacementAttr attribute. To be handled in follow-up patch.
+  if (!VD || VD->hasLocalStorage() || VD->getType().isConstQualified())
+    return;
+
+  if (VarType->isArrayType()) {
+    // Retrieve element type for array declarations.
+    VarType = S.getASTContext().getBaseElementType(VarType);
+  }
+
+  const RecordDecl *RD = VarType->getAsRecordDecl();
+
+  // Check if the record declaration is present and if it has any attributes.
+  if (RD == nullptr)
+    return;
+
+  if (const auto *ConstDecl = RD->getAttr<ReadOnlyPlacementAttr>()) {
+    S.Diag(VD->getLocation(), diag::warn_var_decl_not_read_only) << RD;
+    S.Diag(ConstDecl->getLocation(), diag::note_enforce_read_only_placement);
+    return;
+  }
+}
+
 NamedDecl *Sema::ActOnVariableDeclarator(
     Scope *S, Declarator &D, DeclContext *DC, TypeSourceInfo *TInfo,
     LookupResult &Previous, MultiTemplateParamsArg TemplateParamLists,
@@ -8044,6 +8074,8 @@ NamedDecl *Sema::ActOnVariableDeclarator(
   if (IsMemberSpecialization && !NewVD->isInvalidDecl())
     CompleteMemberSpecialization(NewVD, Previous);
 
+  emitReadOnlyPlacementAttrWarning(*this, NewVD);
+
   return NewVD;
 }