Merge pull request #2820 from SavchenkoValeriy/april_analyzer_cherry_picks_2

Static analyzer cherry picks #22

Author: SavchenkoValeriy

clang/lib/Analysis/BodyFarm.cpp

@@ -742,63 +742,67 @@ static const ObjCIvarDecl *findBackingIvar(const ObjCPropertyDecl *Prop) {
 
 static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
                                       const ObjCMethodDecl *MD) {
-    // First, find the backing ivar.
+  // First, find the backing ivar.
   const ObjCIvarDecl *IVar = nullptr;
+  const ObjCPropertyDecl *Prop = nullptr;
 
   // Property accessor stubs sometimes do not correspond to any property decl
   // in the current interface (but in a superclass). They still have a
   // corresponding property impl decl in this case.
   if (MD->isSynthesizedAccessorStub()) {
     const ObjCInterfaceDecl *IntD = MD->getClassInterface();
     const ObjCImplementationDecl *ImpD = IntD->getImplementation();
-    for (const auto *PI: ImpD->property_impls()) {
-      if (const ObjCPropertyDecl *P = PI->getPropertyDecl()) {
-        if (P->getGetterName() == MD->getSelector())
-          IVar = P->getPropertyIvarDecl();
+    for (const auto *PI : ImpD->property_impls()) {
+      if (const ObjCPropertyDecl *Candidate = PI->getPropertyDecl()) {
+        if (Candidate->getGetterName() == MD->getSelector()) {
+          Prop = Candidate;
+          IVar = Prop->getPropertyIvarDecl();
+        }
       }
     }
   }
 
   if (!IVar) {
-    const ObjCPropertyDecl *Prop = MD->findPropertyDecl();
+    Prop = MD->findPropertyDecl();
     IVar = findBackingIvar(Prop);
-    if (!IVar)
-      return nullptr;
+  }
 
-    // Ignore weak variables, which have special behavior.
-    if (Prop->getPropertyAttributes() & ObjCPropertyAttribute::kind_weak)
-      return nullptr;
+  if (!IVar || !Prop)
+    return nullptr;
+
+  // Ignore weak variables, which have special behavior.
+  if (Prop->getPropertyAttributes() & ObjCPropertyAttribute::kind_weak)
+    return nullptr;
 
-    // Look to see if Sema has synthesized a body for us. This happens in
-    // Objective-C++ because the return value may be a C++ class type with a
-    // non-trivial copy constructor. We can only do this if we can find the
-    // @synthesize for this property, though (or if we know it's been auto-
-    // synthesized).
-    const ObjCImplementationDecl *ImplDecl =
+  // Look to see if Sema has synthesized a body for us. This happens in
+  // Objective-C++ because the return value may be a C++ class type with a
+  // non-trivial copy constructor. We can only do this if we can find the
+  // @synthesize for this property, though (or if we know it's been auto-
+  // synthesized).
+  const ObjCImplementationDecl *ImplDecl =
       IVar->getContainingInterface()->getImplementation();
-    if (ImplDecl) {
-      for (const auto *I : ImplDecl->property_impls()) {
-        if (I->getPropertyDecl() != Prop)
-          continue;
-
-        if (I->getGetterCXXConstructor()) {
-          ASTMaker M(Ctx);
-          return M.makeReturn(I->getGetterCXXConstructor());
-        }
+  if (ImplDecl) {
+    for (const auto *I : ImplDecl->property_impls()) {
+      if (I->getPropertyDecl() != Prop)
+        continue;
+
+      if (I->getGetterCXXConstructor()) {
+        ASTMaker M(Ctx);
+        return M.makeReturn(I->getGetterCXXConstructor());
       }
     }
-
-    // Sanity check that the property is the same type as the ivar, or a
-    // reference to it, and that it is either an object pointer or trivially
-    // copyable.
-    if (!Ctx.hasSameUnqualifiedType(IVar->getType(),
-                                    Prop->getType().getNonReferenceType()))
-      return nullptr;
-    if (!IVar->getType()->isObjCLifetimeType() &&
-        !IVar->getType().isTriviallyCopyableType(Ctx))
-      return nullptr;
   }
 
+  // Sanity check that the property is the same type as the ivar, or a
+  // reference to it, and that it is either an object pointer or trivially
+  // copyable.
+  if (!Ctx.hasSameUnqualifiedType(IVar->getType(),
+                                  Prop->getType().getNonReferenceType()))
+    return nullptr;
+  if (!IVar->getType()->isObjCLifetimeType() &&
+      !IVar->getType().isTriviallyCopyableType(Ctx))
+    return nullptr;
+
   // Generate our body:
   //   return self->_ivar;
   ASTMaker M(Ctx);
@@ -807,11 +811,8 @@ static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
   if (!selfVar)
     return nullptr;
 
-  Expr *loadedIVar =
-    M.makeObjCIvarRef(
-      M.makeLvalueToRvalue(
-        M.makeDeclRefExpr(selfVar),
-        selfVar->getType()),
+  Expr *loadedIVar = M.makeObjCIvarRef(
+      M.makeLvalueToRvalue(M.makeDeclRefExpr(selfVar), selfVar->getType()),
       IVar);
 
   if (!MD->getReturnType()->isReferenceType())

clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp

@@ -11,17 +11,18 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "clang/Lex/Lexer.h"
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/ParentMap.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Analysis/Analyses/LiveVariables.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "llvm/ADT/BitVector.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/SaveAndRestore.h"
 
@@ -408,15 +409,17 @@ class DeadStoreObs : public LiveVariables::Observer {
               // Special case: check for initializations with constants.
               //
               //  e.g. : int x = 0;
+              //         struct A = {0, 1};
+              //         struct B = {{0}, {1, 2}};
               //
               // If x is EVER assigned a new value later, don't issue
               // a warning.  This is because such initialization can be
               // due to defensive programming.
-              if (E->isEvaluatable(Ctx))
+              if (isConstant(E))
                 return;
 
               if (const DeclRefExpr *DRE =
-                  dyn_cast<DeclRefExpr>(E->IgnoreParenCasts()))
+                      dyn_cast<DeclRefExpr>(E->IgnoreParenCasts()))
                 if (const VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
                   // Special case: check for initialization from constant
                   //  variables.
@@ -444,6 +447,29 @@ class DeadStoreObs : public LiveVariables::Observer {
         }
       }
   }
+
+private:
+  /// Return true if the given init list can be interpreted as constant
+  bool isConstant(const InitListExpr *Candidate) const {
+    // We consider init list to be constant if each member of the list can be
+    // interpreted as constant.
+    return llvm::all_of(Candidate->inits(),
+                        [this](const Expr *Init) { return isConstant(Init); });
+  }
+
+  /// Return true if the given expression can be interpreted as constant
+  bool isConstant(const Expr *E) const {
+    // It looks like E itself is a constant
+    if (E->isEvaluatable(Ctx))
+      return true;
+
+    // We should also allow defensive initialization of structs, i.e. { 0 }
+    if (const auto *ILE = dyn_cast<InitListExpr>(E->IgnoreParenCasts())) {
+      return isConstant(ILE);
+    }
+
+    return false;
+  }
 };
 
 } // end anonymous namespace

clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp

@@ -34,9 +34,9 @@ namespace {
 class InnerPointerChecker
     : public Checker<check::DeadSymbols, check::PostCall> {
 
-  CallDescription AppendFn, AssignFn, ClearFn, CStrFn, DataFn, EraseFn,
-      InsertFn, PopBackFn, PushBackFn, ReplaceFn, ReserveFn, ResizeFn,
-      ShrinkToFitFn, SwapFn;
+  CallDescription AppendFn, AssignFn, AddressofFn, ClearFn, CStrFn, DataFn,
+      DataMemberFn, EraseFn, InsertFn, PopBackFn, PushBackFn, ReplaceFn,
+      ReserveFn, ResizeFn, ShrinkToFitFn, SwapFn;
 
 public:
   class InnerPointerBRVisitor : public BugReporterVisitor {
@@ -73,9 +73,10 @@ class InnerPointerChecker
   InnerPointerChecker()
       : AppendFn({"std", "basic_string", "append"}),
         AssignFn({"std", "basic_string", "assign"}),
+        AddressofFn({"std", "addressof"}),
         ClearFn({"std", "basic_string", "clear"}),
-        CStrFn({"std", "basic_string", "c_str"}),
-        DataFn({"std", "basic_string", "data"}),
+        CStrFn({"std", "basic_string", "c_str"}), DataFn({"std", "data"}, 1),
+        DataMemberFn({"std", "basic_string", "data"}),
         EraseFn({"std", "basic_string", "erase"}),
         InsertFn({"std", "basic_string", "insert"}),
         PopBackFn({"std", "basic_string", "pop_back"}),
@@ -90,6 +91,9 @@ class InnerPointerChecker
   /// pointers referring to the container object's inner buffer.
   bool isInvalidatingMemberFunction(const CallEvent &Call) const;
 
+  /// Check whether the called function returns a raw inner pointer.
+  bool isInnerPointerAccessFunction(const CallEvent &Call) const;
+
   /// Mark pointer symbols associated with the given memory region released
   /// in the program state.
   void markPtrSymbolsReleased(const CallEvent &Call, ProgramStateRef State,
@@ -130,6 +134,12 @@ bool InnerPointerChecker::isInvalidatingMemberFunction(
           Call.isCalled(SwapFn));
 }
 
+bool InnerPointerChecker::isInnerPointerAccessFunction(
+    const CallEvent &Call) const {
+  return (Call.isCalled(CStrFn) || Call.isCalled(DataFn) ||
+          Call.isCalled(DataMemberFn));
+}
+
 void InnerPointerChecker::markPtrSymbolsReleased(const CallEvent &Call,
                                                  ProgramStateRef State,
                                                  const MemRegion *MR,
@@ -172,6 +182,11 @@ void InnerPointerChecker::checkFunctionArguments(const CallEvent &Call,
       if (!ArgRegion)
         continue;
 
+      // std::addressof function accepts a non-const reference as an argument,
+      // but doesn't modify it.
+      if (Call.isCalled(AddressofFn))
+        continue;
+
       markPtrSymbolsReleased(Call, State, ArgRegion, C);
     }
   }
@@ -195,36 +210,49 @@ void InnerPointerChecker::checkPostCall(const CallEvent &Call,
                                         CheckerContext &C) const {
   ProgramStateRef State = C.getState();
 
+  // TODO: Do we need these to be typed?
+  const TypedValueRegion *ObjRegion = nullptr;
+
   if (const auto *ICall = dyn_cast<CXXInstanceCall>(&Call)) {
-    // TODO: Do we need these to be typed?
-    const auto *ObjRegion = dyn_cast_or_null<TypedValueRegion>(
+    ObjRegion = dyn_cast_or_null<TypedValueRegion>(
         ICall->getCXXThisVal().getAsRegion());
-    if (!ObjRegion)
-      return;
 
-    if (Call.isCalled(CStrFn) || Call.isCalled(DataFn)) {
-      SVal RawPtr = Call.getReturnValue();
-      if (SymbolRef Sym = RawPtr.getAsSymbol(/*IncludeBaseRegions=*/true)) {
-        // Start tracking this raw pointer by adding it to the set of symbols
-        // associated with this container object in the program state map.
+    // Check [string.require] / second point.
+    if (isInvalidatingMemberFunction(Call)) {
+      markPtrSymbolsReleased(Call, State, ObjRegion, C);
+      return;
+    }
+  }
 
-        PtrSet::Factory &F = State->getStateManager().get_context<PtrSet>();
-        const PtrSet *SetPtr = State->get<RawPtrMap>(ObjRegion);
-        PtrSet Set = SetPtr ? *SetPtr : F.getEmptySet();
-        assert(C.wasInlined || !Set.contains(Sym));
-        Set = F.add(Set, Sym);
+  if (isInnerPointerAccessFunction(Call)) {
 
-        State = State->set<RawPtrMap>(ObjRegion, Set);
-        C.addTransition(State);
-      }
-      return;
+    if (isa<SimpleFunctionCall>(Call)) {
+      // NOTE: As of now, we only have one free access function: std::data.
+      //       If we add more functions like this in the list, hardcoded
+      //       argument index should be changed.
+      ObjRegion =
+          dyn_cast_or_null<TypedValueRegion>(Call.getArgSVal(0).getAsRegion());
     }
 
-    // Check [string.require] / second point.
-    if (isInvalidatingMemberFunction(Call)) {
-      markPtrSymbolsReleased(Call, State, ObjRegion, C);
+    if (!ObjRegion)
       return;
+
+    SVal RawPtr = Call.getReturnValue();
+    if (SymbolRef Sym = RawPtr.getAsSymbol(/*IncludeBaseRegions=*/true)) {
+      // Start tracking this raw pointer by adding it to the set of symbols
+      // associated with this container object in the program state map.
+
+      PtrSet::Factory &F = State->getStateManager().get_context<PtrSet>();
+      const PtrSet *SetPtr = State->get<RawPtrMap>(ObjRegion);
+      PtrSet Set = SetPtr ? *SetPtr : F.getEmptySet();
+      assert(C.wasInlined || !Set.contains(Sym));
+      Set = F.add(Set, Sym);
+
+      State = State->set<RawPtrMap>(ObjRegion, Set);
+      C.addTransition(State);
     }
+
+    return;
   }
 
   // Check [string.require] / first point.

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

@@ -423,6 +423,14 @@ SVal SValBuilder::evalBinOp(ProgramStateRef state, BinaryOperator::Opcode op,
     return UnknownVal();
   }
 
+  if (op == BinaryOperatorKind::BO_Cmp) {
+    // We can't reason about C++20 spaceship operator yet.
+    //
+    // FIXME: Support C++20 spaceship operator.
+    //        The main problem here is that the result is not integer.
+    return UnknownVal();
+  }
+
   if (Optional<Loc> LV = lhs.getAs<Loc>()) {
     if (Optional<Loc> RV = rhs.getAs<Loc>())
       return evalBinOpLL(state, op, *LV, *RV, type);

clang/test/Analysis/PR47511.cpp

@@ -0,0 +1,19 @@
+// RUN: %clang_analyze_cc1 -std=c++20 -w -analyzer-checker=core -verify %s
+
+// expected-no-diagnostics
+
+namespace std {
+struct strong_ordering {
+  int n;
+  constexpr operator int() const { return n; }
+  static const strong_ordering equal, greater, less;
+};
+constexpr strong_ordering strong_ordering::equal = {0};
+constexpr strong_ordering strong_ordering::greater = {1};
+constexpr strong_ordering strong_ordering::less = {-1};
+} // namespace std
+
+void test() {
+  // no crash
+  (void)(0 <=> 0);
+}