[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:6311-6314
@@ +6310,6 @@
+  QualType BaseType;
+  if (auto *E = Base.dyn_cast<const Expr*>())
+    BaseType = E->getType();
+  else
+    BaseType = Base.get<const ValueDecl*>()->getType();
+  // The outermost BaseType may be a pointer if we got an expression like
----------------
`BaseType = getType(Base);`

================
Comment at: lib/AST/ExprConstant.cpp:6315-6318
@@ +6314,6 @@
+    BaseType = Base.get<const ValueDecl*>()->getType();
+  // The outermost BaseType may be a pointer if we got an expression like
+  // `Foo->Bar`.
+  if (BaseType->isPointerType())
+    BaseType = BaseType->getPointeeType();
+
----------------
This seems like the wrong way to handle this case (we'll be computing the wrong type for such `LValue`s in other cases). In `LValueExprEvaluatorBase::VisitMemberExpr`, we should use

  Result.setInvalid(E);
  return false;

instead of

  Result.setInvalid(E->getBase());
  // fall through to perform member access

I thought we'd already made that change, but it appears not.

================
Comment at: lib/AST/ExprConstant.cpp:6331-6333
@@ +6330,5 @@
+      BaseType = CAT->getElementType();
+    } else if (BaseType->isAnyComplexType()) {
+      auto *CT = BaseType->castAs<ComplexType>();
+      BaseType = CT->getElementType();
+    } else if (auto *FD = getAsField(LVal.Designator.Entries[I])) {
----------------
You should check that the `ArrayIndex` is 1 in this case (a pointer to the `__real__` component doesn't point to the end of the object).

================
Comment at: lib/AST/ExprConstant.cpp:6343-6348
@@ +6342,8 @@
+    } else {
+      // Don't update BaseType because we don't want to answer 'true' in:
+      // struct Base { char Foo[1]; };
+      // struct Derived : public Base { char Bar[1]; } d;
+      // __builtin_object_size(((Base*)&d)->Foo, 1)
+      assert(getAsBaseClass(LVal.Designator.Entries[I]) != nullptr &&
+          "Expecting cast to a base class");
+    }
----------------
Not updating `BaseType` will cause odd things to happen on the later iterations of this loop. You should probably just `return false` here.


http://reviews.llvm.org/D12821