george.burgess.iv added a comment.
Looks solid to me overall; just a few nits.
I don't think I have actual ownership over any of this code, so I'll defer to
either Aaron or Richard for the final LGTM
Thanks again!
================
Comment at: clang/lib/Sema/SemaChecking.cpp:317
+ // buffer size would be.
+ auto computeObjectSize = [&](unsigned ObjIdx) {
+ // If the parameter has a pass_object_size attribute, then we should use
----------------
nit: I think the prevailing preference is to name lambdas as you'd name
variables, rather than as you'd name methods/functions
================
Comment at: clang/lib/Sema/SemaChecking.cpp:321
+ // assume type 0.
+ int BOSType = 0;
+ if (auto *POS = FD->getParamDecl(ObjIdx)->getAttr<PassObjectSizeAttr>())
----------------
Should this also try to consider `fortify_stdlib`?
================
Comment at: clang/lib/Sema/SemaChecking.cpp:367
+ DiagID = diag::warn_memcpy_chk_overflow;
+ if (!evaluateObjectSize(TheCall->getNumArgs()-1) ||
+ !evaluateSizeArg(TheCall->getNumArgs()-2))
----------------
nit: All of these cases (and the two lambdas above) look super similar. Might
it be clearer to just set `SizeIndex` and `ObjectIndex` variables from here,
and actually `evaluate` them before `UsedSize.ule(ComputedSize)`?
If not, I'm OK with this as-is.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:5929
+ checkFortifiedBuiltinMemoryFunction(FDecl, TheCall);
+
----------------
Why isn't this in CheckBuiltinFunctionCall?
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58797/new/
https://reviews.llvm.org/D58797
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
