[PATCH] D12821: Allow for C's "writing off the end" idiom in __builtin_object_size

[Richard Smith via cfe-commits]

rsmith added inline comments.

================
Comment at: lib/AST/ExprConstant.cpp:165
@@ -159,1 +164,3 @@
 
+    /// Indicator of whether the most-derived object is an array.
+    bool MostDerivedIsArrayElement : 1;
----------------
array -> array element.

================
Comment at: lib/AST/ExprConstant.cpp:4457-4460
@@ -4434,1 +4456,6 @@
 
+    // Because we set the Base to be the MemberExpr instead of E->getBase(), the
+    // Offset should be from the MemberExpr instead of the MemberExpr's base.
+    if (Result.InvalidBase)
+      Result.Offset = CharUnits::Zero();
+
----------------
I think you should bail out above, with the base set to the `MemberExpr` and with an empty designator, rather than applying a member designator on top of a base that already refers to the member (and then needing to undo the effect on the offset here).

================
Comment at: lib/AST/ExprConstant.cpp:6402
@@ +6401,3 @@
+  // that behaves this way.
+  if (End.InvalidBase && (Type & 1) != 0 &&
+      End.Designator.MostDerivedIsArrayElement &&
----------------
This only seems necessary when `Type` is 1; for type 3, returning 1 would be conservatively correct as a lower bound on the known-accessible storage, and is better than giving up here and evaluating the object size as 0.

================
Comment at: lib/AST/ExprConstant.cpp:6403-6404
@@ +6402,4 @@
+  if (End.InvalidBase && (Type & 1) != 0 &&
+      End.Designator.MostDerivedIsArrayElement &&
+      End.Designator.MostDerivedArraySize < 2) {
+    // EM_FoldDesignator requires that all invalid bases be MemberExprs
----------------
Ideally, we should walk the designator and make sure that at each step we picked the last subobject (last array element, last field, ...) -- that is, we want to know that we're really looking at a trailing flexible array member, and not just a size-1 array in the middle of some object.

You should also check that the designator refers to the most-derived object (that is, that `Entries.size() == MostDerivedPathLength`), because given:

  struct A { char buffer[10]; };
  struct B : A {};
  struct C { B b[1]; } *c;
  int m = __builtin_object_size(c->b[0], 1);
  int n = __builtin_object_size((A*)c->b[0], 1);

... we should presumably compute `m == -1` but `n == 10`, because for `n` the `A` subobject of the `B` object is known to have size 10, even though we're looking at a base subobject of a size-1 trailing array.


http://reviews.llvm.org/D12821