[pull] main from llvm:main

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

@@ -12,8 +12,6 @@
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 
-#include <iostream>
-
 using namespace clang::ast_matchers;
 
 namespace clang {
@@ -132,6 +130,12 @@ void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) {
     VC = VariableCategory::Reference;
   if (Variable->getType()->isPointerType())
     VC = VariableCategory::Pointer;
+  if (Variable->getType()->isArrayType()) {
+    if (const auto *ArrayT = dyn_cast<ArrayType>(Variable->getType())) {
+      if (ArrayT->getElementType()->isPointerType())
+        VC = VariableCategory::Pointer;
+    }
+  }
 
   // Each variable can only be in one category: Value, Pointer, Reference.
   // Analysis can be controlled for every category.

clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp

@@ -10,4 +10,65 @@ void potential_const_pointer() {
   double *p_local0 = &np_local0[1];
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double *' can be declared 'const'
   // CHECK-FIXES: double *const p_local0
+
+  using doublePtr = double*;
+  using doubleArray = double[15];
+  doubleArray np_local1;
+  doublePtr p_local1 = &np_local1[0];
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'doublePtr' (aka 'double *') can be declared 'const'
+  // CHECK-FIXES: doublePtr const p_local1
+}
+
+void range_for() {
+  int np_local0[2] = {1, 2};
+  int *p_local0[2] = {&np_local0[0], &np_local0[1]};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int *[2]' can be declared 'const'
+  // CHECK-FIXES: int *const p_local0[2]
+  for (const int *p_local1 : p_local0) {
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'p_local1' of type 'const int *' can be declared 'const'
+  // CHECK-FIXES: for (const int *const p_local1 : p_local0)
+  }
+
+  int *p_local2[2] = {nullptr, nullptr};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'int *[2]' can be declared 'const'
+  // CHECK-FIXES: int *const p_local2[2]
+  for (const auto *con_ptr : p_local2) {
+  }
+
+}
+
+template <typename T>
+struct SmallVectorBase {
+  T data[4];
+  void push_back(const T &el) {}
+  int size() const { return 4; }
+  T *begin() { return data; }
+  const T *begin() const { return data; }
+  T *end() { return data + 4; }
+  const T *end() const { return data + 4; }
+};
+
+template <typename T>
+struct SmallVector : SmallVectorBase<T> {};
+
+template <class T>
+void EmitProtocolMethodList(T &&Methods) {
+  // Note: If the template is uninstantiated the analysis does not figure out,
+  // that p_local0 could be const. Not sure why, but probably bails because
+  // some expressions are type-dependent.
+  SmallVector<const int *> p_local0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'SmallVector<const int *>' can be declared 'const'
+  // CHECK-FIXES: SmallVector<const int *> const p_local0
+  SmallVector<const int *> np_local0;
+  for (const auto *I : Methods) {
+    if (I == nullptr)
+      np_local0.push_back(I);
+  }
+  p_local0.size();
+}
+void instantiate() {
+  int *p_local0[4] = {nullptr, nullptr, nullptr, nullptr};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int *[4]' can be declared 'const'
+  // CHECK-FIXES: int *const p_local0[4]
+  EmitProtocolMethodList(p_local0);
 }

clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp

@@ -526,18 +526,13 @@ void range_for() {
   // CHECK-FIXES: int const p_local1[2]
   for (const int &const_ref : p_local1) {
   }
+}
 
-  int *p_local2[2] = {&np_local0[0], &np_local0[1]};
-  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'int *[2]' can be declared 'const'
-  // CHECK-FIXES: int *const p_local2[2]
-  for (const int *con_ptr : p_local2) {
-  }
-
-  int *p_local3[2] = {nullptr, nullptr};
-  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local3' of type 'int *[2]' can be declared 'const'
-  // CHECK-FIXES: int *const p_local3[2]
-  for (const auto *con_ptr : p_local3) {
-  }
+void arrays_of_pointers_are_ignored() {
+  int *np_local0[2] = {nullptr, nullptr};
+
+  using intPtr = int*;
+  intPtr np_local1[2] = {nullptr, nullptr};
 }
 
 inline void *operator new(decltype(sizeof(void *)), void *p) { return p; }
@@ -908,41 +903,6 @@ void vlas() {
   sizeof(int[++N]);
 }
 
-template <typename T>
-struct SmallVectorBase {
-  T data[4];
-  void push_back(const T &el) {}
-  int size() const { return 4; }
-  T *begin() { return data; }
-  const T *begin() const { return data; }
-  T *end() { return data + 4; }
-  const T *end() const { return data + 4; }
-};
-
-template <typename T>
-struct SmallVector : SmallVectorBase<T> {};
-
-template <class T>
-void EmitProtocolMethodList(T &&Methods) {
-  // Note: If the template is uninstantiated the analysis does not figure out,
-  // that p_local0 could be const. Not sure why, but probably bails because
-  // some expressions are type-dependent.
-  SmallVector<const int *> p_local0;
-  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'SmallVector<const int *>' can be declared 'const'
-  // CHECK-FIXES: SmallVector<const int *> const p_local0
-  SmallVector<const int *> np_local0;
-  for (const auto *I : Methods) {
-    if (I == nullptr)
-      np_local0.push_back(I);
-  }
-  p_local0.size();
-}
-void instantiate() {
-  int *p_local0[4] = {nullptr, nullptr, nullptr, nullptr};
-  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int *[4]' can be declared 'const'
-  // CHECK-FIXES: int *const p_local0[4]
-  EmitProtocolMethodList(p_local0);
-}
 struct base {
   int member;
 };

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h

@@ -214,8 +214,14 @@ struct NodeBuilderContext {
   const CFGBlock *Block;
   const LocationContext *LC;
 
+  NodeBuilderContext(const CoreEngine &E, const CFGBlock *B,
+                     const LocationContext *L)
+      : Eng(E), Block(B), LC(L) {
+    assert(B);
+  }
+
   NodeBuilderContext(const CoreEngine &E, const CFGBlock *B, ExplodedNode *N)
-      : Eng(E), Block(B), LC(N->getLocationContext()) { assert(B); }
+      : NodeBuilderContext(E, B, N->getLocationContext()) {}
 
   /// Return the CFGBlock associated with this builder.
   const CFGBlock *getBlock() const { return Block; }

clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h

@@ -732,6 +732,7 @@ class ExprEngine {
   /// A multi-dimensional array is also a continuous memory location in a
   /// row major order, so for arr[0][0] Idx is 0 and for arr[2][2] Idx is 8.
   SVal computeObjectUnderConstruction(const Expr *E, ProgramStateRef State,
+                                      const NodeBuilderContext *BldrCtx,
                                       const LocationContext *LCtx,
                                       const ConstructionContext *CC,
                                       EvalCallOptions &CallOpts,
@@ -748,13 +749,13 @@ class ExprEngine {
 
   /// A convenient wrapper around computeObjectUnderConstruction
   /// and updateObjectsUnderConstruction.
-  std::pair<ProgramStateRef, SVal>
-  handleConstructionContext(const Expr *E, ProgramStateRef State,
-                            const LocationContext *LCtx,
-                            const ConstructionContext *CC,
-                            EvalCallOptions &CallOpts, unsigned Idx = 0) {
+  std::pair<ProgramStateRef, SVal> handleConstructionContext(
+      const Expr *E, ProgramStateRef State, const NodeBuilderContext *BldrCtx,
+      const LocationContext *LCtx, const ConstructionContext *CC,
+      EvalCallOptions &CallOpts, unsigned Idx = 0) {
 
-    SVal V = computeObjectUnderConstruction(E, State, LCtx, CC, CallOpts, Idx);
+    SVal V = computeObjectUnderConstruction(E, State, BldrCtx, LCtx, CC,
+                                            CallOpts, Idx);
     State = updateObjectsUnderConstruction(V, E, State, LCtx, CC, CallOpts);
 
     return std::make_pair(State, V);

clang/lib/CodeGen/CGDebugInfo.cpp

@@ -370,6 +370,7 @@ CGDebugInfo::computeChecksum(FileID FID, SmallString<64> &Checksum) const {
     llvm::toHex(llvm::SHA256::hash(Data), /*LowerCase=*/true, Checksum);
     return llvm::DIFile::CSK_SHA256;
   }
+  llvm_unreachable("Unhandled DebugSrcHashKind enum");
 }
 
 Optional<StringRef> CGDebugInfo::getSource(const SourceManager &SM,

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

@@ -485,9 +485,9 @@ CallEvent::getReturnValueUnderConstruction() const {
 
   EvalCallOptions CallOpts;
   ExprEngine &Engine = getState()->getStateManager().getOwningEngine();
-  SVal RetVal =
-    Engine.computeObjectUnderConstruction(getOriginExpr(), getState(),
-                                          getLocationContext(), CC, CallOpts);
+  SVal RetVal = Engine.computeObjectUnderConstruction(
+      getOriginExpr(), getState(), &Engine.getBuilderContext(),
+      getLocationContext(), CC, CallOpts);
   return RetVal;
 }
 

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

@@ -111,9 +111,15 @@ SVal ExprEngine::makeElementRegion(ProgramStateRef State, SVal LValue,
   return LValue;
 }
 
+// In case when the prvalue is returned from the function (kind is one of
+// SimpleReturnedValueKind, CXX17ElidedCopyReturnedValueKind), then
+// it's materialization happens in context of the caller.
+// We pass BldrCtx explicitly, as currBldrCtx always refers to callee's context.
 SVal ExprEngine::computeObjectUnderConstruction(
-    const Expr *E, ProgramStateRef State, const LocationContext *LCtx,
-    const ConstructionContext *CC, EvalCallOptions &CallOpts, unsigned Idx) {
+    const Expr *E, ProgramStateRef State, const NodeBuilderContext *BldrCtx,
+    const LocationContext *LCtx, const ConstructionContext *CC,
+    EvalCallOptions &CallOpts, unsigned Idx) {
+
   SValBuilder &SVB = getSValBuilder();
   MemRegionManager &MRMgr = SVB.getRegionManager();
   ASTContext &ACtx = SVB.getContext();
@@ -210,8 +216,11 @@ SVal ExprEngine::computeObjectUnderConstruction(
           CallerLCtx = CallerLCtx->getParent();
           assert(!isa<BlockInvocationContext>(CallerLCtx));
         }
+
+        NodeBuilderContext CallerBldrCtx(getCoreEngine(),
+                                         SFC->getCallSiteBlock(), CallerLCtx);
         return computeObjectUnderConstruction(
-            cast<Expr>(SFC->getCallSite()), State, CallerLCtx,
+            cast<Expr>(SFC->getCallSite()), State, &CallerBldrCtx, CallerLCtx,
             RTC->getConstructionContext(), CallOpts);
       } else {
         // We are on the top frame of the analysis. We do not know where is the
@@ -251,7 +260,7 @@ SVal ExprEngine::computeObjectUnderConstruction(
       EvalCallOptions PreElideCallOpts = CallOpts;
 
       SVal V = computeObjectUnderConstruction(
-          TCC->getConstructorAfterElision(), State, LCtx,
+          TCC->getConstructorAfterElision(), State, BldrCtx, LCtx,
           TCC->getConstructionContextAfterElision(), CallOpts);
 
       // FIXME: This definition of "copy elision has not failed" is unreliable.
@@ -319,7 +328,7 @@ SVal ExprEngine::computeObjectUnderConstruction(
       CallEventManager &CEMgr = getStateManager().getCallEventManager();
       auto getArgLoc = [&](CallEventRef<> Caller) -> Optional<SVal> {
         const LocationContext *FutureSFC =
-            Caller->getCalleeStackFrame(currBldrCtx->blockCount());
+            Caller->getCalleeStackFrame(BldrCtx->blockCount());
         // Return early if we are unable to reliably foresee
         // the future stack frame.
         if (!FutureSFC)
@@ -338,7 +347,7 @@ SVal ExprEngine::computeObjectUnderConstruction(
         // because this-argument is implemented as a normal argument in
         // operator call expressions but not in operator declarations.
         const TypedValueRegion *TVR = Caller->getParameterLocation(
-            *Caller->getAdjustedParameterIndex(Idx), currBldrCtx->blockCount());
+            *Caller->getAdjustedParameterIndex(Idx), BldrCtx->blockCount());
         if (!TVR)
           return None;
 
@@ -643,8 +652,8 @@ void ExprEngine::handleConstructor(const Expr *E,
     }
 
     // The target region is found from construction context.
-    std::tie(State, Target) =
-        handleConstructionContext(CE, State, LCtx, CC, CallOpts, Idx);
+    std::tie(State, Target) = handleConstructionContext(
+        CE, State, currBldrCtx, LCtx, CC, CallOpts, Idx);
     break;
   }
   case CXXConstructExpr::CK_VirtualBase: {

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

@@ -774,9 +774,9 @@ ProgramStateRef ExprEngine::bindReturnValue(const CallEvent &Call,
     SVal Target;
     assert(RTC->getStmt() == Call.getOriginExpr());
     EvalCallOptions CallOpts; // FIXME: We won't really need those.
-    std::tie(State, Target) =
-        handleConstructionContext(Call.getOriginExpr(), State, LCtx,
-                                  RTC->getConstructionContext(), CallOpts);
+    std::tie(State, Target) = handleConstructionContext(
+        Call.getOriginExpr(), State, currBldrCtx, LCtx,
+        RTC->getConstructionContext(), CallOpts);
     const MemRegion *TargetR = Target.getAsRegion();
     assert(TargetR);
     // Invalidate the region so that it didn't look uninitialized. If this is

clang/test/Analysis/copy-elision.cpp

@@ -20,6 +20,7 @@
 #endif
 
 void clang_analyzer_eval(bool);
+void clang_analyzer_dump(int);
 
 namespace variable_functional_cast_crash {
 
@@ -418,3 +419,31 @@ void test_copy_elision() {
 }
 
 } // namespace address_vector_tests
+
+namespace arg_directly_from_return_in_loop {
+
+struct Result {
+  int value;
+};
+
+Result create() {
+  return Result{10};
+}
+
+int accessValue(Result r) {
+  return r.value;
+}
+
+void test() {
+  for (int i = 0; i < 3; ++i) {
+    int v = accessValue(create());
+    if (i == 0) {
+      clang_analyzer_dump(v); // expected-warning {{10 S32b}}
+    } else {
+      clang_analyzer_dump(v); // expected-warning {{10 S32b}}
+                              // was {{reg_${{[0-9]+}}<int r.value> }} for C++11
+    }
+  }
+}
+
+} // namespace arg_directly_from_return_in_loop