[clang][dataflow] unnamed bitfields should be discarded in InitListExpr

[paulsemel]

Summary

There is a mapping issue between InitListExpr inits and RecordDecl's fields when dealing with Unnamed bitfields. Indeed, those are only used for padding, and thus are not appearing in InitlistExpr inits, which means we can off-by-one the fields to inits mapping while initializing struct's fields.

[paulsemel]

clang/lib/Analysis/FlowSensitive/Transfer.cpp

739

Is there a better way to do this? I tried using llvm::make_filter_range instead, but it didn't seem to work well with llvm::zip...

[ymandel]

Thanks for this fix! Would you mind expanding the description a bit? I had trouble understanding the details of the problem that is being addressed. Thanks!

clang/lib/Analysis/FlowSensitive/Transfer.cpp

735–743

Please comment indicating why you're dropping the unnamed bit fields.

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

5101

Please add comment explaining why there is not contents to the lambda.

[ymandel]

Thanks!

clang/lib/Analysis/FlowSensitive/Transfer.cpp

735–743

clang/lib/Analysis/FlowSensitive/Transfer.cpp

void VisitInitListExpr(const InitListExpr *S) {
 
     auto *Val = Env.createValue(Type);
     if (Val == nullptr)
       return;
 
     Env.setValue(Loc, *Val);
 
     if (Type->isStructureOrClassType()) {
-      for (auto It : llvm::zip(Type->getAsRecordDecl()->fields(), S->inits())) {
+      // Unnamed bitfields are only used for padding and are not appearing in
+      // `InitListExpr`'s inits. However, those fields do appear in RecordDecl's
+      // field list, and we thus need to remove them before mapping inits to
+      // fields to avoid mapping inits to the wrongs fields.
+      std::vector<FieldDecl *> Fields;
+      llvm::copy_if(
+          Type->getAsRecordDecl()->fields(), std::back_inserter(Fields),
+          [](const FieldDecl *Field) { return !Field->isUnnamedBitfield(); });
+      for (auto It : llvm::zip(Fields, S->inits())) {
         const FieldDecl *Field = std::get<0>(It);
         assert(Field != nullptr);
 
         const Expr *Init = std::get<1>(It);
         assert(Init != nullptr);
 
         if (Value *InitVal = Env.getValue(*Init, SkipPast::None))
           cast<StructValue>(Val)->setChild(*Field, *InitVal);

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

runDataflow(
         ASSERT_THAT(FooDecl, NotNull());
 
         auto &FooVal = *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
         EXPECT_TRUE(Env.flowConditionImplies(FooVal));
       },
       {BuiltinOptions{ContextSensitiveOptions{}}});
 }
 
+TEST(TransferTest, UnnamedBitfieldInitializer) {
+  std::string Code = R"(
+    struct B {};
+    struct A {
+      unsigned a;
+      unsigned : 4;
+      unsigned c;
+      B b;
+    };
+    void target() {
+      A a = {};
+      A test = a;
+      (void)test.c;
+    }
+  )";
+  runDataflow(
+      Code,
+      [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+         ASTContext &ASTCtx) {
+        // This doesn't need a body because this test was crashing the framework
+        // before handling correctly Unnamed bitfields in `InitListExpr`.
+      });
+}
+
 } // namespace