[clang][dataflow] Various changes to handling of modeled fields.

- Rename `getReferencedFields()` to `getModeledFields()`. Move the logic that
  returns all object fields when doing a context-sensitive analysis to here from
  `DataflowAnalysisContext::createStorageLocation()`. I think all callers of
  the previous `getReferencedFields()` should use this logic; the fact that they
  were not doing so looks like a bug.

- Make `getModeledFields()` public. I have an upcoming patch that will need to
  use this function from Transfer.cpp, and there doesn't seem to be any reason
  why this function should not be public.

- Use a `SmallSetVector` to get deterministic iteration order. I have a pending
  patch where I'm getting flaky tests because
  `Environment::createValueUnlessSelfReferential()` is non-deterministically
  populating different fields depending on the iteration order. This change
  fixes those flaky tests.

Reviewed By: gribozavr2

Differential Revision: https://reviews.llvm.org/D154586

clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h

@@ -51,8 +51,12 @@ class Logger;
 const Expr &ignoreCFGOmittedNodes(const Expr &E);
 const Stmt &ignoreCFGOmittedNodes(const Stmt &S);
 
+/// A set of `FieldDecl *`. Use `SmallSetVector` to guarantee deterministic
+/// iteration order.
+using FieldSet = llvm::SmallSetVector<const FieldDecl *, 4>;
+
 /// Returns the set of all fields in the type.
