[PATCH] D116048: [clang][CodeGen][UBSan] VLA size checking for unsigned integer parameter
John McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 21 11:27:45 PST 2021
rjmccall added a comment.
Thanks, patch basically looks good. Minor requests and an observation for possible follow-up work.
================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:2247
// Otherwise, evaluate and record it.
- if (const Expr *size = vat->getSizeExpr()) {
+ if (const Expr *SizeExpr = vat->getSizeExpr()) {
// It's possible that we might have emitted this already,
----------------
Please continue to use lowercase names for local variables for consistency with the surrounding code. The rename to `sizeExpr` is good; you can then rename `Size` to `size` or `sizeValue`.
================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:2259
// greater than zero.
- if (SanOpts.has(SanitizerKind::VLABound) &&
- size->getType()->isSignedIntegerType()) {
+ if (SanOpts.has(SanitizerKind::VLABound) && SEType->isIntegerType()) {
SanitizerScope SanScope(this);
----------------
Sema requires the bound expression to have integral type, so you don't need to do this.
================
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);
}
----------------
This would be a different bug, but should UBSan also be doing a bounds check if the type is larger than `size_t`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116048/new/
https://reviews.llvm.org/D116048
More information about the cfe-commits
mailing list