[PATCH] D17407: [Sema] PR25755 Fix crash when initializing out-of-order struct references

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Sat Mar 19 04:07:04 PDT 2016


rsmith added inline comments.

================
Comment at: lib/Sema/SemaInit.cpp:1815-1816
@@ -1814,1 +1814,4 @@
   // worthwhile to skip over the rest of the initializer, though.
+  unsigned FieldIdx = 0;
+  unsigned MaxFieldIdx = 0;
+  llvm::BitVector SeenFields;
----------------
You don't need either of these variables.

================
Comment at: lib/Sema/SemaInit.cpp:1841-1850
@@ +1840,12 @@
+      if (!hadError) {
+        if (CheckForMissingFields) {
+          unsigned FieldSize{0};
+          for (RecordDecl::field_iterator f = FieldStart; f != FieldEnd; ++f) {
+            FieldSize++;
+          }
+          MaxFieldIdx = FieldSize - 1;
+          SeenFields.resize(FieldSize);
+          for (unsigned i = 0; i < FieldIdx; ++i)
+            SeenFields.set(i);
+        }
+        if (Field == FieldEnd) {
----------------
Drop this. Instead, check for `CheckForMissingFields` in the loop below.

================
Comment at: lib/Sema/SemaInit.cpp:1852
@@ +1851,3 @@
+        if (Field == FieldEnd) {
+          FieldIdx = MaxFieldIdx;
+        } else {
----------------
Use `RD->getNumFields() - 1` here.

================
Comment at: lib/Sema/SemaInit.cpp:1914
@@ +1913,3 @@
+    if (!CheckForMissingFields && !hadError)
+      SeenFields.set(FieldIdx);
+    ++FieldIdx;
----------------
Use `Field->getFieldIndex()`

================
Comment at: lib/Sema/SemaInit.cpp:1936-1954
@@ +1935,21 @@
+  if (!hadError) {
+    SmallVector<RecordDecl::field_iterator, 10> MissingFields;
+    if (CheckForMissingFields) {
+      FieldStart = Field;
+    } else {
+      for (int Idx = 0, SeenIdx = SeenFields.find_first(); SeenIdx != -1;
+           SeenIdx = SeenFields.find_next(SeenIdx)) {
+        while (Idx < SeenIdx) {
+          MissingFields.push_back(FieldStart);
+          ++FieldStart;
+          ++Idx;
+        }
+        ++FieldStart;
+        ++Idx;
+      }
+    }
+    while (FieldStart != FieldEnd) {
+      MissingFields.push_back(FieldStart);
+      ++FieldStart;
+    }
+
----------------
Don't build a list of missing fields, just walk all the fields and check whether each one is in the set before processing it. (You can optimize this somewhat: if there were any designators -- including the case where `Field` was not `RD->field_begin()` on entry to this function -- start from `RD->field_begin()`, otherwise start from `Field`.)


http://reviews.llvm.org/D17407





More information about the cfe-commits mailing list