-llvm::DenseSet<const FieldDecl *> getObjectFields(QualType Type);
+FieldSet getObjectFields(QualType Type);
 
 struct ContextSensitiveOptions {
   /// The maximum depth to analyze. A value of zero is equivalent to disabling
@@ -181,6 +185,10 @@ class DataflowAnalysisContext {
   /// been stored in flow conditions.
   Solver::Result querySolver(llvm::SetVector<const Formula *> Constraints);
 
+  /// Returns the fields of `Type`, limited to the set of fields modeled by this
+  /// context.
+  FieldSet getModeledFields(QualType Type);
+
 private:
   friend class Environment;
 
@@ -196,11 +204,7 @@ class DataflowAnalysisContext {
   };
 
   // Extends the set of modeled field declarations.
-  void addModeledFields(const llvm::DenseSet<const FieldDecl *> &Fields);
-
-  /// Returns the fields of `Type`, limited to the set of fields modeled by this
-  /// context.
-  llvm::DenseSet<const FieldDecl *> getReferencedFields(QualType Type);
+  void addModeledFields(const FieldSet &Fields);
 
   /// Adds all constraints of the flow condition identified by `Token` and all
   /// of its transitive dependencies to `Constraints`. `VisitedTokens` is used
@@ -257,7 +261,7 @@ class DataflowAnalysisContext {
   llvm::DenseMap<const FunctionDecl *, ControlFlowContext> FunctionContexts;
 
   // Fields modeled by environments covered by this context.
-  llvm::DenseSet<const FieldDecl *> ModeledFields;
+  FieldSet ModeledFields;
 
   std::unique_ptr<Logger> LogOwner; // If created via flags.
 };

clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp

@@ -40,31 +40,28 @@ static llvm::cl::opt<std::string> DataflowLog(
 namespace clang {
 namespace dataflow {
 
-void DataflowAnalysisContext::addModeledFields(
-    const llvm::DenseSet<const FieldDecl *> &Fields) {
-  llvm::set_union(ModeledFields, Fields);
+FieldSet DataflowAnalysisContext::getModeledFields(QualType Type) {
+  // During context-sensitive analysis, a struct may be allocated in one
+  // function, but its field accessed in a function lower in the stack than
+  // the allocation. Since we only collect fields used in the function where
+  // the allocation occurs, we can't apply that filter when performing
+  // context-sensitive analysis. But, this only applies to storage locations,
+  // since field access it not allowed to fail. In contrast, field *values*
+  // don't need this allowance, since the API allows for uninitialized fields.
+  if (Opts.ContextSensitiveOpts)
+    return getObjectFields(Type);
+
+  return llvm::set_intersection(getObjectFields(Type), ModeledFields);
 }
 
-llvm::DenseSet<const FieldDecl *>
-DataflowAnalysisContext::getReferencedFields(QualType Type) {
-  llvm::DenseSet<const FieldDecl *> Fields = getObjectFields(Type);
-  llvm::set_intersect(Fields, ModeledFields);
-  return Fields;
+void DataflowAnalysisContext::addModeledFields(const FieldSet &Fields) {
+  ModeledFields.set_union(Fields);
 }
 
 StorageLocation &DataflowAnalysisContext::createStorageLocation(QualType Type) {
   if (!Type.isNull() && Type->isRecordType()) {
     llvm::DenseMap<const ValueDecl *, StorageLocation *> FieldLocs;
-    // During context-sensitive analysis, a struct may be allocated in one
-    // function, but its field accessed in a function lower in the stack than
-    // the allocation. Since we only collect fields used in the function where
-    // the allocation occurs, we can't apply that filter when performing
-    // context-sensitive analysis. But, this only applies to storage locations,
-    // since field access it not allowed to fail. In contrast, field *values*
-    // don't need this allowance, since the API allows for uninitialized fields.
-    auto Fields = Opts.ContextSensitiveOpts ? getObjectFields(Type)
-                                            : getReferencedFields(Type);
-    for (const FieldDecl *Field : Fields)
+    for (const FieldDecl *Field : getModeledFields(Type))
       FieldLocs.insert({Field, &createStorageLocation(Field->getType())});
     return arena().create<AggregateStorageLocation>(Type, std::move(FieldLocs));
   }
@@ -305,9 +302,8 @@ const Stmt &clang::dataflow::ignoreCFGOmittedNodes(const Stmt &S) {
 
 // FIXME: Does not precisely handle non-virtual diamond inheritance. A single
 // field decl will be modeled for all instances of the inherited field.
-static void
-getFieldsFromClassHierarchy(QualType Type,
-                            llvm::DenseSet<const FieldDecl *> &Fields) {
+static void getFieldsFromClassHierarchy(QualType Type,
+                                        clang::dataflow::FieldSet &Fields) {
   if (Type->isIncompleteType() || Type->isDependentType() ||
       !Type->isRecordType())
     return;
@@ -320,9 +316,8 @@ getFieldsFromClassHierarchy(QualType Type,
 }
 
 /// Gets the set of all fields in the type.
-llvm::DenseSet<const FieldDecl *>
-clang::dataflow::getObjectFields(QualType Type) {
-  llvm::DenseSet<const FieldDecl *> Fields;
+clang::dataflow::FieldSet clang::dataflow::getObjectFields(QualType Type) {
+  FieldSet Fields;
   getFieldsFromClassHierarchy(Type, Fields);
   return Fields;
 }

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

@@ -170,8 +170,7 @@ static void insertIfFunction(const Decl &D,
 }
 
 static void
-getFieldsGlobalsAndFuncs(const Decl &D,
-                         llvm::DenseSet<const FieldDecl *> &Fields,
+getFieldsGlobalsAndFuncs(const Decl &D, FieldSet &Fields,
                          llvm::DenseSet<const VarDecl *> &Vars,
                          llvm::DenseSet<const FunctionDecl *> &Funcs) {
   insertIfGlobal(D, Vars);
@@ -188,8 +187,7 @@ getFieldsGlobalsAndFuncs(const Decl &D,
 /// global variables and functions that are declared in or referenced from
 /// sub-statements.
 static void
-getFieldsGlobalsAndFuncs(const Stmt &S,
-                         llvm::DenseSet<const FieldDecl *> &Fields,
+getFieldsGlobalsAndFuncs(const Stmt &S, FieldSet &Fields,
                          llvm::DenseSet<const VarDecl *> &Vars,
                          llvm::DenseSet<const FunctionDecl *> &Funcs) {
   for (auto *Child : S.children())
@@ -222,7 +220,7 @@ getFieldsGlobalsAndFuncs(const Stmt &S,
 void Environment::initFieldsGlobalsAndFuncs(const FunctionDecl *FuncDecl) {
   assert(FuncDecl->getBody() != nullptr);
 
-  llvm::DenseSet<const FieldDecl *> Fields;
+  FieldSet Fields;
   llvm::DenseSet<const VarDecl *> Vars;
   llvm::DenseSet<const FunctionDecl *> Funcs;
 
@@ -708,7 +706,7 @@ void Environment::setValue(const StorageLocation &Loc, Value &Val) {
     const QualType Type = AggregateLoc.getType();
     assert(Type->isRecordType());
 
-    for (const FieldDecl *Field : DACtx->getReferencedFields(Type)) {
+    for (const FieldDecl *Field : DACtx->getModeledFields(Type)) {
       assert(Field != nullptr);
       StorageLocation &FieldLoc = AggregateLoc.getChild(*Field);
       MemberLocToStruct[&FieldLoc] = std::make_pair(StructVal, Field);
@@ -846,7 +844,7 @@ Value *Environment::createValueUnlessSelfReferential(
   if (Type->isRecordType()) {
     CreatedValuesCount++;
     llvm::DenseMap<const ValueDecl *, Value *> FieldValues;
-    for (const FieldDecl *Field : DACtx->getReferencedFields(Type)) {
+    for (const FieldDecl *Field : DACtx->getModeledFields(Type)) {
       assert(Field != nullptr);
 
       QualType FieldType = Field->getType();