[PATCH] D40940: [ubsan] Use pass_object_size info in bounds checks

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.


================
Comment at: lib/CodeGen/CGExpr.cpp:833
+  // Arrays don't have pass_object_size attributes, but if they have a constant
+  // size modifier it's the array size (C99 6.5.7.2p1).
+  if (auto *DecayedArrayTy = dyn_cast<DecayedType>(ParamDecl->getType()))
----------------
vsk wrote:
> efriedma wrote:
> > "int f(int a[10])" might look like an array, but it isn't: it's just a different syntax to declare a pointer.  So it's legal to "lie" in the signature.  (If you want to actually pass a pointer to an array, you have to write "int (*a)[10]".)  And the definition of "static" says "an array with at least as many elements as specified by the size expression", which isn't a maximum, so that doesn't really help either.
> > 
> > Most people would consider it bad style to put a number into the array bound which doesn't reflect reality, but I think we shouldn't try to check it unless the user explicitly requests it.
> My copy of the C99 draft (n1256) is a little fuzzy on this point [*]. There's enough of a gray area here that it seems appropriate to back out this part of the patch.
> 
> * It states: "In addition to optional type qualifiers and the keyword static, the [ and ] may delimit an expression or *. If they delimit an expression (which specifies the size of an array) ...". I took the parenthetical literally, and didn't know about the 'at least as many' language.
"which specifies the size of an array" in this context just means that it's the part of the array declarator which specifies the size of the array, not that it's the size of any particular array.  The decay rules for function parameters are the relevant bit of the standard.

And yes, this is all very confusing; this is why C++ has std::array.


https://reviews.llvm.org/D40940