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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits