[clang][dataflow] Fix 2 bugs in `MemberExpr` interpretation.

[ymandel]

Summary

There were two (small) bugs causing crashes in the analysis. This patch fixes both of them.

1. An enum value was accessed as a class member. Now, the engine gracefully ignores such member expressions.

2. Field access in MemberExpr of struct/class-typed global variables. Analysis didn't interpret fields of global vars, because the vars were initialized before the fields were added to the "allowlist". Now, the allowlist is set _before_ init of globals.

[gribozavr2]

LGTM

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

if (const auto *CtorDecl = dyn_cast<CXXConstructorDecl>(&DeclCtx)) {
           Fields.insert(M);
         const Expr *E = Init->getInit();
         assert(E != nullptr);
         getFieldsAndGlobalVars(*E, Fields, Vars);
       }
     }
     getFieldsAndGlobalVars(*FuncDecl->getBody(), Fields, Vars);
 
-    initVars(Vars);
-    // These have to be set before the lines that follow to ensure that create*
-    // work correctly for structs.
+    // These have to be added before the lines that follow to ensure that
+    // `create*` work correctly for structs.
     DACtx.addModeledFields(Fields);
 
+    initVars(Vars);
+
     for (const auto *ParamDecl : FuncDecl->parameters()) {
       assert(ParamDecl != nullptr);
       auto &ParamLoc = createStorageLocation(*ParamDecl);
       setStorageLocation(*ParamDecl, ParamLoc);
       if (Value *ParamVal = createValue(ParamDecl->getType()))
         setValue(ParamLoc, *ParamVal);
     }
 
for (const auto *Init : CtorDecl->inits()) {
       if (const auto *M = Init->getAnyMember())
         Fields.insert(M);
       const Expr *E = Init->getInit();
       assert(E != nullptr);
       getFieldsAndGlobalVars(*E, Fields, Vars);
     }
   }
   getFieldsAndGlobalVars(*FuncDecl->getBody(), Fields, Vars);
-  initVars(Vars);
+
+  // These have to be added before the lines that follow to ensure that
+  // `create*` work correctly for structs.
   DACtx->addModeledFields(Fields);
 
+  initVars(Vars);
+
   const auto *ParamIt = FuncDecl->param_begin();
 
   // FIXME: Parameters don't always map to arguments 1:1; examples include
   // overloaded operators implemented as member functions, and parameter packs.
   for (unsigned ArgIndex = 0; ArgIndex < Args.size(); ++ParamIt, ++ArgIndex) {
     assert(ParamIt != FuncDecl->param_end());
 
     const Expr *Arg = Args[ArgIndex];

clang/lib/Analysis/FlowSensitive/Transfer.cpp

public:
   void VisitMemberExpr(const MemberExpr *S) {
     ValueDecl *Member = S->getMemberDecl();
     assert(Member != nullptr);
 
     // FIXME: Consider assigning pointer values to function member expressions.
     if (Member->isFunctionOrFunctionTemplate())
       return;
 
+    // FIXME: if/when we add support for modeling enums, use that support here.
+    if (isa<EnumConstantDecl>(Member))
+      return;
+
     if (auto *D = dyn_cast<VarDecl>(Member)) {
       if (D->hasGlobalStorage()) {
         auto *VarDeclLoc = Env.getStorageLocation(*D, SkipPast::None);
         if (VarDeclLoc == nullptr)
           return;
 
         if (VarDeclLoc->getType()->isReferenceType()) {
           Env.setStorageLocation(*S, *VarDeclLoc);

clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp

TEST_F(EnvironmentTest, InitGlobalVarsFun) {
   ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
 
   auto Results =
       match(decl(anyOf(varDecl(hasName("Global")).bind("global"),
                        functionDecl(hasName("Target")).bind("target"))),
             Context);
   const auto *Fun = selectFirst<FunctionDecl>("target", Results);
   const auto *Var = selectFirst<VarDecl>("global", Results);
-  ASSERT_TRUE(Fun != nullptr);
+  ASSERT_THAT(Fun, NotNull());
   ASSERT_THAT(Var, NotNull());
 
   // Verify the global variable is populated when we analyze `Target`.
   Environment Env(DAContext, *Fun);
   EXPECT_THAT(Env.getValue(*Var, SkipPast::None), NotNull());
 }
 
+TEST_F(EnvironmentTest, InitGlobalVarsFieldFun) {
+  using namespace ast_matchers;
+
+  std::string Code = R"cc(
+     struct S { int Bar; };
+     S Global = {0};
+     int Target () { return Global.Bar; }
+  )cc";
+
+  auto Unit =
+      tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++11"});
+  auto &Context = Unit->getASTContext();
+
+  ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
+
+  auto Results =
+      match(decl(anyOf(varDecl(hasName("Global")).bind("global"),
+                       functionDecl(hasName("Target")).bind("target"))),
+            Context);
+  const auto *Fun = selectFirst<FunctionDecl>("target", Results);
+  const auto *GlobalDecl = selectFirst<VarDecl>("global", Results);
+  ASSERT_THAT(Fun, NotNull());
+  ASSERT_THAT(GlobalDecl, NotNull());
+
+  ASSERT_TRUE(GlobalDecl->getType()->isStructureType());
+  auto GlobalFields = GlobalDecl->getType()->getAsRecordDecl()->fields();
+
+  FieldDecl *BarDecl = nullptr;
+  for (FieldDecl *Field : GlobalFields) {
+    if (Field->getNameAsString() == "Bar") {
+      BarDecl = Field;
+      break;
+    }
+    FAIL() << "Unexpected field: " << Field->getNameAsString();
+  }
+  ASSERT_THAT(BarDecl, NotNull());
+
+  // Verify the global variable is populated when we analyze `Target`.
+  Environment Env(DAContext, *Fun);
+  const auto *GlobalLoc = cast<AggregateStorageLocation>(
+      Env.getStorageLocation(*GlobalDecl, SkipPast::None));
+  const auto *GlobalVal = cast<StructValue>(Env.getValue(*GlobalLoc));
+  const auto *BarVal = GlobalVal->getChild(*BarDecl);
+  ASSERT_THAT(BarVal, NotNull());
+  EXPECT_TRUE(isa<IntegerValue>(BarVal));
+}
+
 TEST_F(EnvironmentTest, InitGlobalVarsConstructor) {
   using namespace ast_matchers;
 
   std::string Code = R"cc(
      int Global = 0;
      struct Target {
        Target() : Field(Global) {}
        int Field;

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

runDataflow(
 
         const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
         ASSERT_THAT(BazDecl, NotNull());
 
         EXPECT_EQ(Env.getValue(*BazDecl, SkipPast::None), BarVal);
       });
 }
 
+TEST(TransferTest, StructMemberEnum) {
+  std::string Code = R"(
+    struct A {
+      int Bar;
+      enum E { ONE, TWO };
+    };
+
+    void target(A Foo) {
+      A::E Baz = Foo.ONE;
+      // [[p]]
+    }
+  )";
+  // Minimal expectations -- we're just testing that it doesn't crash, since
+  // enums aren't interpreted.
+  runDataflow(
+      Code,
+      [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+         ASTContext &ASTCtx) {
+        EXPECT_THAT(Results.keys(), UnorderedElementsAre("p"));
+      });
+}
+
 TEST(TransferTest, DerivedBaseMemberClass) {
   std::string Code = R"(
     class A {
       int ADefault;
     protected:
       int AProtected;
     private:
       int APrivate;