[analyzer] SValBuilder should have an easy access to AnalyzerOptions

[steakhal]

Summary

SVB.getStateManager().getOwningEngine().getAnalysisManager().getAnalyzerOptions()
is quite a mouthful and might involve a few pointer indirections to get
such a simple thing like an analyzer option.

This patch introduces an AnalyzerOptions reference to the SValBuilder
abstract class, while refactors a few cases to use this simpler accessor.

[NoQ]

clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

55–56

You can move this code inside the constructor. This will keep the functionality but eliminate the extra constructor argument.

[steakhal]

Now acquire the AnalyzerOptions in the ctor.
Also, move the ctor to the implementation to prevent the ExprEngine header inclusion from the SValBuilder.h.

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

190

To be fair, I don't know why don't we use a plain old public const const field for this.
Similarly how AnalysisManager::options is defined.

What should be our preference? Should I follow the getter antipattern?

clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

55–56

I wanted to signify that the SValBuilder will have the AnalyzerOptions, thus I made it this way.
But you might be right that in long term that will be better.

[martong]

Looks good to me, thanks!

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

190

I'd stick to the existing style. IMHO, consistency is very important, b/c later if someone decides to refactor then they could do that even automatically.

[Szelethus]

Lets go! Mind that you wrote "eazy" instead of "easy" in the revision name. But, if you are just being that "kewl", I don't mind!

[steakhal]

In D108824#2982090, @Szelethus wrote:

Lets go! Mind that you wrote "eazy" instead of "easy" in the revision name. But, if you are just being that "kewl", I don't mind!

