[PATCH] D116048: [clang][CodeGen][UBSan] VLA size checking for unsigned integer parameter

[Adam Magier via Phabricator via cfe-commits]

AdamMagierFOSS added a comment.

Thanks once again for the feedback - I'll make the changes and commit directly.



================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:2256
+          llvm::Value *size = EmitScalarExpr(sizeExpr);
+          clang::QualType sizeExprType = sizeExpr->getType();
 
----------------
rjmccall wrote:
> You can sink this into the `if` block.
Sure thing.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:2275
           // undefined behavior to have a negative bound.
-          entry = Builder.CreateIntCast(Size, SizeTy, /*signed*/ false);
+          MapEntry = Builder.CreateIntCast(Size, SizeTy, /*signed*/ false);
         }
----------------
rjmccall wrote:
> AdamMagierFOSS wrote:
> > rjmccall wrote:
> > > This would be a different bug, but should UBSan also be doing a bounds check if the type is larger than `size_t`?
> > Interesting point, I'd have to reread through the spec to give a precise/definitive answer. To keep this review focused I'll table the discussion for a separate forum.
> I'm pretty sure you should, but it's fine to do it in a different patch.  Please leave a FIXME about it, though.
My gut says the check should be performed as you say, but I can't prove it to myself in a satisfactory manner at the moment. I'll leave a FIXME for this as you requested.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116048/new/

https://reviews.llvm.org/D116048