Thanks!

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

 #include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
 #include "llvm/ADT/ImmutableList.h"
 #include "llvm/ADT/Optional.h"
 #include <cstdint>
 
 namespace clang {
 
+class AnalyzerOptions;
 class BlockDecl;
 class CXXBoolLiteralExpr;
 class CXXMethodDecl;
 class CXXRecordDecl;
 class DeclaratorDecl;
 class FunctionDecl;
 class LocationContext;
 class StackFrameContext;
protected:
   /// Manages the creation of symbols.
   SymbolManager SymMgr;
 
   /// Manages the creation of memory regions.
   MemRegionManager MemMgr;
 
   ProgramStateManager &StateMgr;
 
+  const AnalyzerOptions &AnOpts;
+
   /// The scalar type to use for array indices.
   const QualType ArrayIndexTy;
 
   /// The width of the scalar type used for array indices.
   const unsigned ArrayIndexWidth;
 
   SVal evalCastKind(UndefinedVal V, QualType CastTy, QualType OriginalTy);
   SVal evalCastKind(UnknownVal V, QualType CastTy, QualType OriginalTy);
SVal evalCastSubKind(nonloc::LocAsInteger V, QualType CastTy,
                        QualType OriginalTy);
   SVal evalCastSubKind(nonloc::SymbolVal V, QualType CastTy,
                        QualType OriginalTy);
   SVal evalCastSubKind(nonloc::PointerToMember V, QualType CastTy,
                        QualType OriginalTy);
 
 public:
   SValBuilder(llvm::BumpPtrAllocator &alloc, ASTContext &context,
-              ProgramStateManager &stateMgr)
-      : Context(context), BasicVals(context, alloc),
-        SymMgr(context, BasicVals, alloc), MemMgr(context, alloc),
-        StateMgr(stateMgr), ArrayIndexTy(context.LongLongTy),
-        ArrayIndexWidth(context.getTypeSize(ArrayIndexTy)) {}
+              ProgramStateManager &stateMgr);
 
   virtual ~SValBuilder() = default;
 
   bool haveSameType(const SymExpr *Sym1, const SymExpr *Sym2) {
     return haveSameType(Sym1->getType(), Sym2->getType());
   }
 
   bool haveSameType(QualType Ty1, QualType Ty2) {
public:
   const BasicValueFactory &getBasicValueFactory() const { return BasicVals; }
 
   SymbolManager &getSymbolManager() { return SymMgr; }
   const SymbolManager &getSymbolManager() const { return SymMgr; }
 
   MemRegionManager &getRegionManager() { return MemMgr; }
   const MemRegionManager &getRegionManager() const { return MemMgr; }
 
+  const AnalyzerOptions &getAnalyzerOptions() const { return AnOpts; }
+
   // Forwarding methods to SymbolManager.
 
   const SymbolConjured* conjureSymbol(const Stmt *stmt,
                                       const LocationContext *LCtx,
                                       QualType type,
                                       unsigned visitCount,
                                       const void *symbolTag = nullptr) {
     return SymMgr.conjureSymbol(stmt, LCtx, type, visitCount, symbolTag);

clang/lib/StaticAnalyzer/Core/MemRegion.cpp

 #include "clang/AST/Type.h"
 #include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/Analysis/Support/BumpVector.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/SmallString.h"
const auto isFlexibleArrayMemberCandidate = [this,
       if (isa<IncompleteArrayType>(AT))
         return true;
 
       if (const auto *CAT = dyn_cast<ConstantArrayType>(AT)) {
         const llvm::APInt &Size = CAT->getSize();
         if (Size.isNullValue())
           return true;
 
-        // FIXME: Acquire the AnalyzerOptions in a simpler way.
-        const AnalyzerOptions &Opts = SVB.getStateManager()
-                                          .getOwningEngine()
-                                          .getAnalysisManager()
-                                          .getAnalyzerOptions();
+        const AnalyzerOptions &Opts = SVB.getAnalyzerOptions();
         if (Opts.ShouldConsiderSingleElementArraysAsFlexibleArrayMembers &&
             Size.isOneValue())
           return true;
       }
       return false;
     };
 
     if (isFlexibleArrayMemberCandidate(Ty))

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

 using namespace ento;
 
 //===----------------------------------------------------------------------===//
 // Basic SVal creation.
 //===----------------------------------------------------------------------===//
 
 void SValBuilder::anchor() {}
 
+SValBuilder::SValBuilder(llvm::BumpPtrAllocator &alloc, ASTContext &context,
+                         ProgramStateManager &stateMgr)
+    : Context(context), BasicVals(context, alloc),
+      SymMgr(context, BasicVals, alloc), MemMgr(context, alloc),
+      StateMgr(stateMgr),
+      AnOpts(
+          stateMgr.getOwningEngine().getAnalysisManager().getAnalyzerOptions()),
+      ArrayIndexTy(context.LongLongTy),
+      ArrayIndexWidth(context.getTypeSize(ArrayIndexTy)) {}
+
 DefinedOrUnknownSVal SValBuilder::makeZeroVal(QualType type) {
   if (Loc::isLocType(type))
     return makeNull();
 
   if (type->isIntegralOrEnumerationType())
     return makeIntVal(0, type);
 
   if (type->isArrayType() || type->isRecordType() || type->isVectorType() ||

 SVal SValBuilder::makeSymExprValNN(BinaryOperator::Opcode Op,
                                    NonLoc LHS, NonLoc RHS,
                                    QualType ResultTy) {
   SymbolRef symLHS = LHS.getAsSymbol();
   SymbolRef symRHS = RHS.getAsSymbol();
 
   // TODO: When the Max Complexity is reached, we should conjure a symbol
   // instead of generating an Unknown value and propagate the taint info to it.
-  const unsigned MaxComp = StateMgr.getOwningEngine()
-                               .getAnalysisManager()
-                               .options.MaxSymbolComplexity;
+  const unsigned MaxComp = AnOpts.MaxSymbolComplexity;
 
   if (symLHS && symRHS &&
       (symLHS->computeComplexity() + symRHS->computeComplexity()) <  MaxComp)
     return makeNonLoc(symLHS, Op, symRHS, ResultTy);
 
   if (symLHS && symLHS->computeComplexity() < MaxComp)
     if (Optional<nonloc::ConcreteInt> rInt = RHS.getAs<nonloc::ConcreteInt>())
       return makeNonLoc(symLHS, Op, rInt->getValue(), ResultTy);

clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

 // SimpleSValBuilder.cpp - A basic SValBuilder -----------------------*- C++ -*-
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file defines SimpleSValBuilder, a basic implementation of SValBuilder.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SValVisitor.h"
 
 using namespace clang;
 using namespace ento;
 
 namespace {
 class SimpleSValBuilder : public SValBuilder {
 public:
   SimpleSValBuilder(llvm::BumpPtrAllocator &alloc, ASTContext &context,
                     ProgramStateManager &stateMgr)
-                    : SValBuilder(alloc, context, stateMgr) {}
   ~SimpleSValBuilder() override {}
 
   SVal evalMinus(NonLoc val) override;
   SVal evalComplement(NonLoc val) override;
   SVal evalBinOpNN(ProgramStateRef state, BinaryOperator::Opcode op,
                    NonLoc lhs, NonLoc rhs, QualType resultTy) override;
   SVal evalBinOpLL(ProgramStateRef state, BinaryOperator::Opcode op,
                    Loc lhs, Loc rhs, QualType resultTy) override;
public:
   SVal MakeSymIntVal(const SymExpr *LHS, BinaryOperator::Opcode op,
                      const llvm::APSInt &RHS, QualType resultTy);
 };
 } // end anonymous namespace
 
 SValBuilder *ento::createSimpleSValBuilder(llvm::BumpPtrAllocator &alloc,
                                            ASTContext &context,
                                            ProgramStateManager &stateMgr) {
   return new SimpleSValBuilder(alloc, context, stateMgr);
 }
 
 //===----------------------------------------------------------------------===//
 // Transfer function for unary operators.
 //===----------------------------------------------------------------------===//
 
 SVal SimpleSValBuilder::evalMinus(NonLoc val) {
   switch (val.getSubKind()) {
   case nonloc::ConcreteIntKind:
static Optional<NonLoc> tryRearrange(ProgramStateRef State,
                                      BinaryOperator::Opcode Op, NonLoc Lhs,
                                      NonLoc Rhs, QualType ResultTy) {
   ProgramStateManager &StateMgr = State->getStateManager();
   SValBuilder &SVB = StateMgr.getSValBuilder();
 
   // We expect everything to be of the same type - this type.
   QualType SingleTy;
 
-  auto &Opts =
-    StateMgr.getOwningEngine().getAnalysisManager().getAnalyzerOptions();
-
   // FIXME: After putting complexity threshold to the symbols we can always
   //        rearrange additive operations but rearrange comparisons only if
   //        option is set.
-  if(!Opts.ShouldAggressivelySimplifyBinaryOperation)
+  if (!SVB.getAnalyzerOptions().ShouldAggressivelySimplifyBinaryOperation)
     return None;
 
   SymbolRef LSym = Lhs.getAsSymbol();
   if (!LSym)
     return None;
 
   if (BinaryOperator::isComparisonOp(Op)) {
     SingleTy = LSym->getType();